Skip to content

Patches 1.70 #5422

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

Merged
merged 20 commits into from
Aug 17, 2022
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions patches/display-language.diff
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ We can remove this once upstream supports all language packs.
5. Add the locale flag.
6. Remove the redundant locale verification. It does the same as the existing
one but is worse because it does not handle non-existent or empty files.
7. Replace some caching and Node requires because code-server does not restart
when changing the language unlike native Code.

Index: code-server/lib/vscode/src/vs/server/node/serverServices.ts
===================================================================
Expand Down Expand Up @@ -312,3 +314,19 @@ Index: code-server/lib/vscode/src/vs/workbench/contrib/localization/electron-san
await this.jsonEditingService.write(this.environmentService.argvResource, [{ path: ['locale'], value: locale }], true);
return true;
}
Index: code-server/lib/vscode/src/vs/base/node/languagePacks.js
===================================================================
--- code-server.orig/lib/vscode/src/vs/base/node/languagePacks.js
+++ code-server/lib/vscode/src/vs/base/node/languagePacks.js
@@ -73,7 +73,10 @@
function getLanguagePackConfigurations(userDataPath) {
const configFile = path.join(userDataPath, 'languagepacks.json');
try {
- return nodeRequire(configFile);
+ // This must not use Node's require otherwise it will be cached forever.
+ // Code can get away with this since the process actually restarts but
+ // that is not currently the case with code-server.
+ return JSON.parse(fs.readFileSync(configFile, "utf8"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice fix! How did you figure this out?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually fixed this before but it was lost when we switched to the fork so basically I was stepping through the code until suddenly I remembered this issue.

The first time around it just involved stepping through the code one step at time to figure out why the language object was empty until I arrived at this line and realized it would never update since requires are cached. I am not sure where that knowledge came from, probably read it somewhere or maybe it was from when we implemented tarfs which hijacks require.

} catch (err) {
// Do nothing. If we can't read the file we have no
// language pack config.