From 17dc46fbc08d78bc8424c2d861360a404b8f7725 Mon Sep 17 00:00:00 2001 From: cds-amal Date: Fri, 24 Jul 2026 13:13:26 -0400 Subject: [PATCH 1/3] fix(args)!: stop --include-path from swallowing the positional range `--include-path` and `--exclude-path` were declared `num_args(1..)` so they could take space-separated patterns. clap reads that greedily, so in `git-cliff --include-path 'pkg/**' v1.0.0..v1.1.0` the trailing range is absorbed as one more glob: `opt.range` comes back `None` and git-cliff emits the whole (path-filtered) history. Nothing warns, and the output is plausible enough to ship, which is the bad kind of bug. Both options now take exactly one value per occurrence. The two multi-pattern forms that were actually documented still work: repeat the flag, or pass one quoted space-delimited value (`value_delimiter = ' '` still splits it, which is also how the `GIT_CLIFF_INCLUDE_PATH` env var keeps working). The trade-off is spelled out in `cli_include_path_unquoted_second_token_falls_to_range_slot`: the unquoted multi-token form (`--include-path a b`, no quotes) used to be absorbed and now puts `b` in the RANGE slot, where it fails loudly at revparse. Loud failure on an undocumented form beats a silently wrong changelog on a documented one. The fixture suite gains a second run of `test-monorepo-include-path` with the range moved after `--include-path`, sharing the existing `expected.md`: both orderings must now produce the same changelog. BREAKING CHANGE: `--include-path` and `--exclude-path` accept one value per occurrence. Unquoted multi-pattern invocations such as `--include-path 'a/**' 'b/**'` must become repeated flags (`--include-path 'a/**' --include-path 'b/**'`) or a single quoted value (`--include-path 'a/** b/**'`). --- .github/workflows/test-fixtures.yml | 7 ++ git-cliff/src/args.rs | 141 +++++++++++++++++++++++++--- website/docs/usage/args.md | 4 +- 3 files changed, 138 insertions(+), 14 deletions(-) diff --git a/.github/workflows/test-fixtures.yml b/.github/workflows/test-fixtures.yml index 2365a78a52..7160cd52e0 100644 --- a/.github/workflows/test-fixtures.yml +++ b/.github/workflows/test-fixtures.yml @@ -125,8 +125,15 @@ jobs: command: --tag v0.2.0 - fixtures-name: test-custom-remote-api-url command: v1.4.0..v1.4.1 + # NOTE: These two entries share one fixture (and one expected.md). + # The second puts the positional range AFTER --include-path: since + # include-path takes exactly one value per occurrence, the trailing + # range lands in the RANGE slot. (Under the old greedy num_args(1..) + # it was absorbed as one more glob and silently dropped.) - fixtures-name: test-monorepo-include-path command: v2.6.1..v2.7.0 --include-path .github/fixtures/ + - fixtures-name: test-monorepo-include-path + command: --include-path .github/fixtures/ v2.6.1..v2.7.0 - fixtures-name: test-require-conventional-negative - fixtures-name: test-require-conventional-skipped - fixtures-name: test-submodules diff --git a/git-cliff/src/args.rs b/git-cliff/src/args.rs index 1c0e516921..7b51e61952 100644 --- a/git-cliff/src/args.rs +++ b/git-cliff/src/args.rs @@ -115,22 +115,27 @@ pub struct Opt { )] pub repository: Option>, /// Sets the path to include related commits. + // One value per occurrence: a greedy `num_args(1..)` here would absorb a + // trailing positional RANGE as one more pattern, silently dropping the + // range. Multiple patterns come via repeated flags or one + // space-delimited value. #[arg( - long, - env = "GIT_CLIFF_INCLUDE_PATH", - value_name = "PATTERN", - value_delimiter = ' ', - num_args(1..) - )] + long, + env = "GIT_CLIFF_INCLUDE_PATH", + value_name = "PATTERN", + value_delimiter = ' ', + num_args(1) + )] pub include_path: Option>, /// Sets the path to exclude related commits. + // Same one-value-per-occurrence constraint as `include_path` above. #[arg( - long, - env = "GIT_CLIFF_EXCLUDE_PATH", - value_name = "PATTERN", - value_delimiter = ' ', - num_args(1..) - )] + long, + env = "GIT_CLIFF_EXCLUDE_PATH", + value_name = "PATTERN", + value_delimiter = ' ', + num_args(1) + )] pub exclude_path: Option>, /// Sets the regex for matching git tags. #[arg(long, env = "GIT_CLIFF_TAG_PATTERN", value_name = "PATTERN")] @@ -625,4 +630,116 @@ mod tests { Ok(()) } + + #[test] + fn cli_include_path_does_not_swallow_trailing_positional_range() { + // `--include-path` takes exactly one value per occurrence, so a + // trailing positional RANGE lands in the RANGE slot instead of being + // absorbed as one more glob. Under the old greedy `num_args(1..)` + // declaration this exact argv silently lost the range and emitted the + // whole (path-filtered) history. + let opt = Opt::try_parse_from([ + "git-cliff", + "--include-path", + "pkg/**", + "pkg/v1.0.0..pkg/v1.1.0", + ]) + .expect("parse"); + assert_eq!(opt.range.as_deref(), Some("pkg/v1.0.0..pkg/v1.1.0")); + assert_eq!( + opt.include_path, + Some(vec![Pattern::new("pkg/**").expect("pattern")]) + ); + } + + #[test] + fn cli_exclude_path_does_not_swallow_trailing_positional_range() { + let opt = Opt::try_parse_from([ + "git-cliff", + "--exclude-path", + "pkg/**", + "pkg/v1.0.0..pkg/v1.1.0", + ]) + .expect("parse"); + assert_eq!(opt.range.as_deref(), Some("pkg/v1.0.0..pkg/v1.1.0")); + assert_eq!( + opt.exclude_path, + Some(vec![Pattern::new("pkg/**").expect("pattern")]) + ); + } + + #[test] + fn cli_include_path_multiple_patterns_still_parse() { + // The two supported multi-pattern forms: repeated flags, and a single + // space-delimited value (what the space-separated fixture passes). + let repeated = Opt::try_parse_from([ + "git-cliff", + "--include-path", + "website/**/*", + "--include-path", + "docs/**/*", + ]) + .expect("parse"); + let delimited = + Opt::try_parse_from(["git-cliff", "--include-path", "website/**/* docs/**/*"]) + .expect("parse"); + for opt in [&repeated, &delimited] { + assert_eq!( + opt.include_path, + Some(vec![ + Pattern::new("website/**/*").expect("pattern"), + Pattern::new("docs/**/*").expect("pattern"), + ]) + ); + } + } + + #[test] + fn cli_include_path_unquoted_second_token_falls_to_range_slot() { + // Characterization of the compatibility trade-off: the unquoted + // multi-token form (`--include-path a b`, no quotes) used to be + // absorbed greedily; now the second token lands in the positional + // RANGE slot and fails loudly at revparse instead of silently + // producing a wrong changelog. + let opt = Opt::try_parse_from(["git-cliff", "--include-path", "website/**/*", "docs/**/*"]) + .expect("parse"); + assert_eq!( + opt.include_path, + Some(vec![Pattern::new("website/**/*").expect("pattern")]) + ); + assert_eq!(opt.range.as_deref(), Some("docs/**/*")); + } + + #[test] + fn cli_positional_range_before_include_path_is_not_swallowed() { + let opt = Opt::try_parse_from([ + "git-cliff", + "pkg/v1.0.0..pkg/v1.1.0", + "--include-path", + "pkg/**", + ]) + .expect("parse"); + assert_eq!(opt.range.as_deref(), Some("pkg/v1.0.0..pkg/v1.1.0")); + assert_eq!( + opt.include_path, + Some(vec![Pattern::new("pkg/**").expect("pattern")]) + ); + } + + #[test] + fn cli_double_dash_protects_trailing_positional_range() { + let opt = Opt::try_parse_from([ + "git-cliff", + "--include-path", + "pkg/**", + "--", + "pkg/v1.0.0..pkg/v1.1.0", + ]) + .expect("parse"); + assert_eq!(opt.range.as_deref(), Some("pkg/v1.0.0..pkg/v1.1.0")); + assert_eq!( + opt.include_path, + Some(vec![Pattern::new("pkg/**").expect("pattern")]) + ); + } } diff --git a/website/docs/usage/args.md b/website/docs/usage/args.md index 20a62acb12..ac5f2f8484 100644 --- a/website/docs/usage/args.md +++ b/website/docs/usage/args.md @@ -34,8 +34,8 @@ git-cliff [FLAGS] [OPTIONS] [--] [RANGE] --config-url Sets the URL for the configuration file [env: GIT_CLIFF_CONFIG_URL=] -w, --workdir Sets the working directory [env: GIT_CLIFF_WORKDIR=] -r, --repository ... Sets the git repository [env: GIT_CLIFF_REPOSITORY=] - --include-path ... Sets the path to include related commits [env: GIT_CLIFF_INCLUDE_PATH=] - --exclude-path ... Sets the path to exclude related commits [env: GIT_CLIFF_EXCLUDE_PATH=] + --include-path Sets the path to include related commits [env: GIT_CLIFF_INCLUDE_PATH=] + --exclude-path Sets the path to exclude related commits [env: GIT_CLIFF_EXCLUDE_PATH=] --tag-pattern Sets the regex for matching git tags [env: GIT_CLIFF_TAG_PATTERN=] --with-commit ... Sets custom commit messages to include in the changelog [env: GIT_CLIFF_WITH_COMMIT=] --with-tag-message [] Sets custom message for the latest release [env: GIT_CLIFF_WITH_TAG_MESSAGE=] From 132cfa1bae7ac5323a2d8d4b48fc7da56c8e0591 Mon Sep 17 00:00:00 2001 From: cds-amal Date: Fri, 24 Jul 2026 14:19:03 -0400 Subject: [PATCH 2/3] fix(args)!: stop --with-commit and --skip-commit from swallowing the range The same greedy `num_args(1..)` declaration fixed for the path flags in the previous commit sits on `--with-commit` and `--skip-commit`, with the same result: a trailing positional range is consumed as one more message or SHA, `opt.range` comes back `None`, and the changelog covers all of history. Both fail silently, because any string is a plausible commit message or SHA and nothing rejects the range once it lands in the wrong slot. $ git cliff --with-commit "feat: synthetic" v2.12.0..v2.13.0 ...whole history $ git cliff v2.12.0..v2.13.0 --with-commit "feat: synthetic" ...the range Both orderings now produce identical output. `--repository` shares the declaration but not the defect: its `value_parser = Opt::parse_dir` rejects a range as a path and errors loudly, so it is left alone. Neither flag gets a `value_delimiter`, so repeated flags become the only multi-value form. That is deliberate rather than an oversight. A commit message legitimately contains spaces, and `--with-commit " feat: add X"` is a documented form (see website/docs/usage/adding-commits.md), so splitting on whitespace would corrupt it; `cli_with_commit_keeps_whitespace_inside_one_value` pins that. A delimiter would be safe on `--skip-commit`, where SHA1s contain no spaces, but that is new behavior rather than a fix, so it stays out of scope here. This is deliberately a separate commit from the path-flag fix. The path flags are where the bug was reported and where the range/path combination is a documented workflow; these two are the same defect found by inspection. If the wider scope is unwanted, reverting this commit alone leaves the reported fix intact. BREAKING CHANGE: `--with-commit` and `--skip-commit` accept one value per occurrence. Unquoted multi-value invocations such as `--with-commit "feat: a" "feat: b"` must become repeated flags (`--with-commit "feat: a" --with-commit "feat: b"`). --- git-cliff/src/args.rs | 99 ++++++++++++++++++++++++++++++++++---- website/docs/usage/args.md | 4 +- 2 files changed, 91 insertions(+), 12 deletions(-) diff --git a/git-cliff/src/args.rs b/git-cliff/src/args.rs index 7b51e61952..b0d84a7217 100644 --- a/git-cliff/src/args.rs +++ b/git-cliff/src/args.rs @@ -141,12 +141,16 @@ pub struct Opt { #[arg(long, env = "GIT_CLIFF_TAG_PATTERN", value_name = "PATTERN")] pub tag_pattern: Option, /// Sets custom commit messages to include in the changelog. + // One value per occurrence, same reasoning as `include_path`. No + // `value_delimiter` here: a commit message legitimately contains spaces + // (`--with-commit " feat: add X"` is a documented form), so repeated + // flags are the only multi-value form. #[arg( - long, - env = "GIT_CLIFF_WITH_COMMIT", - value_name = "MSG", - num_args(1..) - )] + long, + env = "GIT_CLIFF_WITH_COMMIT", + value_name = "MSG", + num_args(1) + )] pub with_commit: Option>, /// Sets custom message for the latest release. #[arg( @@ -166,12 +170,15 @@ pub struct Opt { #[arg(long, env = "GIT_CLIFF_COUNT_TAGS", value_name = "PATTERN")] pub count_tags: Option, /// Sets commits that will be skipped in the changelog. + // Same one-value-per-occurrence constraint. A `value_delimiter` would be + // safe here (SHA1s contain no spaces) but that is new behavior, not a fix, + // so repeated flags stay the multi-value form. #[arg( - long, - env = "GIT_CLIFF_SKIP_COMMIT", - value_name = "SHA1", - num_args(1..) - )] + long, + env = "GIT_CLIFF_SKIP_COMMIT", + value_name = "SHA1", + num_args(1) + )] pub skip_commit: Option>, /// Prepends entries to the given changelog file. #[arg( @@ -742,4 +749,76 @@ mod tests { Some(vec![Pattern::new("pkg/**").expect("pattern")]) ); } + + #[test] + fn cli_with_commit_does_not_swallow_trailing_positional_range() { + let opt = Opt::try_parse_from([ + "git-cliff", + "--with-commit", + "feat: add x", + "v1.0.0..v1.1.0", + ]) + .expect("parse"); + assert_eq!(opt.range.as_deref(), Some("v1.0.0..v1.1.0")); + assert_eq!(opt.with_commit, Some(vec!["feat: add x".to_string()])); + } + + #[test] + fn cli_skip_commit_does_not_swallow_trailing_positional_range() { + let opt = Opt::try_parse_from([ + "git-cliff", + "--skip-commit", + "a78bc368e9ee382a3016c0c4bab41f7de4503bcd", + "v1.0.0..v1.1.0", + ]) + .expect("parse"); + assert_eq!(opt.range.as_deref(), Some("v1.0.0..v1.1.0")); + assert_eq!( + opt.skip_commit, + Some(vec!["a78bc368e9ee382a3016c0c4bab41f7de4503bcd".to_string()]) + ); + } + + #[test] + fn cli_with_commit_keeps_whitespace_inside_one_value() { + // No `value_delimiter` on this flag, so the documented + // " " form stays a single value rather than splitting + // into two patterns the way the path flags do. + let opt = Opt::try_parse_from([ + "git-cliff", + "--with-commit", + "8f55e69eba6e6ce811ace32bd84cc82215673cb6 feat: add X", + ]) + .expect("parse"); + assert_eq!( + opt.with_commit, + Some(vec![ + "8f55e69eba6e6ce811ace32bd84cc82215673cb6 feat: add X".to_string() + ]) + ); + } + + #[test] + fn cli_repeated_flags_remain_the_multi_value_form() { + let opt = Opt::try_parse_from([ + "git-cliff", + "--with-commit", + "feat: a", + "--with-commit", + "feat: b", + "--skip-commit", + "aaaaaaa", + "--skip-commit", + "bbbbbbb", + ]) + .expect("parse"); + assert_eq!( + opt.with_commit, + Some(vec!["feat: a".to_string(), "feat: b".to_string()]) + ); + assert_eq!( + opt.skip_commit, + Some(vec!["aaaaaaa".to_string(), "bbbbbbb".to_string()]) + ); + } } diff --git a/website/docs/usage/args.md b/website/docs/usage/args.md index ac5f2f8484..6eb53daef9 100644 --- a/website/docs/usage/args.md +++ b/website/docs/usage/args.md @@ -37,12 +37,12 @@ git-cliff [FLAGS] [OPTIONS] [--] [RANGE] --include-path Sets the path to include related commits [env: GIT_CLIFF_INCLUDE_PATH=] --exclude-path Sets the path to exclude related commits [env: GIT_CLIFF_EXCLUDE_PATH=] --tag-pattern Sets the regex for matching git tags [env: GIT_CLIFF_TAG_PATTERN=] - --with-commit ... Sets custom commit messages to include in the changelog [env: GIT_CLIFF_WITH_COMMIT=] + --with-commit Sets custom commit messages to include in the changelog [env: GIT_CLIFF_WITH_COMMIT=] --with-tag-message [] Sets custom message for the latest release [env: GIT_CLIFF_WITH_TAG_MESSAGE=] --skip-tags Sets the tags to skip in the changelog [env: GIT_CLIFF_SKIP_TAGS=] --ignore-tags Sets the tags to ignore in the changelog [env: GIT_CLIFF_IGNORE_TAGS=] --count-tags Sets the tags to count in the changelog [env: GIT_CLIFF_COUNT_TAGS=] - --skip-commit ... Sets commits that will be skipped in the changelog [env: GIT_CLIFF_SKIP_COMMIT=] + --skip-commit Sets commits that will be skipped in the changelog [env: GIT_CLIFF_SKIP_COMMIT=] -p, --prepend [] Prepends entries to the given changelog file [env: GIT_CLIFF_PREPEND=] -o, --output [] Writes output to the given file [env: GIT_CLIFF_OUTPUT=] -t, --tag Sets the tag for the latest version [env: GIT_CLIFF_TAG=] From 4b3607de0d1f5d7a34e2196d472ed6b4bd388386 Mon Sep 17 00:00:00 2001 From: cds-amal Date: Fri, 24 Jul 2026 19:01:11 -0400 Subject: [PATCH 3/3] cargo +nightly fmt --all --- git-cliff/src/args.rs | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/git-cliff/src/args.rs b/git-cliff/src/args.rs index b0d84a7217..3399879afd 100644 --- a/git-cliff/src/args.rs +++ b/git-cliff/src/args.rs @@ -145,12 +145,7 @@ pub struct Opt { // `value_delimiter` here: a commit message legitimately contains spaces // (`--with-commit " feat: add X"` is a documented form), so repeated // flags are the only multi-value form. - #[arg( - long, - env = "GIT_CLIFF_WITH_COMMIT", - value_name = "MSG", - num_args(1) - )] + #[arg(long, env = "GIT_CLIFF_WITH_COMMIT", value_name = "MSG", num_args(1))] pub with_commit: Option>, /// Sets custom message for the latest release. #[arg( @@ -173,12 +168,7 @@ pub struct Opt { // Same one-value-per-occurrence constraint. A `value_delimiter` would be // safe here (SHA1s contain no spaces) but that is new behavior, not a fix, // so repeated flags stay the multi-value form. - #[arg( - long, - env = "GIT_CLIFF_SKIP_COMMIT", - value_name = "SHA1", - num_args(1) - )] + #[arg(long, env = "GIT_CLIFF_SKIP_COMMIT", value_name = "SHA1", num_args(1))] pub skip_commit: Option>, /// Prepends entries to the given changelog file. #[arg(