feat(formatter): Add a warning when Oxfmt is used without a configuration file.#19904
feat(formatter): Add a warning when Oxfmt is used without a configuration file.#19904cpojer wants to merge 1 commit intooxc-project:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a user-facing note when oxfmt runs without any explicit or auto-discovered configuration, mirroring the UX added to oxlint in the referenced PR.
Changes:
- Print a default-configuration note to
stderrwhen no--config, no.oxfmtrc.*, and no.editorconfigare found. - Introduce
uses_default_config(...)helper and re-export it viacore. - Add unit tests for the new helper.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/oxfmt/src/stdin/mod.rs | Emits the default-config note in stdin mode when no config sources exist. |
| apps/oxfmt/src/core/mod.rs | Re-exports the new uses_default_config helper. |
| apps/oxfmt/src/core/config.rs | Adds uses_default_config helper and unit tests. |
| apps/oxfmt/src/cli/mod.rs | Adds shared DEFAULT_CONFIG_NOTE_MESSAGE constant. |
| apps/oxfmt/src/cli/format.rs | Emits the default-config note in normal CLI formatting flow. |
Comments suppressed due to low confidence (1)
apps/oxfmt/src/core/config.rs:540
- Test modules in this crate are consistently named
tests(ortests_*), but this new module is namedtest. Renaming it totestswould match the established convention and make it easier to search for test modules.
#[cfg(test)]
mod test {
use std::path::Path;
| if uses_default_config( | ||
| config_options.config.as_deref(), | ||
| oxfmtrc_path.as_deref(), | ||
| editorconfig_path.as_deref(), | ||
| ) { | ||
| utils::print_and_flush(stderr, DEFAULT_CONFIG_NOTE_MESSAGE); | ||
| } |
There was a problem hiding this comment.
This new default-config note changes stderr output for runs without any .oxfmtrc/.editorconfig, which will likely break existing Vitest CLI snapshot tests (many fixtures don't include config files). Please update/add CLI snapshot coverage to assert the note is printed (and update existing snapshots that now have non-empty stderr).
| if uses_default_config( | ||
| config_options.config.as_deref(), | ||
| oxfmtrc_path.as_deref(), | ||
| editorconfig_path.as_deref(), | ||
| ) { | ||
| utils::print_and_flush(stderr, DEFAULT_CONFIG_NOTE_MESSAGE); | ||
| } |
There was a problem hiding this comment.
This new default-config note will change stderr output for --stdin-filepath runs when no .oxfmtrc/.editorconfig exists, which may break existing stdin-related snapshot tests. Please update/add coverage so tests assert the note is printed in the no-config case (and that it is not printed when config files are present).
|
|
||
| pub(crate) const DEFAULT_CONFIG_NOTE_MESSAGE: &str = | ||
| "Using Oxfmt's default configuration. Run `oxfmt --init` to set up Oxfmt in this project.\n"; | ||
|
|
There was a problem hiding this comment.
DEFAULT_CONFIG_NOTE_MESSAGE advises running oxfmt --init, but --init is only available when the napi feature is enabled (see cli/command.rs). When building without default features (--no-default-features), this message will instruct users to run a flag that doesn't exist. Consider making the message conditional on cfg(feature = "napi"), or adjust the text for non-napi builds (e.g., point to creating .oxfmtrc.json manually).
| pub(crate) const DEFAULT_CONFIG_NOTE_MESSAGE: &str = | |
| "Using Oxfmt's default configuration. Run `oxfmt --init` to set up Oxfmt in this project.\n"; | |
| #[cfg(feature = "napi")] | |
| pub(crate) const DEFAULT_CONFIG_NOTE_MESSAGE: &str = | |
| "Using Oxfmt's default configuration. Run `oxfmt --init` to set up Oxfmt in this project.\n"; | |
| #[cfg(not(feature = "napi"))] | |
| pub(crate) const DEFAULT_CONFIG_NOTE_MESSAGE: &str = | |
| "Using Oxfmt's default configuration. Create a `.oxfmtrc.json` file to configure Oxfmt for this project.\n"; |
|
Thanks for getting things started! However, since we intend to handle this matter as #19913, please allow me to close it for now. 🙏🏻 |
Same as #19313 but for Oxfmt. It will show a warning when Oxfmt is invoked without a configuration file.