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

refactor(language_server)!: remove enable configuration, the client should shutdown the server instead #9990

Conversation

Sysix
Copy link
Collaborator

@Sysix Sysix commented Mar 23, 2025

No description provided.

@github-actions github-actions bot added A-editor Area - Editor and Language Server C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior labels Mar 23, 2025
Copy link
Collaborator Author

Sysix commented Mar 23, 2025


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@Sysix Sysix force-pushed the 03-23-refactor_language_server_remove_enabled_configuration_the_client_should_shutdown_the_server_instead branch from 2b049fe to a71faa5 Compare March 23, 2025 17:40
Copy link

codspeed-hq bot commented Mar 23, 2025

CodSpeed Instrumentation Performance Report

Merging #9990 will not alter performance

Comparing 03-23-refactor_language_server_remove_enabled_configuration_the_client_should_shutdown_the_server_instead (da6336c) with main (45015ee)

Summary

✅ 33 untouched benchmarks

@Sysix Sysix force-pushed the 03-23-refactor_language_server_remove_enabled_configuration_the_client_should_shutdown_the_server_instead branch from a71faa5 to 2c4031a Compare March 23, 2025 17:49
Copy link
Collaborator

@nrayburn-tech nrayburn-tech left a comment

Choose a reason for hiding this comment

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

Went ahead and looked at this even though it's draft, I'm not sure when I would have a chance to get back to it.

Left a few comments on things you might want to review, but nothing big.

@Sysix Sysix force-pushed the 03-23-refactor_language_server_remove_enabled_configuration_the_client_should_shutdown_the_server_instead branch from 2c4031a to 385d773 Compare March 25, 2025 19:21
@Sysix Sysix changed the title refactor(language_server)!: remove enabled configuration, the client should shutdown the server instead refactor(language_server)!: remove enable configuration, the client should shutdown the server instead Mar 25, 2025
@Sysix Sysix marked this pull request as ready for review March 25, 2025 19:29
@Sysix Sysix force-pushed the 03-23-refactor_language_server_remove_enabled_configuration_the_client_should_shutdown_the_server_instead branch 2 times, most recently from ee7d25f to dfa05ef Compare March 25, 2025 20:00
@Sysix Sysix changed the base branch from main to graphite-base/9990 March 25, 2025 21:20
@Sysix Sysix force-pushed the 03-23-refactor_language_server_remove_enabled_configuration_the_client_should_shutdown_the_server_instead branch from dfa05ef to d5bb285 Compare March 25, 2025 21:20
@Sysix Sysix changed the base branch from graphite-base/9990 to 03-25-test_editor_add_test_for_oxc.fixall_command March 25, 2025 21:20
@graphite-app graphite-app bot force-pushed the 03-25-test_editor_add_test_for_oxc.fixall_command branch 2 times, most recently from 0074f73 to 4c77b34 Compare March 26, 2025 01:55
@graphite-app graphite-app bot force-pushed the 03-23-refactor_language_server_remove_enabled_configuration_the_client_should_shutdown_the_server_instead branch from d5bb285 to 34814c8 Compare March 26, 2025 01:55
Copy link
Member

@camchenry camchenry left a comment

Choose a reason for hiding this comment

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

Thanks for working on this and updating the docs a bit. I'm still learning the language server protocol, so I appreciate any extra comments.

@camchenry camchenry added the 0-merge Merge with Graphite Merge Queue label Mar 26, 2025
Copy link
Member

camchenry commented Mar 26, 2025

Merge activity

  • Mar 25, 9:56 PM EDT: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Mar 26, 1:35 PM EDT: camchenry added this pull request to the Graphite merge queue.
  • Mar 26, 1:43 PM EDT: A user merged this pull request with the Graphite merge queue.

@graphite-app graphite-app bot force-pushed the 03-25-test_editor_add_test_for_oxc.fixall_command branch from 4c77b34 to 500add0 Compare March 26, 2025 17:35
@graphite-app graphite-app bot force-pushed the 03-23-refactor_language_server_remove_enabled_configuration_the_client_should_shutdown_the_server_instead branch from 34814c8 to da6336c Compare March 26, 2025 17:36
Base automatically changed from 03-25-test_editor_add_test_for_oxc.fixall_command to main March 26, 2025 17:41
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Mar 26, 2025
@graphite-app graphite-app bot merged commit da6336c into main Mar 26, 2025
28 checks passed
@graphite-app graphite-app bot deleted the 03-23-refactor_language_server_remove_enabled_configuration_the_client_should_shutdown_the_server_instead branch March 26, 2025 17:43
Boshen added a commit that referenced this pull request Apr 1, 2025
## [0.16.4] - 2025-04-01

- da6336c language_server: [**BREAKING**] Remove `enable` configuration,
the client should shutdown the server instead (#9990) (Sysix)

### Features

- 06e3db9 linter: Support `multipleFileExtensions` option for
`unicorn/filename-case` (#10118) (shulaoda)
- dbe0e46 linter: Support `ignore` option for `unicorn/filename-case`
(#10107) (shulaoda)

### Bug Fixes

- 0a33e27 editor: Update `initializationOptions` for a possible restart
(#10121) (Sysix)
- ac780a2 editor: Repair filewatchers when no custom config provided
(#10104) (Sysix)
- 4303ace editor: Dont send `didChangeConfiguration` request to the
server when it is shutdown (#10084) (Sysix)
- 07f2a25 editor: Enable regex parsing in language server (#10035)
(camchenry)
- fcf7702 language_server: Start from a default oxlint configuration +
SafeFix for nested configuration (#10043) (Sysix)
- aba3654 linter: Span disable directive correctly on next line (#10141)
(Ulrich Stark 🦀)
- 2c80858 linter: Enable multi-file analysis for nested configs (#10089)
(camchenry)

### Performance

- 566be59 linter: Replace `phf_set` with `array` in `eslint/func-names`
(#10119) (shulaoda)
- 5e14fe9 linter: Inline `PRE_DEFINE_VAR` and use `array` format
(#10079) (shulaoda)

### Refactor

- 327be53 editor: `LanguageClient` can be undefined (#10112) (Sysix)
- 5ec477c editor: Make `onConfigChange` async (#10110) (Sysix)
- a278d73 editor: Use warning background when the plugin is deactived
(#10085) (Sysix)
- c0e5251 language_server: Set `null` as a default value for
`configPath` (#10047) (Sysix)
- 09c0ac6 linter: Improve `unicorn/filename-case` (#10117) (shulaoda)
- d8e49a1 linter: Compute lintable extensions at compile time (#10090)
(camchenry)
- b3ec235 linter: Use items of `oxc_ast::ast` module directly (#10100)
(Ulrich Stark 🦀)
- 93e6c0b linter: Use `FormalParameter::has_modifier` to detect
parameter properties (#10097) (Ulrich Stark 🦀)
- 5d829c2 semantic: Align handling of declaring symbol for function with
TypeScript (#10086) (Dunqing)

### Testing

- 410b8d6 editor: Use debug build of the language server (#10083)
(Sysix)
- 500add0 editor: Add test for `oxc.fixAll` command (#10045) (Sysix)

Co-authored-by: Boshen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-editor Area - Editor and Language Server C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants