refactor(formatter/oxfmtrc): Fix up Oxfmtrc and oxfmtrc.rs#16933
Conversation
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. |
CodSpeed Performance ReportMerging #16933 will not alter performanceComparing Summary
Footnotes
|
f3c0a66 to
5ed57f9
Compare
Oxfmtrc and oxfmtrc.rs
There was a problem hiding this comment.
Pull request overview
This PR refactors the Oxfmtrc configuration struct to improve its design and maintainability. The primary goals are to clarify that the struct's current location in oxc_formatter is temporary (due to circular dependencies), remove unnecessary default attributes that are handled elsewhere, align SortImportsConfig fields with the Option-based pattern used throughout Oxfmtrc, and move the from_file() method to its only user (oxc_language_server).
Key Changes:
- Added documentation explaining why
Oxfmtrcis temporarily inoxc_formatterinstead ofapps/oxfmt - Removed
#[default]attributes from config enums since defaults are resolved duringinto_options()conversion - Changed all
SortImportsConfigfields toOptiontypes with improved documentation showing defaults - Moved
from_file()method fromoxfmtrc.rstooxc_language_serveras it's the only consumer - Restructured module exports:
oxfmtrcis now a public module, andparse_utilscontent moved toservice/mod.rs
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
crates/oxc_formatter/src/oxfmtrc.rs |
Added explanatory comments, removed enum #[default] attributes, changed SortImportsConfig to use Option fields, removed from_file() method, refactored into_options() to handle Option unwrapping |
crates/oxc_formatter/src/lib.rs |
Changed oxfmtrc from private module to public, simplified service exports |
crates/oxc_formatter/src/service/mod.rs |
Merged parse_utils.rs content directly into mod.rs |
crates/oxc_formatter/src/service/parse_utils.rs |
File deleted (content moved to mod.rs) |
crates/oxc_formatter/Cargo.toml |
Removed json-strip-comments dependency (moved to language server) |
crates/oxc_language_server/src/formatter/server_formatter.rs |
Added from_file() method, updated imports to new oxfmtrc module path |
crates/oxc_language_server/Cargo.toml |
Added json-strip-comments dependency |
apps/oxfmt/src/core/config.rs |
Updated imports to use new oxfmtrc module path |
| Test files and schemas | Updated imports and regenerated JSON schemas with new descriptions showing default values |
Cargo.lock |
Reflects dependency movement between crates |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5ed57f9 to
5ea9f08
Compare
Merge activity
|
This is a refactoring related to the `Oxfmtrc` struct. - Clearly state that defined in `oxc_formatter` is temporary - Also changed export path - Remove useless `default` attribute from `Oxfmtrc` - Defaults are resolved by `into_options()` - Mark all `SortImportsConfig` fields as `Option` - Aligned with `Oxfmtrc`, needed for Node.js API - Move `from_file()` to `oxc_language_server`, which only used by here
5ea9f08 to
ca28355
Compare
…roject#16933) This is a refactoring related to the `Oxfmtrc` struct. - Clearly state that defined in `oxc_formatter` is temporary - Also changed export path - Remove useless `default` attribute from `Oxfmtrc` - Defaults are resolved by `into_options()` - Mark all `SortImportsConfig` fields as `Option` - Aligned with `Oxfmtrc`, needed for Node.js API - Move `from_file()` to `oxc_language_server`, which only used by here

This is a refactoring related to the
Oxfmtrcstruct.oxc_formatteris temporarydefaultattribute fromOxfmtrcinto_options()SortImportsConfigfields asOptionOxfmtrc, needed for Node.js APIfrom_file()tooxc_language_server, which only used by here