-
Notifications
You must be signed in to change notification settings - Fork 29.4k
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
Refactors calls to languages.setLanguageConfiguration to declarative descriptions in language-configuration.json #124015
Conversation
…descriptions in language-configuration.json. This fixes #98621.
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.
As mentioned, I would remove the g
from the wordPatterns in the language-configuration. Other than that. it all looks good!
], | ||
"wordPattern": { | ||
"pattern": "(-?\\d*\\.\\d\\w*)|([^\\`\\~\\!\\@\\$\\^\\&\\*\\(\\)\\=\\+\\[\\{\\]\\}\\\\\\|\\;\\:\\'\\\"\\,\\.\\<\\>\\/\\s]+)", | ||
"flags": "g" |
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.
I believe the 'g' is mandatory for all word patterns. I has to do with the way we use the word pattern regexes.
So I would remove it from the language-configurations and always set it where we create the RegExp object.
So the extension can do "wordPattern": "(-?\\d..." *
(unless i
or u
is needed as well)
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.
Markdown changes look good. Should TypeScript also be updated?
vscode/extensions/typescript-language-features/src/languageFeatures/languageConfiguration.ts
Line 58 in d89e2e1
const jsxTagsLanguageConfiguration: vscode.LanguageConfiguration = { |
I guess yes, but lets do that in another PR, otherwise this becomes a Frankenstein! Also, lets merge this after the May release so there is plenty of time to test these changes in the Insiders. I'm quite sure that my refactoring is correct, but it is very easy to screw up and copy the wrong thing. |
This fixes #98621.
Edit from @alexdima: Since a couple of milestones, it is possible to express everything in the language configuration files, including the
wordPattern
and theonEnterRules
. It is preferable to express the language configuration in JSON because this allows those features to work even when the main extension is not activated.