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.

Sign up for GitHub

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

Jump to bottom

Random regressions due to lsp overriding user settings via lsp-register-custom-settings #4420

Open
3 tasks done
Hi-Angel opened this issue Apr 8, 2024 · 1 comment · May be fixed by #4421
Open
3 tasks done

Random regressions due to lsp overriding user settings via lsp-register-custom-settings #4420

Hi-Angel opened this issue Apr 8, 2024 · 1 comment · May be fixed by #4421
Labels
bug

Comments

@Hi-Angel
Copy link
Contributor

Hi-Angel commented Apr 8, 2024

Thank you for the bug report

  • I am using the latest version of lsp-mode related packages.
  • I checked FAQ and Troubleshooting sections
  • You may also try reproduce the issue using clean environment using the following command: M-x lsp-start-plain

Bug description

Commit 782e1dc by @LaurenceWarne introduced a regression and broke mypy completely for me. That happened, because I had set the same lsp-server settings that the commit modified.

Further research shown that there's nothing wrong with the commit per se, instead the problem is in how lsp-mode handles lsp-server settings. Whatever user sets with lsp-register-custom-settings is being overwritten by lsp-mode, leading to random problems like this one.

Instead lsp-register-custom-settings should not overwrite settings that a user has set.

Steps to reproduce

  1. Create /tmp/.emacs with the following content: 
    (package-initialize)
(use-package lsp-mode
  :config
  (setq lsp-log-io t)
  (lsp-register-custom-settings '(("pylsp.plugins.pylsp_mypy.enabled" t t))))
  2. Start emacs as emacs -Q -l /tmp/.emacs test.py
  3. Start lsp-mode in the test.py buffer by evaluating M-x lsp
  4. Switch to buffer with communication log by evaluating M-x lsp-workspace-show-log
  5. Search for word pylsp_mypy and look at the value of enabled just below the pylsp_mypy line

Expected behavior

It is true.

Which Language Server did you use?

pylsp

OS

Linux

Error callstack

No response

Anything else?

No response
@Hi-Angel Hi-Angel added the bug label Apr 8, 2024
Hi-Angel added a commit to Hi-Angel/lsp-mode that referenced this issue Apr 8, 2024
@Hi-Angel
treewide: do not overwrite lsp-client-settings changed by user
9f94a48 
If a user has set a setting inside `lsp-client-settings`, changing its
value will result in a silent bug in user configuration, that looks
like a regression from `lsp-mode` mode update. Let's avoid that by
adding a new function `lsp-register-new-settings` that avoids
overwritting whatever was set by user and make use of it treewide.

While at it, prohibit `lsp-register-custom-settings` from internal use
for all the same reasons.

Fixes: emacs-lsp#4420
@Hi-Angel Hi-Angel linked a pull request Apr 8, 2024 that will close this issue
Open
@Hi-Angel
Copy link
Contributor Author

Hi-Angel commented Apr 8, 2024

Should be fixed by #4421

Hi-Angel added a commit to Hi-Angel/lsp-mode that referenced this issue Apr 8, 2024
@Hi-Angel
treewide: do not overwrite lsp-client-settings changed by user
42b0e44 
If a user has set a setting inside `lsp-client-settings`, changing its
value will result in a silent bug in user configuration, that looks
like a regression from `lsp-mode` mode update. Let's avoid that by
adding a new function `lsp-register-new-settings` that avoids
overwritting whatever was set by user and make use of it treewide.

While at it, prohibit `lsp-register-custom-settings` from internal use
for all the same reasons.

Fixes: emacs-lsp#4420
Hi-Angel added a commit to Hi-Angel/lsp-mode that referenced this issue Apr 8, 2024
@Hi-Angel
treewide: do not overwrite lsp-client-settings changed by user
9c0905c 
If a user has set a setting inside `lsp-client-settings`, changing its
value will result in a silent bug in user configuration that looks
like a regression from `lsp-mode` mode update. Let's avoid that by
adding a new function `lsp-register-new-settings` that avoids
overwritting whatever was set by user and make use of it treewide.

While at it, prohibit `lsp-register-custom-settings` from internal use
for all the same reasons.

Fixes: emacs-lsp#4420
Hi-Angel added a commit to Hi-Angel/lsp-mode that referenced this issue Apr 8, 2024
@Hi-Angel
treewide: do not overwrite lsp-client-settings changed by user
3df350a 
If a user has set a setting inside `lsp-client-settings`, changing its
value will result in a silent bug in user configuration that looks
like a regression from `lsp-mode` mode update. Let's avoid that by
adding a new function `lsp-register-new-settings` that avoids
overwritting whatever was set by user and make use of it treewide.

While at it, prohibit `lsp-register-custom-settings` from internal use
for all the same reasons.

Fixes: emacs-lsp#4420
Hi-Angel added a commit to Hi-Angel/lsp-mode that referenced this issue Apr 8, 2024
@Hi-Angel
treewide: do not overwrite lsp-client-settings changed by user
42fdebe 
If a user has set a setting inside `lsp-client-settings`, changing its
value will result in a silent bug in user configuration that looks
like a regression from `lsp-mode` mode update. Let's avoid that by
adding a new function `lsp-register-new-settings` that avoids
overwritting whatever was set by user and make use of it treewide.

While at it, prohibit `lsp-register-custom-settings` from internal use
for all the same reasons.

Fixes: emacs-lsp#4420
Hi-Angel added a commit to Hi-Angel/lsp-mode that referenced this issue Apr 8, 2024
@Hi-Angel
treewide: do not overwrite lsp-client-settings changed by user
c048a7f 
If a user has set a setting inside `lsp-client-settings`, changing its
value will result in a silent bug in user configuration that looks
like a regression from `lsp-mode` mode update. Let's avoid that by
adding a new function `lsp-register-new-settings` that avoids
overwritting whatever was set by user and make use of it treewide.

While at it, prohibit `lsp-register-custom-settings` from internal use
for all the same reasons.

Fixes: emacs-lsp#4420
Hi-Angel added a commit to Hi-Angel/lsp-mode that referenced this issue Apr 8, 2024
@Hi-Angel
treewide: do not overwrite lsp-client-settings changed by user
03d89dd 
If a user has set a setting inside `lsp-client-settings`, changing its
value will result in a silent bug in user configuration that looks
like a regression from `lsp-mode` mode update. Let's avoid that by
adding a new function `lsp-register-new-settings` that avoids
overwritting whatever was set by user and make use of it treewide.

While at it, prohibit `lsp-register-custom-settings` from internal use
for all the same reasons.

Fixes: emacs-lsp#4420
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

treewide: do not overwrite lsp-client-settings changed by user Hi-Angel/lsp-mode
1 participant
@Hi-Angel