-
Notifications
You must be signed in to change notification settings - Fork 16
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
Prepare extension for localization #15
Conversation
messageFormat: nls.MessageFormat.file | ||
}; | ||
|
||
// TODO: Should we use VSCODE_NLS_CONFIG? |
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 think VSCODE_NLS_CONFIG
gets automatically consumed by nls.config()
? Should double check.
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.
Yeah, the comment was related to other extensions which seem to pass process.env.VSCODE_NLS_CONFIG
directly to the config()
method, which seems redundant given that vscode-nls
appears to handle that environment variable internally. Personally, I like the ability to configure just our extension as having a specific locale (e.g. pseudo), without having the entire product use that locale.
Until we actually have translations for another language I'm not 100% sure the pipeline works from start to end-packaged-VSIX. That said, it's likely to require, at most, a couple of minor tweaks (which was the point of this PR).
Another question will be how the localization pipeline integrates the with an upcoming bundling change (to use webpack
).
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.
Some comments but looks good!
Prepares the extension for eventual localization:
vscode-nls
)localize()
function for strings andpackage.json
localization mechanismsvscode-nls-dev
)Resolves #14.