diff --git a/src/cli.rs b/src/cli.rs index 57e440203..65128fb7e 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -11,81 +11,108 @@ use git2::{DiffStatsFormat, Repository}; use std::{ ffi::OsString, io::{self, IsTerminal, Write}, + num::NonZeroUsize, path::{Component, Path, PathBuf}, sync::Arc, }; +/// The metadata outputs, named one by one in conflicts so clap's error names +/// the flag that was given rather than the whole group. +const METADATA: [&str; 5] = ["name_only", "name_status", "stat", "numstat", "shortstat"]; + /// Structural diffs with Git-style comparison inputs #[derive(Parser)] #[command( name = env!("CARGO_BIN_NAME"), version, - group(ArgGroup::new("metadata").args(["name_only", "name_status", "stat", "numstat", "shortstat"])), + group(ArgGroup::new("metadata").args(METADATA)), + group(ArgGroup::new("names").args(["name_only", "name_status"])), after_help = "Examples:\n diffr\n diffr --cached\n diffr main...HEAD -- src/\n diffr --no-index -- before.rs after.rs\n diffr main HEAD --format ndjson\n\nUnsupported Git flags are rejected; this is not a complete git diff implementation." )] struct Cli { #[command(subcommand)] command: Option, + /// Directory in the repository to diff from; paths are relative to it #[arg(long, default_value = ".")] repo: PathBuf, /// Concurrent file diffs for --format ndjson; results are emitted as each finishes - #[arg(short, long, default_value_t = 16)] - jobs: usize, + #[arg(short, long, default_value = "16")] + jobs: NonZeroUsize, /// File tag priority: files carrying an earlier listed tag come first #[arg(long, value_delimiter = ',')] order: Vec, + /// Compare the index with HEAD, or with the given revision #[arg(long, visible_alias = "staged")] cached: bool, + /// Compare from the merge base of the given revision and the second one, or HEAD #[arg(long)] merge_base: bool, - #[arg(long)] + /// Compare two files on disk rather than Git revisions + #[arg(long, conflicts_with_all = ["cached", "merge_base", "null"], conflicts_with_all = METADATA)] no_index: bool, + /// Swap the two sides of the comparison #[arg(short = 'R', long)] reverse: bool, + /// Exit with 1 when there are differences #[arg(long)] exit_code: bool, + /// Print nothing; exit with 1 when there are differences #[arg(long)] quiet: bool, + /// Print the names of changed files #[arg(long)] name_only: bool, + /// Print the names and statuses of changed files #[arg(long)] name_status: bool, + /// Print a diffstat #[arg(long)] stat: bool, + /// Print added and deleted line counts for each file #[arg(long)] numstat: bool, + /// Print the diffstat's summary line #[arg(long)] shortstat: bool, - #[arg(short = 'z', long)] + /// Terminate --name-only and --name-status output with NULs + #[arg(short = 'z', long, requires = "names")] null: bool, + /// Do not detect renames #[arg(long)] no_renames: bool, + /// Detect renames, which is the default #[arg(short = 'M', long, conflicts_with = "no_renames")] find_renames: bool, /// Unchanged lines kept around each change; defaults to plugins.bundled.context.lines #[arg(short = 'U', long)] unified: Option, /// Write the event stream to stdout instead of opening the terminal UI - #[arg(long)] + #[arg(long, conflicts_with = "quiet", conflicts_with_all = METADATA)] format: Option, /// Emit initial files followed by deferred annotations (NDJSON v4) - #[arg(long, requires = "format")] + #[arg(long, requires = "format", conflicts_with = "quiet", conflicts_with_all = METADATA)] stream_annotations: bool, /// Include every token's tree-sitter capture name in --format ndjson output - #[arg(long)] + #[arg(long, requires = "format", conflicts_with = "quiet", conflicts_with_all = METADATA)] syntax: bool, /// Columns for --stat; defaults to the terminal's width #[arg(long)] width: Option, + /// Don't consider comments when diffing #[arg(long)] ignore_comments: bool, + /// Files larger than this many bytes on either side get a line diff; defaults to diff.byte_limit #[arg(long)] byte_limit: Option, + /// The largest AST matching graph to explore for one file; defaults to diff.graph_limit #[arg(long)] graph_limit: Option, + /// Files with more parse errors than this get a line diff; defaults to diff.parse_error_limit #[arg(long)] parse_error_limit: Option, + /// Revisions, then paths items: Vec, + /// Paths #[arg(last = true)] paths: Vec, } @@ -162,9 +189,6 @@ pub(crate) fn run() -> Result { if !streaming && !metadata_or_quiet { return launch_tui(&frontend_args, true); } - if streaming && metadata_or_quiet { - return Err("--format ndjson cannot be combined with --quiet or metadata output".into()); - } let stream_options = crate::protocol::stream::Options { syntax: args.syntax, updates: args.stream_annotations, @@ -176,9 +200,6 @@ pub(crate) fn run() -> Result { stream_options, ); } - if args.null && !args.name_only && !args.name_status { - return Err("-z currently requires --name-only or --name-status".into()); - } let location = std::fs::canonicalize(&args.repo)?; let repo = Repository::discover(&location)?; let workspace = repo.workdir().unwrap_or(repo.path()); @@ -215,12 +236,9 @@ pub(crate) fn run() -> Result { )?; session.diff_options = diff_options; let changed = session.remaining() > 0; - if args.jobs == 0 { - return Err("--jobs must be at least 1".into()); - } let ended = crate::protocol::stream::write( session, - args.jobs, + args.jobs.get(), Arc::new(pipeline), stream_options, &mut io::stdout().lock(), @@ -461,9 +479,6 @@ fn no_index( if paths.len() != 2 { return Err("--no-index requires two file paths".into()); } - if args.cached || args.merge_base || args.metadata() || args.null { - return Err("--no-index currently supports structural file output and --quiet, not index or metadata options".into()); - } let mut paths = paths; if args.reverse { paths.swap(0, 1); diff --git a/src/options.rs b/src/options.rs index fa8cf2c20..a537213f5 100644 --- a/src/options.rs +++ b/src/options.rs @@ -5,9 +5,8 @@ use std::ffi::OsStr; use std::fmt::Display; use std::path::{Path, PathBuf}; -use clap::Args; +use clap::{error::ErrorKind, Args}; -use crate::exit_codes::EXIT_BAD_ARGUMENTS; use crate::parse::guess_language::{language_override_from_name, LanguageOverride}; pub(crate) const DEFAULT_BYTE_LIMIT: usize = 1_000_000; @@ -44,60 +43,55 @@ impl Default for DiffOptions { /// `diffr debug`: syntax dumps and the language list. #[derive(Args)] pub(crate) struct DebugArgs { - #[arg( - long, - value_name = "PATH", - long_help = "Parse a single file with tree-sitter and display the diffr syntax tree.", - help_heading = "DEBUG OPTIONS" - )] - dump_syntax: Option, - #[arg( - long, - value_name = "PATH", - long_help = "Parse a single file with tree-sitter and display the diffr syntax tree, as a DOT graph.", - help_heading = "DEBUG OPTIONS" - )] - dump_syntax_dot: Option, - #[arg( - long, - value_name = "PATH", - long_help = "Parse a single file with tree-sitter and display the tree-sitter parse tree.", - help_heading = "DEBUG OPTIONS" - )] - dump_ts: Option, - #[arg( - long, - env = "DFT_IGNORE_COMMENTS", - help = "Don't consider comments when diffing." - )] + #[command(flatten)] + action: DebugAction, + /// Don't consider comments when diffing. + #[arg(long, env = "DFT_IGNORE_COMMENTS")] ignore_comments: bool, + /// Associate this glob pattern with this language, overriding normal language detection + /// + /// For example: + /// + /// $ diffr debug --override='*.c:C++' --dump-syntax file.c + /// + /// See --list-languages for the list of language names. Language names are matched case insensitively. Overrides may also specify the language "text" to treat a file as plain text. + /// + /// This argument may be given more than once. For example: + /// + /// $ diffr debug --override='CustomFile:json' --override='*.c:text' --dump-syntax file.c + /// + /// To configure multiple overrides using environment variables, diffr also accepts DFT_OVERRIDE_1 up to DFT_OVERRIDE_9. + /// + /// $ export DFT_OVERRIDE='CustomFile:json' + /// $ export DFT_OVERRIDE_1='*.c:text' + /// $ export DFT_OVERRIDE_2='*.js:javascript jsx' + /// + /// When multiple overrides are specified, the first matching override wins. #[arg( long = "override", value_name = "GLOB:NAME", env = "DFT_OVERRIDE", - help = concat!("Associate this glob pattern with this language, overriding normal language detection. For example: - -$ ", env!("CARGO_BIN_NAME"), " debug --override='*.c:C++' --dump-syntax file.c - -See --list-languages for the list of language names. Language names are matched case insensitively. Overrides may also specify the language \"text\" to treat a file as plain text. - -This argument may be given more than once. For example: - -$ ", env!("CARGO_BIN_NAME"), " debug --override='CustomFile:json' --override='*.c:text' --dump-syntax file.c - -To configure multiple overrides using environment variables, diffr also accepts DFT_OVERRIDE_1 up to DFT_OVERRIDE_9. - -$ export DFT_OVERRIDE='CustomFile:json' -$ export DFT_OVERRIDE_1='*.c:text' -$ export DFT_OVERRIDE_2='*.js:javascript jsx' - -When multiple overrides are specified, the first matching override wins.") - )] - overrides: Vec, - #[arg( - long, - help = "Print all the languages supported by diffr, along with their recognised extensions." + value_parser = parse_override, + verbatim_doc_comment )] + overrides: Vec<(LanguageOverride, glob::Pattern)>, +} + +/// What `diffr debug` does: exactly one of these. +#[derive(Args)] +#[group(required = true, multiple = false)] +struct DebugAction { + /// Parse a single file with tree-sitter and display the diffr syntax tree. + #[arg(long, value_name = "PATH", help_heading = "DEBUG OPTIONS")] + dump_syntax: Option, + /// Parse a single file with tree-sitter and display the diffr syntax tree, as a DOT graph. + #[arg(long, value_name = "PATH", help_heading = "DEBUG OPTIONS")] + dump_syntax_dot: Option, + /// Parse a single file with tree-sitter and display the tree-sitter parse tree. + #[arg(long, value_name = "PATH", help_heading = "DEBUG OPTIONS")] + dump_ts: Option, + /// Print all the languages supported by diffr, along with their recognised extensions. + #[arg(long)] list_languages: bool, } @@ -168,95 +162,94 @@ pub(crate) enum Mode { }, } -fn parse_overrides_or_die(raw_overrides: &[String]) -> Vec<(LanguageOverride, Vec)> { - let mut overrides: Vec<(LanguageOverride, Vec)> = vec![]; - let mut invalid_syntax = false; +/// One `GLOB:LANG_NAME` override, as `--override` and `DFT_OVERRIDE_N` take it. +fn parse_override(raw: &str) -> Result<(LanguageOverride, glob::Pattern), String> { + let (glob_str, lang_name) = raw + .rsplit_once(':') + .ok_or("expected GLOB:LANG_NAME, e.g. '*.js:JSON'")?; + let pattern = glob::Pattern::new(glob_str) + .map_err(|error| format!("invalid glob '{glob_str}': {}", error.msg))?; + let language = language_override_from_name(lang_name).ok_or_else(|| { + format!("no such language '{lang_name}'; see --list-languages for the names, which match case insensitively") + })?; + Ok((language, pattern)) +} - for raw_override in raw_overrides { - if let Some((glob_str, lang_name)) = raw_override.rsplit_once(':') { - match glob::Pattern::new(glob_str) { - Ok(pattern) => { - if let Some(language_override) = language_override_from_name(lang_name) { - overrides.push((language_override, vec![pattern])); - } else { - eprintln!("No such language '{}'", lang_name); - eprintln!("See --list-languages for the names of all languages available. Language overrides are case insensitive."); - invalid_syntax = true; - } - } - Err(e) => { - eprintln!("Invalid glob syntax '{}'", glob_str); - eprintln!("Glob parsing error: {}", e.msg); - invalid_syntax = true; - } - } - } else { - eprintln!("Invalid language override syntax '{}'", raw_override); - eprintln!("Language overrides are in the format 'GLOB:LANG_NAME', e.g. '*.js:JSON'."); - invalid_syntax = true; +/// `DFT_OVERRIDE_1` up to `DFT_OVERRIDE_9`, which clap does not read. +fn numbered_env_overrides() -> Vec<(LanguageOverride, glob::Pattern)> { + let mut overrides = vec![]; + for i in 1..=9 { + let name = format!("DFT_OVERRIDE_{i}"); + let value = match env::var(&name) { + Ok(value) => value, + Err(env::VarError::NotPresent) => continue, + Err(env::VarError::NotUnicode(_)) => clap::Error::raw( + ErrorKind::InvalidUtf8, + format!("{name} is not valid UTF-8\n"), + ) + .exit(), + }; + match parse_override(&value) { + Ok(language_override) => overrides.push(language_override), + Err(message) => clap::Error::raw( + ErrorKind::ValueValidation, + format!("invalid value '{value}' for {name}: {message}\n"), + ) + .exit(), } } + overrides +} - if invalid_syntax { - std::process::exit(EXIT_BAD_ARGUMENTS); - } - - let mut combined_overrides: Vec<(LanguageOverride, Vec)> = vec![]; - for (lang, globs) in overrides { - if let Some((prev_lang, prev_globs)) = combined_overrides.last_mut() { - if *prev_lang == lang { - prev_globs.extend(globs); - } else { - combined_overrides.push((lang, globs)); - } - } else { - combined_overrides.push((lang, globs)); +/// Adjacent overrides naming the same language share one entry. +fn combine_overrides( + overrides: impl IntoIterator, +) -> Vec<(LanguageOverride, Vec)> { + let mut combined: Vec<(LanguageOverride, Vec)> = vec![]; + for (lang, pattern) in overrides { + match combined.last_mut() { + Some((prev_lang, prev_globs)) if *prev_lang == lang => prev_globs.push(pattern), + _ => combined.push((lang, vec![pattern])), } } - - combined_overrides + combined } impl DebugArgs { pub(crate) fn mode(&self) -> Mode { - let mut raw_overrides = self.overrides.clone(); - for i in 1..=9 { - if let Ok(value) = env::var(format!("DFT_OVERRIDE_{}", i)) { - raw_overrides.push(value); - } - } - - let language_overrides = parse_overrides_or_die(&raw_overrides); - - if self.list_languages { + let language_overrides = combine_overrides( + self.overrides + .iter() + .cloned() + .chain(numbered_env_overrides()), + ); + + let action = &self.action; + if action.list_languages { return Mode::ListLanguages { language_overrides }; } - - if let Some(path) = &self.dump_syntax { + if let Some(path) = &action.dump_syntax { return Mode::DumpSyntax { path: path.to_owned(), ignore_comments: self.ignore_comments, language_overrides, }; } - - if let Some(path) = &self.dump_syntax_dot { + if let Some(path) = &action.dump_syntax_dot { return Mode::DumpSyntaxDot { path: path.to_owned(), ignore_comments: self.ignore_comments, language_overrides, }; } - - if let Some(path) = &self.dump_ts { - return Mode::DumpTreeSitter { - path: path.to_owned(), - language_overrides, - }; + let path = action + .dump_ts + .as_ref() + .expect("clap requires exactly one debug action"); + Mode::DumpTreeSitter { + path: path.to_owned(), + language_overrides, } - - eprintln!("Pass one of --dump-syntax, --dump-syntax-dot, --dump-ts or --list-languages."); - std::process::exit(EXIT_BAD_ARGUMENTS); } } diff --git a/tests/cli.rs b/tests/cli.rs index 3f89e9607..4cca45459 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -173,6 +173,33 @@ fn config_ui_requires_a_terminal() { .stderr(predicate::str::contains("terminal UI needs a terminal")); } +/// Invalid flag combinations are rejected while parsing, before the terminal +/// UI would launch. +#[test] +fn invalid_flag_combinations_are_rejected_before_the_terminal_ui() { + for (args, message) in [ + (&["-z"][..], "<--name-only|--name-status>"), + (&["--jobs", "0"], "invalid value '0' for '--jobs '"), + (&["--syntax"], "--format "), + ( + &["--format", "ndjson", "--stat"], + "cannot be used with '--stat'", + ), + (&["--syntax", "--stat"], "cannot be used with '--stat'"), + ( + &["--no-index", "--cached", "a", "b"], + "cannot be used with '--cached'", + ), + ] { + get_base_command() + .args(args) + .assert() + .failure() + .code(2) + .stderr(predicate::str::contains(message)); + } +} + #[test] fn diff_flags_before_a_subcommand_are_rejected() { get_base_command() @@ -184,3 +211,46 @@ fn diff_flags_before_a_subcommand_are_rejected() { "the argument '--cached' cannot be used with the 'config' subcommand", )); } + +#[test] +fn a_malformed_override_is_a_clap_error() { + debug_command() + .args(["--override=*.c:Nope", "--list-languages"]) + .assert() + .failure() + .code(2) + .stderr(predicate::str::contains( + "invalid value '*.c:Nope' for '--override ': no such language 'Nope'", + )); +} + +#[test] +fn a_malformed_numbered_override_names_its_variable() { + debug_command() + .arg("--list-languages") + .env("DFT_OVERRIDE_2", "bad") + .assert() + .failure() + .code(2) + .stderr(predicate::str::contains( + "invalid value 'bad' for DFT_OVERRIDE_2: expected GLOB:LANG_NAME", + )); +} + +#[test] +fn debug_needs_exactly_one_action() { + debug_command() + .arg("--ignore-comments") + .assert() + .failure() + .code(2) + .stderr(predicate::str::contains( + "the following required arguments were not provided", + )); + debug_command() + .args(["--list-languages", "--dump-ts", "sample_files/simple_1.js"]) + .assert() + .failure() + .code(2) + .stderr(predicate::str::contains("cannot be used with")); +}