feat(web): add support for language packs #146012

jsjoeio · 2022-03-24T23:39:41Z

Description

This PR adds support for using language packs with VS Code Web via the CLI flag --locale.

Known Limitations

There are a couple limitations with this implementation. Hoping that the Code Team can provide guidance on whether these limitations need to be addressed before merging and if so, how they recommend addressing them.

Implementation only works on a full build

I am not sure why exactly, but this patch only works when you do a full build of VS Code and run that (meaning you can't use this in dev mode).

Requires `commit` in `product.json`

I had to manually add commit to product.json in the build output because of this logic.

Only works with `--locale` CLI flag

Switching the display language only works using the --locale CLI flag. I believe this is because the argv.json is only modified in the client and saved in the IndexedDB.

If we patched this to use the file system instead of userRoamingDataHome, it could potentially work normally (without passing a CLI flag).

Still shows "Install in Browser" which leads to error

Even if it's installed on the remote/server, you still see the "Install in Browser" button. When you click on it, you get an error toast notification. Error comes from this block. Do language packs not have manifests?

Demo

Screen.Recording.2022-03-30.at.9.29.12.AM.mov

How to Test

git clone https://github.com/microsoft/vscode.git
gh pr checkout 146012
yarn
yarn gulp "vscode-reh-web-darwin-x64-min" (replace darwin-x64 with your platform)
cd .. && ./vscode-reh-web-darwin-x64/bin/code-server-oss
Open VS Code in browser
Download an extension pack (i.e. Spanish)
Install from VSIX file
Kill VS Code and run again: ./vscode-reh-web-darwin-x64/bin/code-server-oss --locale es
See display in Spanish

Fixes #82595
Fixes coder/code-server#4703

Contributors

@code-asher - wrote original patch
@jsjoeio - extracted from code-server

jsjoeio · 2022-03-28T22:14:23Z

src/vs/server/node/remoteLanguagePacks.ts

@@ -44,3 +50,39 @@ export namespace InternalNLSConfiguration {
 		return candidate && typeof candidate._languagePackId === 'string';
 	}
 }
+
+export const getLocaleFromConfig = async (argvResource: string): Promise<string> => {


May be able to use this function:

vscode/src/main.js

Line 585 in f3a9245

function getUserDefinedLocale(argvConfig) {

Actually...not 1:1 and can't import from JS file I don't think (well I don't see any other files using this since it's not exported) so going to leave as is.

Similar function/logic as well:

vscode/src/main.js

Lines 233 to 256 in f3a9245

function readArgvConfigSync() {

// Read or create the argv.json config file sync before app('ready')

const argvConfigPath = getArgvConfigPath();

let argvConfig;

try {

argvConfig = JSON.parse(stripComments(fs.readFileSync(argvConfigPath).toString()));

} catch (error) {

if (error && error.code === 'ENOENT') {

createDefaultArgvConfigSync(argvConfigPath);

} else {

console.warn(`Unable to read argv.json configuration file in ${argvConfigPath}, falling back to defaults (${error})`);

}

}

// Fallback to default

if (!argvConfig) {

argvConfig = {

'disable-color-correct-rendering': true // Force pre-Chrome-60 color profile handling (for https://github.com/microsoft/vscode/issues/51791)

};

}

return argvConfig;

}

src/vs/server/node/remoteLanguagePacks.ts

jsjoeio · 2022-03-28T22:35:00Z

I'm running into issues building locally. Any ideas? Does windows-process-tree only get installed on Windows? I'm developing on macOS.

[15:28:58] Error: /Users/jp/dev/oss/vscode/src/vs/workbench/contrib/debug/node/terminals.ts(9,10): '"windows-process-tree"' has no exported member named 'IProcessTreeNode'. Did you mean 'ProcessTreeNode'?
[15:28:58] Error: /Users/jp/dev/oss/vscode/src/vs/platform/terminal/node/windowsShellHelper.ts(135,89): '"/Users/jp/dev/oss/vscode/node_modules/@types/windows-process-tree/index"' has no exported member named 'IProcessTreeNode'. Did you mean 'ProcessTreeNode'?
[15:28:58] Finished compilation with 2 errors after 165738 ms
[15:28:58] Error: /Users/jp/dev/oss/vscode/src/vs/workbench/contrib/debug/node/terminals.ts(9,10): '"windows-process-tree"' has no exported member named 'IProcessTreeNode'. Did you mean 'ProcessTreeNode'?
[15:28:58] Error: /Users/jp/dev/oss/vscode/src/vs/platform/terminal/node/windowsShellHelper.ts(135,89): '"/Users/jp/dev/oss/vscode/node_modules/@types/windows-process-tree/index"' has no exported member named 'IProcessTreeNode'. Did you mean 'ProcessTreeNode'?
[15:28:58] Finished compilation with 2 errors after 165740 ms
[15:28:58] 'vscode-reh-web-darwin-x64-min' errored after 2.85 min
[15:28:58] Error: Found 2 errors
    at Stream.<anonymous> (/Users/jp/dev/oss/vscode/build/lib/reporter.js:91:29)
    at _end (/Users/jp/dev/oss/vscode/node_modules/through/index.js:65:9)
    at Stream.stream.end (/Users/jp/dev/oss/vscode/node_modules/through/index.js:74:5)
    at Stream.onend (internal/streams/legacy.js:47:10)
    at Stream.emit (events.js:412:35)
    at Stream.emit (domain.js:532:15)
    at drain (/Users/jp/dev/oss/vscode/node_modules/through/index.js:34:23)
    at Stream.stream.queue.stream.push (/Users/jp/dev/oss/vscode/node_modules/through/index.js:45:5)
    at Stream.end (/Users/jp/dev/oss/vscode/node_modules/through/index.js:15:35)
    at _end (/Users/jp/dev/oss/vscode/node_modules/through/index.js:65:9)
error Command failed with exit code 1.

TylerLeonhardt · 2022-03-29T01:33:18Z

Looks like the build is in a bad state. Unrelated to your change. Should be fixed tomorrow.

TylerLeonhardt · 2022-03-29T14:33:16Z

@jsjoeio just an FYI I updated your brand from main to help with the CI and local failures.

TylerLeonhardt · 2022-03-29T14:41:33Z

Implementation only works on a full build
Requires commit in product.json

Yes I believe all of this is expected and is the same as Desktop. The commit is used in the path of one of the localization files and also when running out of sources we don’t bundle files (for easier debugging) but bundling is important for the localization feature. @dbaeumer please let me know if I have this wrong.

TylerLeonhardt · 2022-03-29T14:42:38Z

Only works with --locale CLI flag

yeah I’d like to get to the bottom of this before we merge this PR in. I should have some cycles to look into it this week.

jsjoeio · 2022-03-29T16:37:05Z

I should have some cycles to look into it this week.

Awesome! Thank you so much for the quick replies and offering to help us get to the bottom of this! Really appreciate it. Let me know how else I can be helpful.

jsjoeio · 2022-03-30T16:40:29Z

Okay a couple updates:

added demo video
updated testing instructions

I'm not sure how to fix this Hygine error:

$ node build/lib/layersChecker.js
[build/lib/layersChecker.ts]: Reference to 'document' from 'lib.dom.d.ts' violates layer '**/vs/base/common/platform.ts' (/home/runner/work/vscode/vscode/src/vs/base/common/platform.ts ([8](https://github.com/microsoft/vscode/runs/5756751534?check_suite_focus=true#step:11:8)7,20)
[build/lib/layersChecker.ts]: Reference to 'document' from 'lib.dom.d.ts' violates layer '**/vs/base/common/platform.ts' (/home/runner/work/vscode/vscode/src/vs/base/common/platform.ts (87,48)
[build/lib/layersChecker.ts]: Reference to 'getElementById' from 'lib.dom.d.ts' violates layer '**/vs/base/common/platform.ts' (/home/runner/work/vscode/vscode/src/vs/base/common/platform.ts (87,57)
[build/lib/layersChecker.ts]: Reference to 'getAttribute' from 'lib.dom.d.ts' violates layer '**/vs/base/common/platform.ts' (/home/runner/work/vscode/vscode/src/vs/base/common/platform.ts (88,32)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

I see navigator used in the same block where I'm using document.

Maybe there's a way to refactor this code to do it the "proper way":

	const el = typeof document !== 'undefined' && document.getElementById('vscode-remote-nls-configuration');
	const rawNlsConfig = el && el.getAttribute('data-settings');

jsjoeio · 2022-04-12T19:22:07Z

Friendly bump @TylerLeonhardt - do you think you'll have a chance to look at this this month?

TylerLeonhardt · 2022-05-19T18:08:22Z

👋 @jsjoeio so sorry for the delay. Lots of stuff happened (including personally) and I wasn't able to get around to this. I really appreciate all the work you've done to achieve language pack support.

Based on your findings, I'm currently working on solving the known limitations by allowing the client to dictate what the language is so that we can avoid some of things you've pointed out.

I can speak more about that once I actually have something to show off 😅 but essentially the idea is to have navigator.language dictate the locale to use.

jsjoeio · 2022-05-19T20:23:40Z

@TylerLeonhardt no worries at all - hope everything is okay personally! Thanks so much for the update.

I can speak more about that once I actually have something to show off 😅 but essentially the idea is to have navigator.language dictate the locale to use.

That sounds amazing! I'll keep an eye out then for an update from you 😄

jsjoeio · 2022-06-09T22:08:39Z

@TylerLeonhardt now that 1.68 has core localization support, is this still needed?

TylerLeonhardt · 2022-06-09T22:59:29Z

I think we can close this now since we went in a different direction. This wasn't thrown away though, we all learned from the work you did for sure so thank you for taking the time to contribute. It means a lot to the community!

wip: commit message

f509cab

vscode-triage-bot assigned TylerLeonhardt Mar 25, 2022

jsjoeio added 11 commits March 25, 2022 15:05

Merge branch 'main' into 82595-language-patch

e5db20c

Merge branch 'main' into 82595-language-patch

36ee2f9

feat(serverServices): register localizationsChannel

2e08386

feat(platform): get rawNlsConfig in Web Env

28a6ae6

feat(workbench.html): add NLS Configuration

7f53fb9

feat(remoteLangPacks): add logic to get locale from config

6d6bb4b

feat: add locale to webClientServer and serverEnvService

d1f9d25

feat(services/browser): add localizationsService

6e9ebb2

Merge branch 'main' into 82595-language-patch

cd40910

feat(services/browser): add localizationsService

763f219

refactor(remoteLangPacks): use stripComments from common/json

474babb

jsjoeio commented Mar 28, 2022

View reviewed changes

jsjoeio marked this pull request as ready for review March 28, 2022 22:36

Merge branch 'main' into 82595-language-patch

0594a90

TylerLeonhardt requested review from TylerLeonhardt and dbaeumer March 29, 2022 14:35

Merge branch 'main' into 82595-language-patch

981affe

jsjoeio added 3 commits March 29, 2022 10:22

Merge branch 'main' into 82595-language-patch

4d7aa51

Merge branch 'main' into 82595-language-patch

a5519d5

Merge branch 'main' into 82595-language-patch

fbc6e64

Merge branch 'main' into 82595-language-patch

86ada87

Merge branch 'main' into 82595-language-patch

01fddc1

Merge branch 'main' into 82595-language-patch

bba5311

TylerLeonhardt closed this Jun 9, 2022

jsjoeio deleted the 82595-language-patch branch June 10, 2022 15:45

jsjoeio mentioned this pull request Jun 15, 2022

Extract display language patch and submit PR upstream coder/code-server#4703

Closed

github-actions bot locked and limited conversation to collaborators Jul 24, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(web): add support for language packs #146012

feat(web): add support for language packs #146012

jsjoeio commented Mar 24, 2022 •

edited

Loading

jsjoeio Mar 28, 2022

jsjoeio Mar 28, 2022

jsjoeio Mar 28, 2022

jsjoeio commented Mar 28, 2022 •

edited

Loading

TylerLeonhardt commented Mar 29, 2022

TylerLeonhardt commented Mar 29, 2022

TylerLeonhardt commented Mar 29, 2022

TylerLeonhardt commented Mar 29, 2022

jsjoeio commented Mar 29, 2022

jsjoeio commented Mar 30, 2022

jsjoeio commented Apr 12, 2022

TylerLeonhardt commented May 19, 2022

jsjoeio commented May 19, 2022

jsjoeio commented Jun 9, 2022

TylerLeonhardt commented Jun 9, 2022

	function readArgvConfigSync() {

	// Read or create the argv.json config file sync before app('ready')
	const argvConfigPath = getArgvConfigPath();
	let argvConfig;
	try {
	argvConfig = JSON.parse(stripComments(fs.readFileSync(argvConfigPath).toString()));
	} catch (error) {
	if (error && error.code === 'ENOENT') {
	createDefaultArgvConfigSync(argvConfigPath);
	} else {
	console.warn(`Unable to read argv.json configuration file in ${argvConfigPath}, falling back to defaults (${error})`);
	}
	}

	// Fallback to default
	if (!argvConfig) {
	argvConfig = {
	'disable-color-correct-rendering': true // Force pre-Chrome-60 color profile handling (for https://github.com/microsoft/vscode/issues/51791)
	};
	}

	return argvConfig;
	}

feat(web): add support for language packs #146012

feat(web): add support for language packs #146012

Conversation

jsjoeio commented Mar 24, 2022 • edited Loading

Description

Known Limitations

Implementation only works on a full build

Requires commit in product.json

Only works with --locale CLI flag

Still shows "Install in Browser" which leads to error

Demo

How to Test

Contributors

jsjoeio Mar 28, 2022

Choose a reason for hiding this comment

jsjoeio Mar 28, 2022

Choose a reason for hiding this comment

jsjoeio Mar 28, 2022

Choose a reason for hiding this comment

jsjoeio commented Mar 28, 2022 • edited Loading

TylerLeonhardt commented Mar 29, 2022

TylerLeonhardt commented Mar 29, 2022

TylerLeonhardt commented Mar 29, 2022

TylerLeonhardt commented Mar 29, 2022

jsjoeio commented Mar 29, 2022

jsjoeio commented Mar 30, 2022

jsjoeio commented Apr 12, 2022

TylerLeonhardt commented May 19, 2022

jsjoeio commented May 19, 2022

jsjoeio commented Jun 9, 2022

TylerLeonhardt commented Jun 9, 2022

jsjoeio commented Mar 24, 2022 •

edited

Loading

Requires `commit` in `product.json`

Only works with `--locale` CLI flag

jsjoeio commented Mar 28, 2022 •

edited

Loading