-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Expose JSON worker #4299
Expose JSON worker #4299
Conversation
9821483
to
ab02ed3
Compare
@microsoft-github-policy-service agree |
@@ -4,7 +4,8 @@ | |||
*--------------------------------------------------------------------------------------------*/ | |||
|
|||
import * as mode from './jsonMode'; | |||
import { Emitter, IEvent, languages } from '../../fillers/monaco-editor-core'; | |||
import { Emitter, IEvent, languages, Uri } from 'monaco-editor-core'; | |||
import * as jsonService from 'vscode-json-languageservice'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The types that are used from jsonService
should be copied to json/monaco.contribution.ts
so that the d.ts file is self-contained.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, lemme fix it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though, I wonder then why we have pretty much the same setup in https://github.com/microsoft/monaco-editor/blob/main/src/language/typescript/monaco.contribution.ts#L6 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, sorry, it's jsonMode
not jsonService
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aeschli looks crazy, but I'm done, sir :)
28cb6ca
to
a8d1f30
Compare
Yes, that's a lot of types, I don't like it either. Especially as these types duplicate some of the Monaco types (e.g. IPosition, IRange), but Monaco types 1 based (the first column and line is 1, not 0), while the LSP types are 0 based. Not sure what approach to choose:
@alexdima What do yo think? |
Think, I could leave it to only |
a8d1f30
to
eb704f3
Compare
@aeschli cleaned up a lil bit |
Looks good! |
@aeschli thx for the review, when shall this end up in |
I think @hediet makes a Monaco editor release in the same week VS Code is released. So this should be in the last week of January. |
This PR fixes #727 and addresses review comments (I think so) from an original PR #3956 which became stale for some reason.
I just want to get things JSON worker faster to Monaco, because it's needed for my project.
My use case is to provide insights to the end user after analyzing JSON schemas of the edited document. Plus, include the motivation from an original PR: