fix: honor explicit -n N limit for git log on merge commits#2015
Merged
Conversation
When user runs 'git log -1 --format='%H' HEAD' where HEAD is a merge commit, rtk was adding --no-merges which filtered out the merge commit itself and returned the second parent instead. This made 'git log -1' return wrong SHAs for merge commits. Fix: don't add --no-merges when user explicitly passes -n N or --max-count=N. When a user specifies an exact count they expect exactly that many commits, not filtered results. Also skip --no-merges if user already passed --merges or --no-merges explicitly. Fixes rtk-ai#2009.
|
|
Contributor
|
Hey, pretty straightforward Thanks for contributing to RTK ! |
aeppling
approved these changes
May 23, 2026
Merged
YOMXXX
added a commit
to YOMXXX/rtk
that referenced
this pull request
May 26, 2026
…ions Builds on the partial fix in rtk-ai#2015 (which honoured `-n N` / `--max-count=N`). rtk-ai#2015 closed rtk-ai#2009 but only covered count-based pinning; this PR extends the same idea to every remaining shape from the issue's reproduction: | invocation | upstream | this PR | |---------------------------------------|----------|---------| | `rtk git log` | default | default | `rtk git log -1 HEAD` | wrong* | fixed | `rtk git log -1` | default | default | `rtk git log --format=%H HEAD` | wrong* | fixed | `rtk git log <sha>` | wrong* | fixed | `rtk git log --graph` | wrong* | fixed | `rtk git log --graph --oneline` | wrong* | fixed | `rtk git log --merges` | wrong* | fixed (opt-in) | `rtk git log --min-parents=2` | wrong* | fixed (opt-in) | `rtk git log --author=foo` | default | default | `rtk git log --max-count=5` | fixed | fixed (via rtk-ai#2015) *wrong = `--no-merges` silently injected even though the user pinned a specific selection. On a `--no-ff` merge commit M (parents [P1, P2]), git log walks past the dropped merge and returns P2 — a real, recent, plausible, but wrong SHA. Caller can't tell the result is wrong because P2 looks valid. Root cause: `run_log` was unconditionally injecting `--no-merges` as a token-saving default. `--no-merges` is the right default only when RTK is fully driving the output (bare `git log`, no flags, no ref, no format). As soon as the user signals "I'm picking", the default has to stand down. This PR extracts `should_add_no_merges_default(args, has_format, has_limit)` as a pure helper. Returns `true` only when none of these signals are present: - `--format` / `--pretty` / `--oneline` (caller controls output shape) - `-N` / `-n N` / `--max-count=N` (caller picked a window — rtk-ai#2015) - `--graph` (caller wants topology) - any positional argument (ref / SHA / tag / pathspec) - explicit `--merges` / `--min-parents=2` (caller already opted in) `has_format` and `has_limit` are passed in from the caller's existing scan so we don't re-walk the args list. Tests (11 new): - test_no_merges_added_for_bare_git_log — token-saving default stays. - test_no_merges_skipped_with_explicit_limit_one + _only — covers `-1 HEAD` and bare `-1`, the headline reproduction. - test_no_merges_skipped_with_format_flag_and_ref + _explicit_sha + _graph_flag + _graph_and_oneline — covers every other shape the issue body called out as broken. - test_no_merges_skipped_with_explicit_merges + _min_parents — locks in the pre-existing opt-in semantics so the refactor doesn't regress it. - test_no_merges_still_added_with_author_filter — documents the deliberate narrow scope: filter flags that don't pick a single commit still benefit from the token-saving default. - test_no_merges_skipped_with_max_count_arg — short-circuit via has_limit (overlaps with rtk-ai#2015 to lock the contract in tests). cargo +1.93.0 fmt / clippy --all-targets / test --bin rtk: 1957 passed, 0 warnings. Refs rtk-ai#2009 (closed by rtk-ai#2015 for `-n N`; this completes the other shapes)
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.
No description provided.