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

feat: require one of check or overwrite options in format command #51

Merged
merged 5 commits into from
Jan 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
48 changes: 26 additions & 22 deletions src/commands/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,10 @@ 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.\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
Expand All @@ -70,12 +71,21 @@ pub struct FormatArgs {
#[arg(long, value_name = "SIZE")]
pub indentation_size: Option<usize>,

/// 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,
}

Expand All @@ -98,20 +108,23 @@ fn read_source(path: &Path) -> Result<String> {

/// 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 the document failed to parse, this emits the diagnostics and returns
/// `Ok(count)` of the diagnostics to the caller.
///
/// A return value of `Ok(0)` indicates the document was formatted.
fn format_document(
config: Config,
path: &Path,
overwrite: bool,
report_mode: Mode,
no_color: bool,
check: bool,
check_only: bool,
) -> Result<usize> {
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 {
Expand Down Expand Up @@ -147,7 +160,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);
Expand All @@ -156,12 +169,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)
}
Expand All @@ -188,10 +198,6 @@ 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 {
bail!("formatting a directory requires the `--overwrite` or `--check` option");
}

for entry in WalkDir::new(&args.path) {
let entry = entry.with_context(|| {
format!(
Expand All @@ -207,20 +213,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,
)?;
}

Expand Down
Loading