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

fix lsp config reload #9415

Merged
merged 1 commit into from
Feb 13, 2024
Merged

fix lsp config reload #9415

merged 1 commit into from
Feb 13, 2024

Conversation

kyfanc
Copy link
Contributor

@kyfanc kyfanc commented Jan 24, 2024

This PR attempts to fix #9398

syn_loader was replaced rather than interior value being replace,
old value was still being referenced and not updated after :config-refresh.

By using ArcSwap similarly to config, each .load() call will return the most updated value.

Testing Steps

  1. Open the helix repo in Helix and open a document.
  2. Add a custom language config for rust-analyzer in languages.toml. 
      [language-server.rust-analyzer.config]
  cargo.features = "all" 
  rust-analyzer.check.command = "clippy"
+ rust-analyzer.inlayHints.typeHints.enable= false
+ rust-analyzer.inlayHints.parameterHints.enable= false
+ rust-analyzer.inlayHints.chainingHints.enable= false
  3. :config-reload and :lsp-restart
  4. page-up/down to trigger re-rendering
  5. inlay hints should now be disabled, lsp config is reloaded

PS. This is my first PR, please let me know if there is anything I missed/ overlooked.

@kirawi kirawi added C-bug Category: This is a bug A-helix-term Area: Helix term improvements labels Jan 24, 2024
kirawi
kirawi approved these changes Jan 28, 2024
View reviewed changes
Copy link
Member

@kirawi kirawi left a comment

Choose a reason for hiding this comment

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

I don't see anything obviously incorrect here; it's pretty straightforward.

@kirawi kirawi added the S-waiting-on-review Status: Awaiting review from a maintainer. label Feb 7, 2024
the-mikedavis
the-mikedavis previously approved these changes Feb 10, 2024
View reviewed changes
pascalkuthe
pascalkuthe previously approved these changes Feb 12, 2024
View reviewed changes
@pascalkuthe
Copy link
Member

There are some small conflicts with some other PRs that were merged once those are resolved this lgtm.

fix lsp config reload
6c03f77 
`syn_loader` was replaced rather than interior value being replace,
old value was still being referenced and not updated after `:config-refresh`.
By using `ArcSwap` like for `config`, each `.load()` call will return the most
updated value.
@pascalkuthe pascalkuthe merged commit fe869e5 into helix-editor:master Feb 13, 2024
6 checks passed
cosmikwolf pushed a commit to cosmikwolf/helix that referenced this pull request Feb 26, 2024
@kyfanc @cosmikwolf
fix lsp config reload (helix-editor#9415)
150b567 
`syn_loader` was replaced rather than interior value being replace,
old value was still being referenced and not updated after `:config-refresh`.
By using `ArcSwap` like for `config`, each `.load()` call will return the most
updated value.

Co-authored-by: kyfan <kyfan@email>
postsolar pushed a commit to postsolar/helix that referenced this pull request Apr 4, 2024
@kyfanc @postsolar
fix lsp config reload (helix-editor#9415)
c4ae70e 
`syn_loader` was replaced rather than interior value being replace,
old value was still being referenced and not updated after `:config-refresh`.
By using `ArcSwap` like for `config`, each `.load()` call will return the most
updated value.

Co-authored-by: kyfan <kyfan@email>
mtoohey31 pushed a commit to mtoohey31/helix that referenced this pull request Jun 2, 2024
@kyfanc @mtoohey31
fix lsp config reload (helix-editor#9415)
7ac63c4 
`syn_loader` was replaced rather than interior value being replace,
old value was still being referenced and not updated after `:config-refresh`.
By using `ArcSwap` like for `config`, each `.load()` call will return the most
updated value.

Co-authored-by: kyfan <kyfan@email>
Vulpesx pushed a commit to Vulpesx/helix that referenced this pull request Jun 7, 2024
@kyfanc @Vulpesx
fix lsp config reload (helix-editor#9415)
55b518a 
`syn_loader` was replaced rather than interior value being replace,
old value was still being referenced and not updated after `:config-refresh`.
By using `ArcSwap` like for `config`, each `.load()` call will return the most
updated value.

Co-authored-by: kyfan <kyfan@email>
smortime pushed a commit to smortime/helix that referenced this pull request Jul 10, 2024
@kyfanc
fix lsp config reload (helix-editor#9415)
7f9fbf6 
`syn_loader` was replaced rather than interior value being replace,
old value was still being referenced and not updated after `:config-refresh`.
By using `ArcSwap` like for `config`, each `.load()` call will return the most
updated value.

Co-authored-by: kyfan <kyfan@email>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@the-mikedavis the-mikedavis the-mikedavis approved these changes

@pascalkuthe pascalkuthe pascalkuthe approved these changes

@kirawi kirawi kirawi approved these changes

Assignees
No one assigned
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Language server config is not updated for :lsp-restart after :config-reload
4 participants
@kyfanc @pascalkuthe @the-mikedavis @kirawi