Skip to content
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

~/.local/share/code-server/User/settings.json is not respected in code-server v4.0.0 #4609

Closed
benz0li opened this issue Dec 11, 2021 · 6 comments · Fixed by #4631
Closed
Assignees
Labels
bug Something isn't working
Milestone

Comments

@benz0li
Copy link
Contributor

benz0li commented Dec 11, 2021

OS/Web Information

  • Web Browser: Firefox 95.0 (64-bit)
  • Local OS: macOS 10.14.6 (Mojave)
  • Remote OS: Debian bullseye
  • Remote Architecture: 64-bit
  • code-server --version: 4.0.0 0f39595

Steps to Reproduce

Log in at https://vscode-r.jupyter.b-data.ch.

Expected

The following settings should be applied:

~/.local/share/code-server/User/settings.json

{
  "editor.tabSize": 2,
  "telemetry.enableTelemetry": false,
  "gitlens.advanced.telemetry.enabled": false,
  "r.bracketedPaste": true,
  "r.plot.useHttpgd": true,
  "r.rterm.linux": "/usr/local/bin/radian",
  "r.rterm.option": [],
  "r.workspaceViewer.showObjectSize": true,
  "workbench.colorTheme": "Default Dark+",
  "terminal.integrated.fontFamily": "MesloLGS NF",
  "r.sessionWatcher": true
}

Actual

Settings are not applied.
ℹ️ It seems v4.0.0 is using the browser storage instead of the file system to read/set settings.

Logs

On demand only.
👉 Issue may be reproduced any time by @code-asher, @jsjoeio or @bpmct at https://vscode-r.jupyter.b-data.ch.

Screenshot

None.

Notes

See #2274 for an identical issue with previous versions.

This issue can be reproduced in VS Code: No

@benz0li
Copy link
Contributor Author

benz0li commented Dec 11, 2021

I don't mind that the state is stored in the browser's storage (#4212).

But one should have the possibility to pre-populate ~/.local/share/code-server/User/settings.json with default settings (#2274 (comment)), e.g. when creating a docker image.

@w0otness
Copy link

On a similar note, "bind-addr:" in ~/.config/code-server/config.yaml is not respected either.

All other default options are still respected though.

@jsjoeio jsjoeio added the needs-investigation This issue needs to be further investigated label Dec 13, 2021
@jsjoeio jsjoeio modified the milestones: 4.0.1, 4.0.0 Dec 13, 2021
@code-asher code-asher self-assigned this Dec 13, 2021
@code-asher
Copy link
Member

code-asher commented Dec 13, 2021

Might be another patch that was not ported over when we hard reset to upstream's new server. Hopefully I can just apply the old patch and it will work. This is probably why #2306 has cropped up again too.

I think we may want to enforce e2e tests for various patches we make to VS Code since otherwise we tend to lose them when major refactors due to upstream changes like this happen. Individual patch files instead of a fork would also make the changes more obvious which might be worth considering.

@jsjoeio
Copy link
Contributor

jsjoeio commented Dec 13, 2021

Individual patch files instead of a fork would also make the changes more obvious which might be worth considering.

I was thinking about this too. Could we combine the workflows somehow? I.e. keep the fork (so we can submit upstream) and then maintain patches in that repo?

@code-asher
Copy link
Member

Could definitely do. One advantage to keeping patches here is that we would not need to make double PRs for every fix.

code-asher added a commit to coder/vscode that referenced this issue Dec 14, 2021
This is another patch we lost although I have implemented it in a
slightly different way (it used to use the payload but I changed it to
match the method we currently use to pass data to the frontend although
maybe we should have used the payload for all that).

coder/code-server#4609
@jsjoeio
Copy link
Contributor

jsjoeio commented Dec 14, 2021

Let's plan to have a discussion offline in January and then I'll share notes in a Discussion afterwards with the community.

@code-asher code-asher added bug Something isn't working and removed needs-investigation This issue needs to be further investigated labels Dec 14, 2021
code-asher added a commit to code-asher/code-server that referenced this issue Dec 15, 2021
Fixes coder#3410
Fixes coder#4604
Fixes coder#4607
Fixes coder#4608
Fixes coder#4609

Also has the foundation for
coder#4619.
jsjoeio pushed a commit that referenced this issue Dec 15, 2021
Fixes #3410
Fixes #4604
Fixes #4607
Fixes #4608
Fixes #4609

Also has the foundation for
#4619.
ZauberNerd pushed a commit to ZauberNerd/vscode that referenced this issue Dec 23, 2021
This is another patch we lost although I have implemented it in a
slightly different way (it used to use the payload but I changed it to
match the method we currently use to pass data to the frontend although
maybe we should have used the payload for all that).

coder/code-server#4609
ZauberNerd pushed a commit to ZauberNerd/vscode that referenced this issue Dec 23, 2021
This is another patch we lost although I have implemented it in a
slightly different way (it used to use the payload but I changed it to
match the method we currently use to pass data to the frontend although
maybe we should have used the payload for all that).

coder/code-server#4609
ZauberNerd pushed a commit to ZauberNerd/vscode that referenced this issue Dec 23, 2021
This is another patch we lost although I have implemented it in a
slightly different way (it used to use the payload but I changed it to
match the method we currently use to pass data to the frontend although
maybe we should have used the payload for all that).

coder/code-server#4609
benz0li referenced this issue Nov 21, 2023
The main goal of this patch was to make user settings stored on disk
instead of in the browser, but this stopped working some time ago.  Not
only that but it is causing a bug where a workspace will not fully open.

A secondary goal was to fix the Vim extension but the extension appears
to work just fine without this change now (both the server and browser
versions).

This patch is not useful anymore anyway because there are remote-level
settings that *do* get stored on disk and can be used instead of
user-level settings when necessary.

Fixes #3061, and possibly #6153.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants