From 0ce17f804855a5f47453d1cdf1507f02e1ad1355 Mon Sep 17 00:00:00 2001 From: Scott Smith Date: Wed, 8 Jan 2025 16:46:34 -0800 Subject: [PATCH 1/5] feat: require one of check or overwrite options in format command --- src/commands/format.rs | 44 ++++++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/src/commands/format.rs b/src/commands/format.rs index 68c59ac..7537cf0 100644 --- a/src/commands/format.rs +++ b/src/commands/format.rs @@ -43,9 +43,9 @@ const DEFAULT_SPACE_IDENT_SIZE: usize = 4; author, version, about, - after_help = "By default, the `format` command will print a single formatted WDL \ - document.\n\nUse the `--overwrite` option to replace a WDL document, or a \ - directory containing WDL documents, with the formatted source." + after_help = "Use the `--overwrite` option to replace a WDL document, or a directory \ + containing WDL documents, with the formatted source. Use the `--check` option \ + to print the diff or none if the source is already formatted." )] pub struct FormatArgs { /// The path to the WDL document to format (`-` for STDIN); the path may be @@ -70,12 +70,21 @@ pub struct FormatArgs { #[arg(long, value_name = "SIZE")] pub indentation_size: Option, + /// Argument group defining the mode of behavior + #[command(flatten)] + mode: ModeGroup, +} + +/// Argument group defining the mode of behavior +#[derive(Parser, Debug)] +#[group(required = true, multiple = false)] +pub struct ModeGroup { /// Overwrite the WDL documents with the formatted versions #[arg(long, conflicts_with = "check")] pub overwrite: bool, /// Check if files are formatted correctly and print diff if not - #[arg(long, conflicts_with = "overwrite")] + #[arg(long)] pub check: bool, } @@ -98,6 +107,9 @@ fn read_source(path: &Path) -> Result { /// Formats a document. /// +/// Checks if the document is formatted correctly and prints the diff if not +/// if check_only is true. +/// /// If the document failed to parse, this emits the diagnostics and returns /// `Ok(count)` of the diagnostics to the caller. /// @@ -105,13 +117,12 @@ fn read_source(path: &Path) -> Result { fn format_document( config: Config, path: &Path, - overwrite: bool, report_mode: Mode, no_color: bool, - check: bool, + check_only: bool, ) -> Result { if path.to_str() != Some("-") { - let action = if check { "checking" } else { "formatting" }; + let action = if check_only { "checking" } else { "formatting" }; println!( "{action_colored} `{path}`", action_colored = if no_color { @@ -147,7 +158,7 @@ fn format_document( let formatter = Formatter::new(config); let formatted = formatter.format(&document)?; - if check { + if check_only { if formatted != source { print!("{}", StrComparison::new(&source, &formatted)); return Ok(1); @@ -156,12 +167,9 @@ fn format_document( return Ok(0); } - if overwrite { - fs::write(path, formatted) - .with_context(|| format!("failed to write `{path}`", path = path.display()))?; - } else { - print!("{formatted}"); - } + // write file because check is not true + fs::write(path, formatted) + .with_context(|| format!("failed to write `{path}`", path = path.display()))?; Ok(0) } @@ -188,7 +196,7 @@ pub fn format(args: FormatArgs) -> Result<()> { let mut diagnostics = 0; if args.path.to_str() != Some("-") && args.path.is_dir() { - if !args.overwrite && !args.check { + if !args.mode.overwrite && !args.mode.check { bail!("formatting a directory requires the `--overwrite` or `--check` option"); } @@ -207,20 +215,18 @@ pub fn format(args: FormatArgs) -> Result<()> { diagnostics += format_document( config, path, - args.overwrite, args.report_mode, args.no_color, - args.check, + args.mode.check, )?; } } else { diagnostics += format_document( config, &args.path, - args.overwrite, args.report_mode, args.no_color, - args.check, + args.mode.check, )?; } From cfbb95dae3e63e08665f2b4725391d966b9f0098 Mon Sep 17 00:00:00 2001 From: Andrew Frantz Date: Thu, 16 Jan 2025 10:42:59 -0500 Subject: [PATCH 2/5] Apply suggestions from code review --- src/commands/format.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/commands/format.rs b/src/commands/format.rs index 7537cf0..c7332c2 100644 --- a/src/commands/format.rs +++ b/src/commands/format.rs @@ -43,9 +43,10 @@ const DEFAULT_SPACE_IDENT_SIZE: usize = 4; author, version, about, - after_help = "Use the `--overwrite` option to replace a WDL document, or a directory \ - containing WDL documents, with the formatted source. Use the `--check` option \ - to print the diff or none if the source is already formatted." + after_help = "Use the `--overwrite` option to replace a WDL document or a directory \ + containing WDL documents with the formatted source.\nUse the `--check` option \ + to verify that a document or a directory containing WDL documents is already formatted \ + and print the diff if not." )] pub struct FormatArgs { /// The path to the WDL document to format (`-` for STDIN); the path may be @@ -107,8 +108,8 @@ fn read_source(path: &Path) -> Result { /// Formats a document. /// -/// Checks if the document is formatted correctly and prints the diff if not -/// if check_only is true. +/// If `check_only` is true, checks if the document is formatted correctly and prints the diff if not +/// then exits. Else will format and overwrite the document. /// /// If the document failed to parse, this emits the diagnostics and returns /// `Ok(count)` of the diagnostics to the caller. From 16e530211272a480c965f9f4e2d3ad88a5f850c1 Mon Sep 17 00:00:00 2001 From: Andrew Frantz Date: Thu, 16 Jan 2025 10:44:40 -0500 Subject: [PATCH 3/5] Update src/commands/format.rs --- src/commands/format.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/commands/format.rs b/src/commands/format.rs index c7332c2..599fdc0 100644 --- a/src/commands/format.rs +++ b/src/commands/format.rs @@ -197,9 +197,6 @@ pub fn format(args: FormatArgs) -> Result<()> { let mut diagnostics = 0; if args.path.to_str() != Some("-") && args.path.is_dir() { - if !args.mode.overwrite && !args.mode.check { - bail!("formatting a directory requires the `--overwrite` or `--check` option"); - } for entry in WalkDir::new(&args.path) { let entry = entry.with_context(|| { From 268c06cff624de8f390a3d41b577f994ce8c649f Mon Sep 17 00:00:00 2001 From: Andrew Frantz Date: Thu, 16 Jan 2025 10:47:16 -0500 Subject: [PATCH 4/5] Update format.rs --- src/commands/format.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/commands/format.rs b/src/commands/format.rs index 599fdc0..8858608 100644 --- a/src/commands/format.rs +++ b/src/commands/format.rs @@ -45,8 +45,8 @@ const DEFAULT_SPACE_IDENT_SIZE: usize = 4; about, after_help = "Use the `--overwrite` option to replace a WDL document or a directory \ containing WDL documents with the formatted source.\nUse the `--check` option \ - to verify that a document or a directory containing WDL documents is already formatted \ - and print the diff if not." + to verify that a document or a directory containing WDL documents is already \ + formatted and print the diff if not." )] pub struct FormatArgs { /// The path to the WDL document to format (`-` for STDIN); the path may be @@ -108,8 +108,9 @@ fn read_source(path: &Path) -> Result { /// Formats a document. /// -/// If `check_only` is true, checks if the document is formatted correctly and prints the diff if not -/// then exits. Else will format and overwrite the document. +/// If `check_only` is true, checks if the document is formatted correctly and +/// prints the diff if not then exits. Else will format and overwrite the +/// document. /// /// If the document failed to parse, this emits the diagnostics and returns /// `Ok(count)` of the diagnostics to the caller. @@ -197,7 +198,6 @@ pub fn format(args: FormatArgs) -> Result<()> { let mut diagnostics = 0; if args.path.to_str() != Some("-") && args.path.is_dir() { - for entry in WalkDir::new(&args.path) { let entry = entry.with_context(|| { format!( From d609f477fb8e5716a58a7f2943923cb538c5a540 Mon Sep 17 00:00:00 2001 From: Andrew Frantz Date: Thu, 16 Jan 2025 10:53:14 -0500 Subject: [PATCH 5/5] Update CHANGELOG.md --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b92452a..a2c7274 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Changed + +* `format` now requires one of the `--check` or `--overwrite` arguments ([#51](/~https://github.com/stjude-rust-labs/sprocket/pull/51)). + ## 0.9.0 - 10-22-2024 ### Changed