feat(cli): introduce commit ranges with interval-notation endpoint flags#1489
Open
cds-amal wants to merge 23 commits intoorhun:mainfrom
Open
feat(cli): introduce commit ranges with interval-notation endpoint flags#1489cds-amal wants to merge 23 commits intoorhun:mainfrom
cds-amal wants to merge 23 commits intoorhun:mainfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1489 +/- ##
==========================================
+ Coverage 48.86% 51.76% +2.91%
==========================================
Files 26 27 +1
Lines 2272 2390 +118
==========================================
+ Hits 1110 1237 +127
+ Misses 1162 1153 -9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Author
|
Hi @orhun , I don't know how to proceed with the failing tests; any advice please? |
Owner
|
Seems like a flaky test due to a network error (404) - I just restarted the job. |
- resolve_rev(rev): resolve a revision string (tag/branch/SHA prefix/HEAD) to a full commit SHA, error if unresolvable. - is_root_commit(rev): detect whether a revision is the root (no parents), so callers can handle the `A^` fallback for inclusive-root endpoints.
- inclusive range descriptors `[`, `(`, `)`, `]` - all four default to `None` via `#[derive(Default)]`.
args: - `start_at`, - `start_after`, - `end_at`, - `end_before` TODO: wire up
todo -- docu them tests
Replace the procedural if/else chain with a three-line call into the `range` module: transform_range -> resolve_with -> execute_range. Net behavior is unchanged because every legacy flag (--latest, --current, --unreleased, positional A..B) produces the same commit range as before, verified by the 20+ range-related fixtures that already cover this surface. Range module's unit tests back-stop behavior at a finer grain. The pipeline normalizes endpoints too full SHAs via resolve_with, which is what the old hand-written body effectively did by pulling commit SHAs from the `tags` map keys.
Make the range endpoint cli args operational. New tests: one parse per option, intra-pair conflicts on both sides, and a parametric sweep over the legacy-vs-new cross product.
`--dry-run` prints the computed interval, the number of commits the
range covers, and the emitted git revision range, then exits without
rendering the changelog. Intended for users sanity-checking their range
selection before committing to a full render, and for CI pipelines that
want a cheap "any good goods to release?" signal.
Output shape (`--start-at v0.1.0 --end-at v0.2.0`):
range: [v0.1.0, v0.2.0]
commits: 3
emitted: <sha>^..<sha>
The `range:` line displays user friendly refs; the `emitted:` line shows
the actual git range the walker receives (SHAs, post-resolve), matching
what `set_commit_range` expects.
- s/tracing/log/ for logging - cargo +nightly fmt
Break `transform_range`'s one-long-function shape into a dispatcher plus a handful of small helpers, each responsible for one translation row: `current_range` preserves teh legacy `tag_names.len() < 2` fallback by delegating to `latest_range`. The old `determine_commit_range` code had this behavior and it's exercised (indirectly) by teh fixture suite; the stricter "always error if current tag is mising" variant would be a behavior regression.
…range
Three small clarity changes to the `range` module's surface:
- Import `git_cliff_core::{config::GitConfig, error::{Error, Result}}`
at the top of `range.rs`. Drops fully-qualified paths from seven
signatures and helper bodies; call sites now read as plain
`Result<CommitRange>` instead of teh two-colon wall.
- Introduce `RangeSelection { canonical, emitted }` for
`determine_commit_range`'s return value. The tuple-of-two shape was
easy to misread at the call site (which is which?); the named fields
document themselves. The two consumers in `lib.rs` now spell the
relevant half explicitly (`.emitted` for the walker, `.canonical` for
display).
- Move the dry-run output block (12 lines of `println!` + match) out of
`run_with_changelog_modifier` and into `range::print_dry_run`. The
formatting was only used for dry-run and it's cohesive with
`format_interval`, which can now drop `pub(crate)` and stay private.
- TODO
Two new fixtures exercise the new endpoint options on a small deterministic repo (template: Initial, two tagged releases v0.1.0 and v0.2.0 each with feat+fix, plus an unreleased test commit):
`Option<&String>` is unidiomatic. The standard Rust signature for a borrowed optional string is `Option<&str>`. Flip the helper's parameters and use `.as_deref()` at the call sites. `Endpoint::inclusive` / `::exclusive` already accept `impl Into<String>`, so use `&str`
Extending the skip patterns (`command` and `repo`) to include `resolve_with`. Hopefully this is the right call.
Reduce dry-run testing to string comparisons.
Add tests indicated by tarpaulin (LLVM engine --all-features)
- usage/args.md: list --dry-run, the four range endpoint options, and a "Range selection" section. - configuration/git.md: document the `start_at` / `start_after` / `end_at` / `end_before` keys. - usage/examples.md: examples for the new endpoint options and --dry-run.
The unbounded-left placeholder rendered as `first` in `--dry-run` output, which sat awkwardly next to a real ref like `HEAD` and read as if it might be something a user could type. `<first-commit>` makes the placeholder nature obvious; the angle brackets are the same convention git uses for value names in usage strings. No behavioral change beyond the cosmetic; the four test assertions on the literal are updated to match.
`transform_range` previously merged CLI and config field-by-field with `or`, so `--start-at X` on the CLI plus `start_after = "Y"` in the config ended up with both inclusive and exclusive slots filled and tripped the mutual-exclusion check; users expecting CLI to win got an error instead of an override, rats! The new shape is precedence-first: build a baseline `CommitRange` from config (which still errors if config alone is internally inconsistent), then let the CLI replace each side as a unit. Setting `--start-at` or `--start-after` overrides both `start_at` and `start_after` from config; same for the right side. The two sides remain independent, so a CLI `--start-at` does not disturb config's `end_at`. Three new tests cover the cross-key override (CLI `start_after` over config `start_at`, resulting endpoint is exclusive), the partial-side override (CLI replaces left, config still drives right), and the config-only conflict (both keys set in TOML still errors).
Whoops! `--end-before X` (and the equivalent `end_before` config key) without a left bound crashed with "unable to parse OID - too long". The path: `execute_range` for `(left=None, right=Some(exclusive))` emits `<sha>^` (a single revspec, no `..` in it), `set_commit_range` saw no `..`, took the single-OID branch, and `Oid::from_str` choked on the `^`. The narrow contract on `set_commit_range` was a bug: the function comment said "When a single SHA is provided", but in practice the input can be a tag, a branch, or a parent expression like `<sha>^`. Switch to `revparse_single().id()`, which handles the full revspec grammar. Add regression test in a fixture (`test-range-end-before-only`) that exercises `--end-before <tag>` with no left bound, wired into the CI matrix next to the existing `test-range-end-before-exclusive`.
cds-amal
changed the title
This was referenced Apr 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR refactors commit-range selection on the CLI by borrowing math-interval notation, which captures both location (which commit) and inclusivity (whether the endpoint is in the range). A range is bounded by a
startand anend;[/]are inclusive,(/)are exclusive.Internally, the range is now a pure value (
CommitRangewith explicitEndpoints) flowing through a transform -> resolve -> execute pipeline that's inspectable (--dry-run) and unit-testable.New endpoint options
--start-at Xstart_at = "X"X; walk forward.[X, ...--start-after Xstart_after = "X"X; walk forward.(X, ...--end-at Yend_at = "Y"Y; walk back...., Y]--end-before Yend_before = "Y"Y; stop before it...., Y)Naming convention:
*_atis inclusive,*_after/*_beforeis exclusive. Within each pair, at most one may be set. The two pairs are independent, so any inclusivity combination is describable.How legacy flags map to endpoints
Every git-cliff invocation already selects a contiguous slice of commits; the legacy flags are named shortcuts for picking the two endpoints of that slice, with inclusivity baked in.
--end-at HEAD(left defaults to first commit)--unreleased--start-after <last_tag> --end-at HEAD--latest--start-after <prev_tag> --end-at <last_tag>--current--start-after <prev_tag> --end-at <current_tag><A>..<B>--start-after A --end-at BConflicts and precedence
--start-atand--start-aftercannot be combined; same for--end-atand--end-before. This holds within the CLI and withincliff.tomlindependently.--start-ator--start-afterreplaces bothstart_atandstart_afterfrom config (so--start-at Xcleanly wins over a stalestart_after = "Y"incliff.toml); the right side behaves the same way.--latest,--current,--unreleased,--bump, positionalA..B). Pick one style.Previewing with
--dry-run--dry-runprints the computed interval, the number of commits it covers, and the git revision range that will be walked, then exits without rendering a changelog:Examples
Select a range with explicit endpoint inclusivity (
*-atis inclusive;*-after/*-beforeis exclusive):These options can also be set in
cliff.tomlunder[git]:Motivation and Context
This started from a discussion / research described in #655 .The legacy commit-range flags (
--latest,--current,--unreleased, positionalA..B) are named shortcuts that bundle endpoint location and inclusivity together:--unreleasedresolves to "after the last tag, up to and including HEAD",--latestto "after the previous tag, up to and including the last tag", and so on. Inclusivity is implicit, locked into each flag's semantics. Adding new endpoint kinds (Nth-most-recent-tag, dates, branch tips) would either expand that combinatorics or thread a parallel path through argument parsing, repo traversal, and release assembly.Math-interval notation factors location from inclusivity cleanly:
[X, Y]is bounded inclusive on both sides;(X, Y)is bounded exclusive;[X, Y)mixes them. Modeling aCommitRangeas two optionalEndpoints (each withrevandinclusive) captures any combination as a pure data value flowing through a transform -> resolve -> execute pipeline. Future endpoint kinds become pure follow-ups: new transforms thread through the same data path, no fan-out across the CLI / config / walker layers.--dry-runfalls out for free as "render the interval and the resolved revspec, then exit".Implications of this architectural change
See: this issue comment
No tracking issue.
How Has This Been Tested?
The collapsed block below records actual
--dry-runoutput across the verified scenarios on this very repo (git-cliff itself); the test plan after it lists what reviewers should rerun locally and what CI exercises.Verified end-to-end (click to expand)
Spot-checks that the four config keys flow through the same pipeline as their CLI counterparts, that the CLI overrides config (same-key, cross-key, and partial-side), and that the trickier walker shapes (right-only exclusive) resolve cleanly. All runs use
--dry-runso no changelog is written.TOML drives the range:
Flipping inclusivity on both sides:
Only one side set; the other falls back to its default:
Conflict detection fires from TOML alone:
CLI overrides config, same key. TOML pins the upper bound to
v2.13.0; the CLI bumps it tov2.13.1:CLI overrides config, cross-key (flips inclusivity). Config sets
start_after = "v2.12.0"(exclusive); CLI passes--start-at v2.12.0(inclusive). The CLI replaces the entire left side, so the resolved range becomes[v2.12.0, v2.13.0]and the commit count picks upv2.12.0itself:CLI overrides one side; config still drives the other. CLI moves the left endpoint back to
v2.11.0; the right endpoint stays where the config put it:Sanity:
git rev-list --count v2.12.0..v2.13.0is85,..v2.13.1is87,v2.12.0^..v2.13.0is86, andv2.11.0..v2.13.0is102, all matching the dry-run counts above.Right side only, exclusive. No left bound;
--end-beforegives an upper-exclusive range that walks back from the parent of the named tag:Compare with
--end-at v2.12.0for the inclusive sibling:[<first-commit>, v2.12.0]at 1461 commits (one more, picking upv2.12.0itself).Test plan
cargo test -- --skip "repo::test::git_upstream_remote"passescargo test --no-default-features -- --skip "repo::test::git_upstream_remote"passesresolve_withtests likecommand/repo)test-range-start-at-inclusive,test-range-end-before-exclusive, and the newtest-range-end-before-only(right-only-exclusive regression)git cliff --start-at <tag> --end-at <tag> --dry-runprints expected interval/commits/revspecgit cliff --end-before <tag> --dry-run(no left bound) prints[<first-commit>, <tag>)and resolves without error--latest,--current,--unreleased,A..B) still produce identical changelogs tomainScreenshots / Logs (if applicable)
See the collapsed "Verified end-to-end" block under How Has This Been Tested? above for the full set of
--dry-runlogs.Types of Changes
(Notes: the
--end-beforewalker fix is for in-PR code, not released behavior, so I left "Bug fix" unchecked. Every change here is additive or strictly more permissive: the new endpoint flags are new surface, theset_commit_rangewidening accepts a superset of what it used to, and legacy flag behavior is unchanged.)Checklist:
cargo +nightly fmt --all(verified clean via--check).cargo clippy --tests --verbose -- -D warnings— no new warnings frommain. (Pre-existingclippy::unnecessary_sort_byhits ingit-cliff-coreare present onmainand untouched by this branch.)