Fix loading translations when running on web #1165
Conversation
Signed-off-by: David Thompson <[email protected]>
Translations weren't working at all when running the language server in a web worker. In fact, the server was crashing immediately, since the server was trying to load them from the filesystem. This PR provides the translations to the server using the webworker message passing mechanism. Requires redhat-developer/yaml-language-server#1165 Signed-off-by: David Thompson <[email protected]>
datho7561
force-pushed
the
fix-l10n-on-web
branch
from
b6426e9 to
f191760
Compare
On the client side, in VS Code's l10n library, it doesn't provide the translation bundle if the language is set to English. This change makes the text in the calls to `l10n.t` the English text, so that the English text appears if no translations are loaded. Signed-off-by: David Thompson <[email protected]>
The localization tests need to initialize the language server with the correct translation loading function. Signed-off-by: David Thompson <[email protected]>
datho7561
force-pushed
the
fix-l10n-on-web
branch
from
f191760 to
dee8097
Compare
There was a problem hiding this comment.
I reviewed the code and the implementation makes sense to me overall. I noticed a few minor typos/redundancies, plus one question about how the worker bootstraps.
I didn't test this manually though since I don’t really use GitLab and I’m not sure how to test this there, but I’m happy to test it if you want me to / can share a quick set of steps.
| import { InitializeParams } from 'vscode-languageserver'; | ||
|
|
||
| /** | ||
| * Loads translations from the filesystem based on the configured locale and the folder of translations provided in hte initilaization parameters. |
There was a problem hiding this comment.
| * Loads translations from the filesystem based on the configured locale and the folder of translations provided in hte initilaization parameters. | |
| * Loads translations from the filesystem based on the configured locale and the folder of translations provided in the initialization parameters. |
Just a typo
| Diagnostic.create( | ||
| this.getRangeOf(document, node.srcToken), | ||
| l10n.t('flowStyleMapForbidden', 'Flow style mapping is forbidden'), | ||
| l10n.t('Flow style mapping is forbidden', 'Flow style mapping is forbidden'), |
There was a problem hiding this comment.
| l10n.t('Flow style mapping is forbidden', 'Flow style mapping is forbidden'), | |
| l10n.t('Flow style mapping is forbidden'), |
| results.push( | ||
| CodeAction.create( | ||
| l10n.t('convertToBlockStyle', 'Convert to block style {0}', blockTypeDescription), | ||
| l10n.t('Convert to block style {0}', 'Convert to block style {0}', blockTypeDescription), |
There was a problem hiding this comment.
| l10n.t('Convert to block style {0}', 'Convert to block style {0}', blockTypeDescription), | |
| l10n.t('Convert to block style {0}', blockTypeDescription), |
| readFile: (fsPath: string) => { | ||
| return connection.sendRequest(FSReadFile.type, fsPath); | ||
| }, | ||
| self.onmessage = (e) => { |
There was a problem hiding this comment.
Here the worker bootstraps the server inside self.onmessage and immediately calls l10n.config({ contents: e.data.l10nBundle }). This seems to assume the first message received contains l10nBundle, but I’m not sure if that’s always guaranteed? It looks true with the implementation here, but I just wanted to confirm I’m understanding this correctly.
Because, in my understanding, if the first message was instead another message like an LSP initialize message, then e.data.l10nBundle would be undefined and we could bootstrap with the wrong payload? and in that case, translations might just never be configured since after startup the JSON-RPC reader takes over ownership of onmessage and the bootstrap handler will not run again?
3 participants
What does this PR do?
l10n.t(). VS Code's tools to load translations don't load translations if the language is English. This could also be helpful if loading the English translations fails.What issues does this PR fix or reference?
Fixes redhat-developer/vscode-yaml#1191
Is it tested? How?
Manually. I'm hoping to write some smoke tests for running as a web worker in vscode-yaml.