fix(oxfmt/lsp): revert #19977, prefer file extension over languageId for format strategy#20623
Merged
graphite-app[bot] merged 1 commit intomainfrom Mar 23, 2026
Conversation
Member
Author
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
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. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reverts oxfmt’s LSP formatting strategy selection for file:// URIs to align with the CLI by preferring the real file extension over the editor-provided languageId, while continuing to use languageId-based strategy selection for non-file schemes (in-memory documents).
Changes:
- Revert
file://formatting to selectFormatFileStrategystrictly from the on-disk file path/extension. - Remove the LSP test case + fixture/snapshot that validated formatting a
.txtfile as TypeScript vialanguageId. - Simplify LSP module helpers by removing
apply_language_id_extensionand adjusting fake-path generation for in-memory URIs.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| apps/oxfmt/src/lsp/server_formatter.rs | Reverts file:// strategy selection to use only the real path extension; keeps language-id-based behavior for non-file URIs. |
| apps/oxfmt/src/lsp/mod.rs | Removes the helper that replaced extensions and updates fake-path creation used for in-memory formatting. |
| apps/oxfmt/test/lsp/format/format.test.ts | Drops the .ts.txt “format as TS” case from file-based formatting tests. |
| apps/oxfmt/test/lsp/format/fixtures/format/test.ts.txt | Removes the no-longer-needed fixture for the reverted behavior. |
| apps/oxfmt/test/lsp/format/snapshots/format.test.ts.snap | Updates snapshots to remove the reverted .ts.txt expectation. |
leaysgur
approved these changes
Mar 23, 2026
Member
Merge activity
|
…for format strategy (#20623) closes oxc-project/oxc-vscode#140 Preferring the languageID over the real file extension creates inconsistency between the editor and the CLI. Reverting the changes from #19977 back to align back with CLI. This only effects `file://` scheme, where we can expect a real file on a disk. Other schemes are still preferring the languageID, because we do not to what exactly they are relating to.
4ffee91 to
c1b461b
Compare
This was referenced Mar 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes oxc-project/oxc-vscode#140
Preferring the languageID over the real file extension creates inconsistency between the editor and the CLI.
Reverting the changes from #19977 back to align back with CLI.
This only effects
file://scheme, where we can expect a real file on a disk. Other schemes are still preferring the languageID, because we do not to what exactly they are relating to.