fix(args)!: stop greedy multi-value options from swallowing the positional range

.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

git-cliff/src/args.rs

@@ -115,33 +115,37 @@ pub struct Opt {
 	)]
     pub repository: Option<Vec<PathBuf>>,
     /// 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<Vec<Pattern>>,
     /// 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<Vec<Pattern>>,
     /// Sets the regex for matching git tags.
     #[arg(long, env = "GIT_CLIFF_TAG_PATTERN", value_name = "PATTERN")]
     pub tag_pattern: Option<Regex>,
     /// Sets custom commit messages to include in the changelog.
-    #[arg(
-		long,
-		env = "GIT_CLIFF_WITH_COMMIT",
-		value_name = "MSG",
-		num_args(1..)
-	)]
+    // One value per occurrence, same reasoning as `include_path`. No
+    // `value_delimiter` here: a commit message legitimately contains spaces
+    // (`--with-commit "<sha> 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))]
     pub with_commit: Option<Vec<String>>,
     /// Sets custom message for the latest release.
     #[arg(
@@ -161,12 +165,10 @@ pub struct Opt {
     #[arg(long, env = "GIT_CLIFF_COUNT_TAGS", value_name = "PATTERN")]
     pub count_tags: Option<Regex>,
     /// Sets commits that will be skipped in the changelog.
-    #[arg(
-		long,
-		env = "GIT_CLIFF_SKIP_COMMIT",
-		value_name = "SHA1",
-		num_args(1..)
-	)]
+    // 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))]
     pub skip_commit: Option<Vec<String>>,
     /// Prepends entries to the given changelog file.
     #[arg(
@@ -625,4 +627,188 @@ 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")])
+        );
+    }
+
+    #[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
+        // "<sha> <message>" 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()])
+        );
+    }
 }

website/docs/usage/args.md

@@ -34,15 +34,15 @@ git-cliff [FLAGS] [OPTIONS] [--] [RANGE]
     --config-url <URL>             Sets the URL for the configuration file [env: GIT_CLIFF_CONFIG_URL=]
 -w, --workdir <PATH>               Sets the working directory [env: GIT_CLIFF_WORKDIR=]
 -r, --repository <PATH>...         Sets the git repository [env: GIT_CLIFF_REPOSITORY=]
-    --include-path <PATTERN>...    Sets the path to include related commits [env: GIT_CLIFF_INCLUDE_PATH=]
-    --exclude-path <PATTERN>...    Sets the path to exclude related commits [env: GIT_CLIFF_EXCLUDE_PATH=]
+    --include-path <PATTERN>       Sets the path to include related commits [env: GIT_CLIFF_INCLUDE_PATH=]
+    --exclude-path <PATTERN>       Sets the path to exclude related commits [env: GIT_CLIFF_EXCLUDE_PATH=]
     --tag-pattern <PATTERN>        Sets the regex for matching git tags [env: GIT_CLIFF_TAG_PATTERN=]
-    --with-commit <MSG>...         Sets custom commit messages to include in the changelog [env: GIT_CLIFF_WITH_COMMIT=]
+    --with-commit <MSG>            Sets custom commit messages to include in the changelog [env: GIT_CLIFF_WITH_COMMIT=]
     --with-tag-message [<MSG>]     Sets custom message for the latest release [env: GIT_CLIFF_WITH_TAG_MESSAGE=]
     --skip-tags <PATTERN>          Sets the tags to skip in the changelog [env: GIT_CLIFF_SKIP_TAGS=]
     --ignore-tags <PATTERN>        Sets the tags to ignore in the changelog [env: GIT_CLIFF_IGNORE_TAGS=]
     --count-tags <PATTERN>         Sets the tags to count in the changelog [env: GIT_CLIFF_COUNT_TAGS=]
-    --skip-commit <SHA1>...        Sets commits that will be skipped in the changelog [env: GIT_CLIFF_SKIP_COMMIT=]
+    --skip-commit <SHA1>           Sets commits that will be skipped in the changelog [env: GIT_CLIFF_SKIP_COMMIT=]
 -p, --prepend [<PATH>]             Prepends entries to the given changelog file [env: GIT_CLIFF_PREPEND=]
 -o, --output [<PATH>]              Writes output to the given file [env: GIT_CLIFF_OUTPUT=]
 -t, --tag <TAG>                    Sets the tag for the latest version [env: GIT_CLIFF_TAG=]