Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add default code examples and behavior #23

Merged
merged 7 commits into from
Aug 19, 2024
Merged

Conversation

amareshsm
Copy link
Member

@amareshsm amareshsm commented Aug 13, 2024

Prerequisites checklist

What is the purpose of this pull request?

fixes #11

What changes did you make? (Give an overview)

Add default code examples and behavior

Related Issues

#11

Is there anything you'd like reviewers to focus on?

  1. The React Monaco Editor seems to support only JSON as the default mode, not JSONC. I tried passing jsonc as the language, but it wasn't recognized, so it will likely show errors for comments in the JSON.
  2. In js the editor breaks when we choose sourceType as module and ECMAScript version as latest.

@amareshsm amareshsm marked this pull request as ready for review August 13, 2024 20:26
Copy link

netlify bot commented Aug 13, 2024

Deploy Preview for eslint-code-explorer ready!

Name Link
🔨 Latest commit cac89e5
🔍 Latest deploy log https://app.netlify.com/sites/eslint-code-explorer/deploys/66bf87a6b71b0500087dc37c
😎 Deploy Preview https://deploy-preview-23--eslint-code-explorer.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

export type SourceType = Exclude<Options['sourceType'], undefined>;
export type Version = Exclude<Options['ecmaVersion'], undefined>;

type ExplorerState = {
tool: 'ast' | 'scope' | 'path';
setTool: (tool: ExplorerState['tool']) => void;

code: string;
setCode: (code: string) => void;
JSCode: string;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im not sure if we would want to scale the state variables like this. Going forward there are chances where we can actually have markdown as well? in that case we would have to then add a markdown code state and a setter for it. We might want to think of something generic for this case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I had the same thought while working on #24. This PR contains a lot of redundant code for the setter. I’ve simplified it to two states and will update the PR shortly.

src/App.tsx Outdated
import { ThemeProvider, useTheme } from './components/theme-provider';

function App() {
const { theme } = useTheme();
const { language, tool, code, setCode } = useExplorer();
const { language, tool, JSCode, setJSCode, JSONCode, setJSONCode } = useExplorer();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest jsCode and jsonCode to match our code conventions.

src/lib/const.ts Outdated
"key3": [1, 2, "3", 1e10, 1e-3]
}
}
`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
`
`.trim();

src/lib/const.ts Outdated
export default [
...js.configs.recommended,
getConfig()
];`;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove extra empty line at the start.

Suggested change
];`;
];`.trim();

@nzakas
Copy link
Member

nzakas commented Aug 15, 2024

I think we will probably need to create our own language definition for JSONC. There's playground to help with that:
https://microsoft.github.io/monaco-editor/monarch.html

We can do that separate from this PR.

@amareshsm
Copy link
Member Author

I think we will probably need to create our own language definition for JSONC. There's playground to help with that: https://microsoft.github.io/monaco-editor/monarch.html

We can do that separate from this PR.

Sure I will check. when I checked online suggested to create own language definition.

@nzakas
Copy link
Member

nzakas commented Aug 15, 2024

We could also consider using CodeMirror instead. It looks like creating custom languages may be a bit complex.

@amareshsm
Copy link
Member Author

We could also consider using CodeMirror instead. It looks like creating custom languages may be a bit complex.

There is a way to enable comments in the Monaco Editor by configuring the DiagnosticsOptions.
ref: https://microsoft.github.io/monaco-editor/typedoc/interfaces/languages.json.DiagnosticsOptions.html

@nzakas
Copy link
Member

nzakas commented Aug 16, 2024

Looks like there's a conflict now.

@nzakas
Copy link
Member

nzakas commented Aug 16, 2024

To wrap up this PR, let's go ahead and enable comments when JSONC is selected. That will at least get things functional, then we can figure out a better long-term approach.

Copy link
Member

@harish-sethuraman harish-sethuraman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@amareshsm
Copy link
Member Author

To wrap up this PR, let's go ahead and enable comments when JSONC is selected. That will at least get things functional, then we can figure out a better long-term approach.

I have already made the change.

@amareshsm amareshsm requested a review from nzakas August 16, 2024 18:47
Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@nzakas nzakas merged commit 2f4f998 into main Aug 19, 2024
7 checks passed
@nzakas nzakas deleted the update_code_examples branch August 19, 2024 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change: Code examples and behavior
3 participants