* [PATCH v16 0/7] branch: delete-merged
From: Harald Nordgren via GitGitGadget @ 2026-06-18 19:25 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Johannes Sixt, Phillip Wood,
Harald Nordgren
In-Reply-To: <pull.2285.v15.git.git.1781542042.gitgitgadget@gmail.com>
Delete branches that have already been merged on upstream.
Changes in v16:
* Convert delete_merged_branches() to take an unsigned int flags argument
instead of separate quiet/dry_run booleans, matching delete_branches()
* Reuse the strbuf across the skip-config loop (strbuf_reset per iteration,
single strbuf_release after) instead of allocating and freeing it each
time
* Rewrite the --delete-merged tests as integration tests: branches that
land commits upstream, with deletion and the checked-out, upstream-gone,
and push-equals-upstream safety cases exercised together in one run and
output asserted via test_cmp
* Collapse the many per-aspect test repos into a single reused repo set up
by a setup_repo_for_delete_merged helper, and rename helpers off the old
pm_/prune naming
* Nest single-repo setup sequences in ( cd ... ) subshells instead of
prefixing every command with -C
Changes in v15:
* Renamed --prune-merged to --delete-merged throughout. Not necessarily
final, but something to advance the discussion.
* --delete-merged now silently skips not-yet-merged branches instead of
warning.
* Initialized the delete_branches() flag locals where declared. Only force
stays deferred.
* delete_branches()/check_branch_commit() doc and code cleanups: redundant
branch NULL checks dropped, ref_array candidates = { 0 }, a BUG() for the
unreachable non-branch ref, and reworked --delete-merged doc wording.
* Broadened the --forked tests (local commits for realism, remote add -f,
--forked coverage), renamed the misleading trunk fixture, and replaced
the misnamed detached branch with git checkout --detach.
Changes in v14:
* Fixed a git branch -d -r regression (broke t5404/t5505/t5514): the
remotes path set a local force but not the DELETE_BRANCH_FORCE bit that
check_branch_commit() reads, so it wrongly ran the merge check.
* Made flags the single source of truth in delete_branches() so the bit and
the derived locals can't disagree.
* Works locally, but GitHub CI has problems that are there for other
branches too, hopefully not related
(https://github.com/git/git/pull/2285).
Changes in v13:
* Reworked --forked into a real ref-filter applied in apply_ref_filter()
instead of a post-pass, so non-matching branches are never allocated.
* Match exact --forked patterns on full refnames (only globs use the
abbreviated upstream), and dropped the old helper machinery, forward
declaration, and string_list in favor of a strvec.
* Replaced the boolean parameters of
delete_branches()/check_branch_commit() with a single unsigned int flags.
* --prune-merged now collects candidates via filter_refs() rather than its
own branch walk.
* --prune-merged now takes its patterns as positional arguments (e.g. git
branch --prune-merged origin/main 'feature*') instead of repeating the
option.
Changes in v12:
* Reworked --forked from a standalone action into a --list-mode filter.
* Switched --forked and --prune-merged to repeatable OPT_STRING_LIST
options.
* Dropped the bare-remote-name resolution for --forked, the argument is now
a ref or a glob.
Changes in v11:
* The flags now take a branch, not a remote. --forked and --prune-merged
accept a literal upstream short name like origin/main or a wildmatch
pattern like origin/. The old --all-remotes flag is gone, since origin/
covers that case.
* The prune guard now compares @{push} against @{upstream}. A branch is
spared when these are equal. That is the trunk like case, such as local
main tracking and pushing to origin/main, where "fully merged to
upstream" cannot be told apart from "just pulled". Only branches that
push somewhere other than their upstream, typically fork based topics,
are candidates. The earlier /HEAD by name guard that the reviewer
rejected is gone.
* New --dry-run for --prune-merged.
Changes in v10:
* --forked / --prune-merged now take a branch glob instead of a remote name
— origin, origin/*, origin/release-- all work. This replaces the
remote-only form and subsumes the old --all-remotes flag, which has been
dropped.
* New --dry-run for --prune-merged.
Changes in v9:
* --force no longer has special meaning with --prune-merged; reachability
is always enforced. Use git branch -D to delete an unmerged branch.
Matches how git branch's other read/safe actions treat --force.
* Synopsis drops [-f]; "not fully merged" hint points at git branch -D.
* Dropped the --prune-merged --force tests.
Changes in v8:
* Delete only when the branch's work is actually reachable from its
upstream
* Skip branches whose upstream is gone (even with --force)
* Simplified the internal safety flag to live in one place
Changes in v7:
* --prune-merged now checks if a branch is merged into its own upstream
first. If the upstream is gone, it checks against the remote's default
branch instead. If neither exists, the branch is refused (use --force to
delete anyway).
Changes in v6:
* --prune-merged now measures merged-ness against the remote's default
branch instead of the candidate's upstream — so the decision no longer
depends on which branch happens to be checked out locally.
* delete_branches() / check_branch_commit() gained a per-candidate override
that lets a caller substitute a different "what counts as merged"
reference (or skip the check). branch -d callers pass NULL and keep their
existing semantics.
* prune_merged_branches() resolves each candidate's push-remote HEAD and
threads it through, so --prune-merged --all-remotes measures each
candidate against its own remote rather than a single global reference.
Changes in v5:
* Drop commit 'fetch: add --prune-merged'
Changes in v4:
* Resolve each remote's HEAD and collect the targets into a
protected_default_refs set in collect_forked_set.
* In prune_merged_branches, skip a candidate when its upstream is a
protected default ref and the local branch name matches the default
branch's leaf name (so a local main tracking origin/main is spared, but a
renamed trunk tracking origin/main is not).
* Also skip when the candidate's push ref points at a protected default
ref, so a topic branch configured to push to origin/main is never pruned.
* Tests: spare the local default branch; only protect by matching leaf name
(not by upstream alone); spare a branch whose push ref is the remote
default.
Changes in v3:
* s/remote-tracking refs/remote-tracking branches/g
Changes in v2:
* The whole feature moved out of git fetch and into git branch. git fetch
--prune-merged now just calls git branch --prune-merged after fetching.
* The fetch.pruneLocalBranches and remote..pruneLocalBranches config
options are gone, replaced by per-branch opt-out via branch..pruneMerged.
* New git branch --forked lists local branches whose upstream lives on the
given remote (read-only building block).
* New git branch --prune-merged deletes those branches, but only if their
tip is reachable from the upstream tracking ref; --force skips that
safety check.
* New git branch --all-remotes lets --forked/--prune-merged operate across
every configured remote at once.
* The currently checked-out branch in any worktree is always preserved.
* branch..pruneMerged=false lets you exempt a branch (e.g. a long-running
topic branch) even with --force; doesn't affect explicit git branch -d.
* delete_branches() got a warn_only mode so bulk deletion prints a one-line
warning per skipped branch instead of the noisy four-line hint that git
branch -d shows.
* New section in git-branch docs; git-fetch docs trimmed to just mention
--prune-merged.
* New tests in t3200-branch.sh for the new branch flags; t5510-fetch.sh
shrunk since most logic moved.
Harald Nordgren (7):
branch: add --forked filter for --list mode
branch: convert delete_branches() to a flags argument
branch: let delete_branches skip unmerged branches on bulk refusal
branch: prepare delete_branches for a bulk caller
branch: add --delete-merged <branch>
branch: add branch.<name>.deleteMerged opt-out
branch: add --dry-run for --delete-merged
Documentation/config/branch.adoc | 7 +
Documentation/git-branch.adoc | 43 ++++-
builtin/branch.c | 186 ++++++++++++++++++----
ref-filter.c | 70 +++++++++
ref-filter.h | 10 ++
t/t3200-branch.sh | 262 +++++++++++++++++++++++++++++++
6 files changed, 550 insertions(+), 28 deletions(-)
base-commit: 4621f8ce5e9b97aa2e8d0d9ffe9d25df2471074d
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2285%2FHaraldNordgren%2Ffetch-prune-local-branches-v16
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2285/HaraldNordgren/fetch-prune-local-branches-v16
Pull-Request: https://github.com/git/git/pull/2285
Range-diff vs v15:
1: da741b5ea7 ! 1: 1f6a758265 branch: add --forked filter for --list mode
@@ t/t3200-branch.sh: test_expect_success 'errors if given a bad branch name' '
+test_expect_success '--forked: setup' '
+ test_create_repo forked-upstream &&
-+ test_commit -C forked-upstream base &&
-+ git -C forked-upstream branch one base &&
-+ git -C forked-upstream branch two base &&
++ (
++ cd forked-upstream &&
++ test_commit base &&
++ git branch one base &&
++ git branch two base
++ ) &&
+
+ test_create_repo forked-other &&
-+ test_commit -C forked-other other-base &&
-+ git -C forked-other branch foreign other-base &&
++ (
++ cd forked-other &&
++ test_commit other-base &&
++ git branch foreign other-base
++ ) &&
+
+ git clone forked-upstream forked &&
-+ git -C forked remote add -f other ../forked-other &&
-+ git -C forked remote set-head origin one &&
-+ git -C forked branch local-base &&
-+ git -C forked branch --track local-one origin/one &&
-+ git -C forked branch --track local-two origin/two &&
-+ git -C forked branch --track local-foreign other/foreign &&
-+ git -C forked branch --track local-onbase local-base &&
++ (
++ cd forked &&
++ git remote add -f other ../forked-other &&
++ git remote set-head origin one &&
++ git branch local-base &&
++ git branch --track local-one origin/one &&
++ git branch --track local-two origin/two &&
++ git branch --track local-foreign other/foreign &&
++ git branch --track local-onbase local-base &&
+
-+ git -C forked checkout local-one &&
-+ test_commit -C forked --no-tag local-one-work local-one.t &&
-+ git -C forked checkout local-foreign &&
-+ test_commit -C forked --no-tag local-foreign-work local-foreign.t &&
-+ git -C forked checkout --detach
++ git checkout local-one &&
++ test_commit --no-tag local-one-work local-one.t &&
++ git checkout local-foreign &&
++ test_commit --no-tag local-foreign-work local-foreign.t &&
++ git checkout --detach
++ )
+'
+
+test_expect_success '--forked <upstream-tracking-branch> filters by upstream' '
2: 91c35f10cc = 2: 4f8af602ba branch: convert delete_branches() to a flags argument
3: e101dd2886 = 3: efc891c255 branch: let delete_branches skip unmerged branches on bulk refusal
4: 6c3534901a = 4: b1ecd38fe3 branch: prepare delete_branches for a bulk caller
5: 5899013b8f ! 5: 998fb6a68c branch: add --delete-merged <branch>
@@ builtin/branch.c: static int parse_opt_forked(const struct option *opt, const ch
}
+static int delete_merged_branches(int argc, const char **argv,
-+ int quiet)
++ unsigned int flags)
+{
+ struct ref_store *refs = get_main_ref_store(the_repository);
+ struct ref_filter filter = REF_FILTER_INIT;
@@ builtin/branch.c: static int parse_opt_forked(const struct option *opt, const ch
+ FILTER_REFS_BRANCHES,
+ DELETE_BRANCH_SKIP_UNMERGED |
+ DELETE_BRANCH_NO_HEAD_FALLBACK |
-+ (quiet ? DELETE_BRANCH_QUIET : 0));
++ flags);
+
+ strvec_clear(&deletable);
+ ref_array_clear(&candidates);
@@ builtin/branch.c: int cmd_branch(int argc,
(quiet ? DELETE_BRANCH_QUIET : 0));
goto out;
+ } else if (delete_merged) {
-+ ret = delete_merged_branches(argc, argv, quiet);
++ ret = delete_merged_branches(argc, argv,
++ quiet ? DELETE_BRANCH_QUIET : 0);
+ goto out;
} else if (show_current) {
print_current_branch_name();
@@ t/t3200-branch.sh: test_expect_success '--forked narrows a <pattern> argument' '
'
+test_expect_success '--delete-merged: setup' '
-+ test_create_repo pm-upstream &&
-+ test_commit -C pm-upstream base &&
-+ git -C pm-upstream checkout -b next &&
-+ test_commit -C pm-upstream one-commit &&
-+ test_commit -C pm-upstream two-commit &&
-+ git -C pm-upstream branch one HEAD~ &&
-+ git -C pm-upstream branch two HEAD &&
-+ git -C pm-upstream branch wip main &&
-+ git -C pm-upstream checkout main &&
-+ test_create_repo pm-fork
++ git init -b main upstream &&
++ (
++ cd upstream &&
++ test_commit base &&
++ git checkout -b next &&
++ test_commit next-work &&
++ git checkout main
++ ) &&
++ git init -b main other &&
++ test_commit -C other other-base &&
++ git init -b main fork
+'
+
-+test_expect_success '--delete-merged deletes branches integrated into upstream' '
-+ test_when_finished "rm -rf pm-merged" &&
-+ git clone pm-upstream pm-merged &&
-+ git -C pm-merged remote add fork ../pm-fork &&
-+ test_config -C pm-merged remote.pushDefault fork &&
-+ test_config -C pm-merged push.default current &&
-+ git -C pm-merged branch one one-commit &&
-+ git -C pm-merged branch --set-upstream-to=origin/next one &&
-+ git -C pm-merged branch two two-commit &&
-+ git -C pm-merged branch --set-upstream-to=origin/next two &&
-+
-+ git -C pm-merged branch --delete-merged "origin/*" &&
-+
-+ test_must_fail git -C pm-merged rev-parse --verify refs/heads/one &&
-+ test_must_fail git -C pm-merged rev-parse --verify refs/heads/two
-+'
-+
-+test_expect_success '--delete-merged accepts a literal upstream' '
-+ test_when_finished "rm -rf pm-literal" &&
-+ git clone pm-upstream pm-literal &&
-+ git -C pm-literal remote add fork ../pm-fork &&
-+ test_config -C pm-literal remote.pushDefault fork &&
-+ test_config -C pm-literal push.default current &&
-+ git -C pm-literal branch one one-commit &&
-+ git -C pm-literal branch --set-upstream-to=origin/next one &&
-+
-+ git -C pm-literal branch --delete-merged origin/next &&
-+
-+ test_must_fail git -C pm-literal rev-parse --verify refs/heads/one
-+'
-+
-+test_expect_success '--delete-merged unions multiple <branch> arguments' '
-+ test_when_finished "rm -rf pm-union" &&
-+ git clone pm-upstream pm-union &&
-+ git -C pm-union remote add fork ../pm-fork &&
-+ test_config -C pm-union remote.pushDefault fork &&
-+ test_config -C pm-union push.default current &&
-+ git -C pm-union branch one one-commit &&
-+ git -C pm-union branch --set-upstream-to=origin/next one &&
-+ git -C pm-union branch two base &&
-+ git -C pm-union branch --set-upstream-to=origin/main two &&
-+ git -C pm-union checkout --detach &&
-+
-+ git -C pm-union branch --delete-merged origin/next origin/main &&
-+
-+ test_must_fail git -C pm-union rev-parse --verify refs/heads/one &&
-+ test_must_fail git -C pm-union rev-parse --verify refs/heads/two
-+'
-+
-+test_expect_success '--delete-merged accepts a local upstream' '
-+ test_when_finished "rm -rf pm-local" &&
-+ git clone pm-upstream pm-local &&
-+ git -C pm-local remote add fork ../pm-fork &&
-+ test_config -C pm-local remote.pushDefault fork &&
-+ test_config -C pm-local push.default current &&
-+ git -C pm-local checkout -b mainline &&
-+ git -C pm-local branch one one-commit &&
-+ git -C pm-local branch --set-upstream-to=mainline one &&
-+ git -C pm-local merge --ff-only one-commit &&
-+
-+ git -C pm-local branch --delete-merged mainline &&
-+
-+ test_must_fail git -C pm-local rev-parse --verify refs/heads/one
-+'
-+
-+test_expect_success '--delete-merged silently skips un-integrated commits' '
-+ test_when_finished "rm -rf pm-unmerged" &&
-+ git clone pm-upstream pm-unmerged &&
-+ git -C pm-unmerged remote add fork ../pm-fork &&
-+ test_config -C pm-unmerged remote.pushDefault fork &&
-+ test_config -C pm-unmerged push.default current &&
-+ git -C pm-unmerged checkout -b wip origin/wip &&
-+ git -C pm-unmerged branch --set-upstream-to=origin/next wip &&
-+ test_commit -C pm-unmerged local-only &&
-+ git -C pm-unmerged checkout - &&
-+
-+ git -C pm-unmerged branch --delete-merged "origin/*" 2>err &&
-+ test_grep ! "not fully merged" err &&
-+ git -C pm-unmerged rev-parse --verify refs/heads/wip
-+'
-+
-+test_expect_success '--delete-merged is silent about not-merged-to-HEAD' '
-+ test_when_finished "rm -rf pm-nohead" &&
-+ git clone pm-upstream pm-nohead &&
-+ git -C pm-nohead remote add fork ../pm-fork &&
-+ test_config -C pm-nohead remote.pushDefault fork &&
-+ test_config -C pm-nohead push.default current &&
-+ git -C pm-nohead branch topic one-commit &&
-+ git -C pm-nohead branch --set-upstream-to=origin/next topic &&
-+
-+ git -C pm-nohead branch --delete-merged "origin/*" 2>err &&
-+
-+ test_grep ! "not yet merged to HEAD" err &&
-+ test_must_fail git -C pm-nohead rev-parse --verify refs/heads/topic
-+'
-+
-+test_expect_success '--delete-merged skips branches whose upstream is gone' '
-+ test_when_finished "rm -rf pm-upstream-gone" &&
-+ git clone pm-upstream pm-upstream-gone &&
-+ git -C pm-upstream-gone remote add fork ../pm-fork &&
-+ test_config -C pm-upstream-gone remote.pushDefault fork &&
-+ test_config -C pm-upstream-gone push.default current &&
-+ git -C pm-upstream-gone branch one one-commit &&
-+ git -C pm-upstream-gone branch --set-upstream-to=origin/next one &&
-+
-+ git -C pm-upstream-gone update-ref -d refs/remotes/origin/next &&
-+ git -C pm-upstream-gone branch --delete-merged "origin/*" &&
-+
-+ git -C pm-upstream-gone rev-parse --verify refs/heads/one
-+'
-+
-+test_expect_success '--delete-merged never deletes the checked-out branch' '
-+ test_when_finished "rm -rf pm-head" &&
-+ git clone pm-upstream pm-head &&
-+ git -C pm-head remote add fork ../pm-fork &&
-+ test_config -C pm-head remote.pushDefault fork &&
-+ test_config -C pm-head push.default current &&
-+ git -C pm-head checkout -b one one-commit &&
-+ git -C pm-head branch --set-upstream-to=origin/next one &&
-+
-+ git -C pm-head branch --delete-merged "origin/*" &&
-+
-+ git -C pm-head rev-parse --verify refs/heads/one
-+'
-+
-+test_expect_success '--delete-merged spares branches that push back to their upstream' '
-+ test_when_finished "rm -rf pm-push-eq" &&
-+ git clone pm-upstream pm-push-eq &&
-+ git -C pm-push-eq checkout --detach &&
-+
-+ git -C pm-push-eq branch --delete-merged "origin/*" &&
-+
-+ git -C pm-push-eq rev-parse --verify refs/heads/main
-+'
-+
-+test_expect_success '--delete-merged spares a per-branch pushRemote==upstream remote' '
-+ test_when_finished "rm -rf pm-push-branch" &&
-+ git clone pm-upstream pm-push-branch &&
-+ git -C pm-push-branch remote add fork ../pm-fork &&
-+ test_config -C pm-push-branch remote.pushDefault fork &&
-+ test_config -C pm-push-branch push.default current &&
-+ test_config -C pm-push-branch branch.main.pushRemote origin &&
-+ git -C pm-push-branch checkout --detach &&
++setup_repo_for_delete_merged () {
++ rm -rf repo &&
++ git clone upstream repo &&
++ (
++ cd repo &&
++ git remote add fork ../fork &&
++ git remote add other ../other &&
++ git config remote.pushDefault fork &&
++ git config push.default current &&
++ git fetch other
++ )
++}
+
-+ git -C pm-push-branch branch --delete-merged "origin/*" &&
++merged_branch () {
++ (
++ cd repo &&
++ git checkout -b "$1" "$2" &&
++ git commit --allow-empty -m "$1 work" &&
++ git push origin "$1:next" &&
++ git fetch origin &&
++ git branch --set-upstream-to="$2" "$1"
++ )
++}
+
-+ git -C pm-push-branch rev-parse --verify refs/heads/main
++test_expect_success '--delete-merged deletes merged branches and spares the rest' '
++ test_when_finished "rm -rf repo" &&
++ setup_repo_for_delete_merged &&
++ merged_branch merged origin/next &&
++ (
++ cd repo &&
++ git checkout -b unmerged origin/next &&
++ git commit --allow-empty -m "unmerged work" &&
++ git branch --set-upstream-to=origin/next unmerged &&
++ git checkout -b tracks-other other/main &&
++ git branch --set-upstream-to=other/main tracks-other &&
++ git checkout --detach
++ ) &&
++ sha=$(git -C repo rev-parse --short merged) &&
++
++ git -C repo branch --delete-merged origin/next >actual 2>&1 &&
++
++ echo "Deleted branch merged (was $sha)." >expect &&
++ test_cmp expect actual &&
++ git -C repo for-each-ref --format="%(refname:short)" refs/heads/ >actual &&
++ cat >expect <<-\EOF &&
++ main
++ tracks-other
++ unmerged
++ EOF
++ test_cmp expect actual
+'
+
-+test_expect_success '--delete-merged prunes when @{push} differs from @{upstream}' '
-+ test_when_finished "rm -rf pm-push-diff" &&
-+ git clone pm-upstream pm-push-diff &&
-+ git -C pm-push-diff remote add fork ../pm-fork &&
-+ test_config -C pm-push-diff remote.pushDefault fork &&
-+ test_config -C pm-push-diff push.default current &&
-+ git -C pm-push-diff branch topic one-commit &&
-+ git -C pm-push-diff branch --set-upstream-to=origin/next topic &&
-+ git -C pm-push-diff checkout --detach &&
-+
-+ git -C pm-push-diff branch --delete-merged "origin/*" &&
-+
-+ test_must_fail git -C pm-push-diff rev-parse --verify refs/heads/topic
++test_expect_success '--delete-merged deletes merged branches and spares protected ones' '
++ test_when_finished "rm -rf repo" &&
++ setup_repo_for_delete_merged &&
++ merged_branch on-next origin/next &&
++ merged_branch checked-out origin/next &&
++ merged_branch upstream-gone origin/next &&
++ (
++ cd repo &&
++ git checkout -b mainline main &&
++ git checkout -b on-local mainline &&
++ git branch --set-upstream-to=mainline on-local &&
++ git update-ref refs/remotes/origin/topic refs/remotes/origin/next &&
++ git branch --set-upstream-to=origin/topic upstream-gone &&
++ git update-ref -d refs/remotes/origin/topic &&
++ git branch --set-upstream-to=origin/main main &&
++ git config branch.main.pushRemote origin &&
++ git checkout -b tracks-other other/main &&
++ git branch --set-upstream-to=other/main tracks-other &&
++ git checkout checked-out
++ ) &&
++
++ git -C repo branch --delete-merged origin/next mainline &&
++
++ git -C repo for-each-ref --format="%(refname:short)" refs/heads/ >actual &&
++ cat >expect <<-\EOF &&
++ checked-out
++ main
++ mainline
++ tracks-other
++ upstream-gone
++ EOF
++ test_cmp expect actual
+'
+
+test_expect_success '--delete-merged requires at least one <branch>' '
+ test_must_fail git -C forked branch --delete-merged 2>err &&
+ test_grep "requires at least one <branch>" err
+'
-+
-+test_expect_success '--delete-merged takes positional <branch> arguments' '
-+ test_when_finished "rm -rf pm-positional" &&
-+ git clone pm-upstream pm-positional &&
-+ git -C pm-positional remote add fork ../pm-fork &&
-+ test_config -C pm-positional remote.pushDefault fork &&
-+ test_config -C pm-positional push.default current &&
-+ git -C pm-positional branch one one-commit &&
-+ git -C pm-positional branch --set-upstream-to=origin/next one &&
-+ git -C pm-positional branch two base &&
-+ git -C pm-positional branch --set-upstream-to=origin/main two &&
-+ git -C pm-positional checkout --detach &&
-+
-+ git -C pm-positional branch --delete-merged origin/next origin/main &&
-+
-+ test_must_fail git -C pm-positional rev-parse --verify refs/heads/one &&
-+ test_must_fail git -C pm-positional rev-parse --verify refs/heads/two
-+'
+
test_done
6: 72aaca0666 ! 6: a27d2724a2 branch: add branch.<name>.deleteMerged opt-out
@@ Documentation/git-branch.adoc: A branch is not deleted when:
A branch whose work has not yet been merged into its upstream is
## builtin/branch.c ##
+@@ builtin/branch.c: static int delete_merged_branches(int argc, const char **argv,
+ struct ref_filter filter = REF_FILTER_INIT;
+ struct ref_array candidates = { 0 };
+ struct strvec deletable = STRVEC_INIT;
++ struct strbuf key = STRBUF_INIT;
++ bool quiet = flags & DELETE_BRANCH_QUIET;
+ int i, ret = 0;
+
+ if (!argc)
@@ builtin/branch.c: static int delete_merged_branches(int argc, const char **argv,
const char *short_name;
struct branch *branch;
const char *upstream, *push;
-+ struct strbuf key = STRBUF_INIT;
+ int opt_out;
if (!skip_prefix(full_name, "refs/heads/", &short_name))
@@ builtin/branch.c: static int delete_merged_branches(int argc, const char **argv,
if (!push || !strcmp(push, upstream))
continue;
++ strbuf_reset(&key);
+ strbuf_addf(&key, "branch.%s.deletemerged", short_name);
+ if (!repo_config_get_bool(the_repository, key.buf, &opt_out) &&
+ !opt_out) {
@@ builtin/branch.c: static int delete_merged_branches(int argc, const char **argv,
+ fprintf(stderr,
+ _("Skipping '%s' (branch.%s.deleteMerged is false)\n"),
+ short_name, short_name);
-+ strbuf_release(&key);
+ continue;
+ }
-+ strbuf_release(&key);
+
strvec_push(&deletable, short_name);
}
+@@ builtin/branch.c: static int delete_merged_branches(int argc, const char **argv,
+ DELETE_BRANCH_NO_HEAD_FALLBACK |
+ flags);
+
++ strbuf_release(&key);
+ strvec_clear(&deletable);
+ ref_array_clear(&candidates);
+ ref_filter_clear(&filter);
## t/t3200-branch.sh ##
-@@ t/t3200-branch.sh: test_expect_success '--delete-merged takes positional <branch> arguments' '
- test_must_fail git -C pm-positional rev-parse --verify refs/heads/two
+@@ t/t3200-branch.sh: test_expect_success '--delete-merged requires at least one <branch>' '
+ test_grep "requires at least one <branch>" err
'
+test_expect_success '--delete-merged honours branch.<name>.deleteMerged=false' '
-+ test_when_finished "rm -rf pm-optout" &&
-+ git clone pm-upstream pm-optout &&
-+ git -C pm-optout remote add fork ../pm-fork &&
-+ test_config -C pm-optout remote.pushDefault fork &&
-+ test_config -C pm-optout push.default current &&
-+ git -C pm-optout branch one one-commit &&
-+ git -C pm-optout branch --set-upstream-to=origin/next one &&
-+ git -C pm-optout branch two two-commit &&
-+ git -C pm-optout branch --set-upstream-to=origin/next two &&
-+ test_config -C pm-optout branch.one.deleteMerged false &&
++ test_when_finished "rm -rf repo" &&
++ setup_repo_for_delete_merged &&
++ merged_branch deleted origin/next &&
++ merged_branch kept origin/next &&
++ git -C repo config branch.kept.deleteMerged false &&
++ git -C repo checkout --detach &&
+
-+ git -C pm-optout branch --delete-merged "origin/*" 2>err &&
++ git -C repo branch --delete-merged origin/next 2>err &&
+
-+ git -C pm-optout rev-parse --verify refs/heads/one &&
-+ test_must_fail git -C pm-optout rev-parse --verify refs/heads/two &&
-+ test_grep "Skipping .one." err
++ test_grep "Skipping .kept." err &&
++ test_must_fail git -C repo rev-parse --verify refs/heads/deleted &&
++ git -C repo rev-parse --verify refs/heads/kept
+'
+
-+test_expect_success 'branch -d still deletes a deleteMerged=false branch' '
-+ test_when_finished "rm -rf pm-optout-d" &&
-+ git clone pm-upstream pm-optout-d &&
-+ git -C pm-optout-d branch one one-commit &&
-+ git -C pm-optout-d branch --set-upstream-to=origin/next one &&
-+ test_config -C pm-optout-d branch.one.deleteMerged false &&
++test_expect_success "branch -d still deletes a deleteMerged=false branch" '
++ test_when_finished "rm -rf repo" &&
++ setup_repo_for_delete_merged &&
++ merged_branch kept origin/next &&
++ git -C repo config branch.kept.deleteMerged false &&
++ git -C repo checkout --detach &&
+
-+ git -C pm-optout-d branch -d one &&
-+ test_must_fail git -C pm-optout-d rev-parse --verify refs/heads/one
++ git -C repo branch -d kept &&
++ test_must_fail git -C repo rev-parse --verify refs/heads/kept
+'
+
test_done
7: 7b2b01b988 ! 7: 6d5c52353e branch: add --dry-run for --delete-merged
@@ Documentation/git-branch.adoc: A branch whose work has not yet been merged into
`--verbose`::
## builtin/branch.c ##
-@@ builtin/branch.c: static int parse_opt_forked(const struct option *opt, const char *arg, int unset
- }
-
- static int delete_merged_branches(int argc, const char **argv,
-- int quiet)
-+ int quiet, int dry_run)
- {
- struct ref_store *refs = get_main_ref_store(the_repository);
- struct ref_filter filter = REF_FILTER_INIT;
-@@ builtin/branch.c: static int delete_merged_branches(int argc, const char **argv,
- FILTER_REFS_BRANCHES,
- DELETE_BRANCH_SKIP_UNMERGED |
- DELETE_BRANCH_NO_HEAD_FALLBACK |
-- (quiet ? DELETE_BRANCH_QUIET : 0));
-+ (quiet ? DELETE_BRANCH_QUIET : 0) |
-+ (dry_run ? DELETE_BRANCH_DRY_RUN : 0));
-
- strvec_clear(&deletable);
- ref_array_clear(&candidates);
@@ builtin/branch.c: int cmd_branch(int argc,
int delete = 0, rename = 0, copy = 0, list = 0,
unset_upstream = 0, show_current = 0, edit_description = 0;
@@ builtin/branch.c: int cmd_branch(int argc,
if (!submodule_propagate_branches)
die(_("branch with --recurse-submodules can only be used if submodule.propagateBranches is enabled"));
@@ builtin/branch.c: int cmd_branch(int argc,
- (quiet ? DELETE_BRANCH_QUIET : 0));
goto out;
} else if (delete_merged) {
-- ret = delete_merged_branches(argc, argv, quiet);
-+ ret = delete_merged_branches(argc, argv, quiet, dry_run);
+ ret = delete_merged_branches(argc, argv,
+- quiet ? DELETE_BRANCH_QUIET : 0);
++ (quiet ? DELETE_BRANCH_QUIET : 0) |
++ (dry_run ? DELETE_BRANCH_DRY_RUN : 0));
goto out;
} else if (show_current) {
print_current_branch_name();
## t/t3200-branch.sh ##
-@@ t/t3200-branch.sh: test_expect_success 'branch -d still deletes a deleteMerged=false branch' '
- test_must_fail git -C pm-optout-d rev-parse --verify refs/heads/one
+@@ t/t3200-branch.sh: test_expect_success '--delete-merged deletes merged branches and spares the rest
+ ) &&
+ sha=$(git -C repo rev-parse --short merged) &&
+
+- git -C repo branch --delete-merged origin/next >actual 2>&1 &&
++ git -C repo branch --dry-run --delete-merged origin/next >actual 2>&1 &&
++ echo "Would delete branch merged (was $sha)." >expect &&
++ test_cmp expect actual &&
++ git -C repo rev-parse --verify refs/heads/merged &&
+
++ git -C repo branch --delete-merged origin/next >actual 2>&1 &&
+ echo "Deleted branch merged (was $sha)." >expect &&
+ test_cmp expect actual &&
+ git -C repo for-each-ref --format="%(refname:short)" refs/heads/ >actual &&
+@@ t/t3200-branch.sh: test_expect_success "branch -d still deletes a deleteMerged=false branch" '
+ test_must_fail git -C repo rev-parse --verify refs/heads/kept
'
-+test_expect_success '--delete-merged --dry-run lists but does not delete' '
-+ test_when_finished "rm -rf pm-dry" &&
-+ git clone pm-upstream pm-dry &&
-+ git -C pm-dry remote add fork ../pm-fork &&
-+ test_config -C pm-dry remote.pushDefault fork &&
-+ test_config -C pm-dry push.default current &&
-+ git -C pm-dry branch one one-commit &&
-+ git -C pm-dry branch --set-upstream-to=origin/next one &&
-+ git -C pm-dry branch two two-commit &&
-+ git -C pm-dry branch --set-upstream-to=origin/next two &&
-+
-+ git -C pm-dry branch --dry-run --delete-merged "origin/*" >actual &&
-+ test_grep "Would delete branch one " actual &&
-+ test_grep "Would delete branch two " actual &&
-+
-+ git -C pm-dry rev-parse --verify refs/heads/one &&
-+ git -C pm-dry rev-parse --verify refs/heads/two
-+'
-+
-+test_expect_success '--delete-merged --dry-run only lists branches the live run would delete' '
-+ test_when_finished "rm -rf pm-dry-mixed" &&
-+ git clone pm-upstream pm-dry-mixed &&
-+ git -C pm-dry-mixed remote add fork ../pm-fork &&
-+ test_config -C pm-dry-mixed remote.pushDefault fork &&
-+ test_config -C pm-dry-mixed push.default current &&
-+ git -C pm-dry-mixed checkout -b wip origin/next &&
-+ git -C pm-dry-mixed branch --set-upstream-to=origin/next wip &&
-+ test_commit -C pm-dry-mixed local-only &&
-+ git -C pm-dry-mixed checkout - &&
-+ git -C pm-dry-mixed branch merged one-commit &&
-+ git -C pm-dry-mixed branch --set-upstream-to=origin/next merged &&
-+
-+ git -C pm-dry-mixed branch --dry-run --delete-merged "origin/*" >out &&
-+ test_grep "Would delete branch merged" out &&
-+ test_grep ! "Would delete branch wip" out &&
-+ git -C pm-dry-mixed rev-parse --verify refs/heads/wip &&
-+ git -C pm-dry-mixed rev-parse --verify refs/heads/merged
-+'
-+
+test_expect_success '--dry-run without --delete-merged is rejected' '
+ test_must_fail git -C forked branch --dry-run 2>err &&
+ test_grep "requires --delete-merged" err
--
gitgitgadget
^ permalink raw reply
* [PATCH v3 4/4] history: re-edit a squash with every message
From: Harald Nordgren via GitGitGadget @ 2026-06-18 19:17 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2337.v3.git.git.1781810226.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
By default "git history squash" reuses the oldest commit's message.
When --reedit-message is given it only reopened that one message, so the
messages of the folded-in commits were lost.
Gather the messages of every commit in the range, oldest first, and use
them as the editor template when re-editing, mirroring how "git rebase
-i" presents a squash. The combined message is built before the
descendant walk so it is not disturbed by the flags that walk leaves on
the commits.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
Documentation/git-history.adoc | 5 +--
builtin/history.c | 61 +++++++++++++++++++++++++++++++++-
t/t3454-history-squash.sh | 37 +++++++++++++++++++++
3 files changed, 100 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-history.adoc b/Documentation/git-history.adoc
index d3a5ad28a3..dd3544832d 100644
--- a/Documentation/git-history.adoc
+++ b/Documentation/git-history.adoc
@@ -111,8 +111,9 @@ history squash @~3..` folds the three most recent commits into one, and
`git history squash @~5..@~2` squashes an interior range while leaving
the two newest commits in place.
+
-The oldest commit's message and authorship are preserved by default,
-unless you specify `--reedit-message`. A merge commit inside the range is
+The oldest commit's message and authorship are preserved by default. With
+`--reedit-message`, an editor opens pre-filled with the messages of all the
+folded commits so you can combine them. A merge commit inside the range is
folded like any other, but the range must have a single base, so a range
that reaches more than one entry point (for example a side branch that
forked before the range and was later merged into it) is rejected.
diff --git a/builtin/history.c b/builtin/history.c
index 9d9416870f..eb12a5d7e8 100644
--- a/builtin/history.c
+++ b/builtin/history.c
@@ -1047,6 +1047,56 @@ out:
return ret;
}
+static int build_squash_message(struct repository *repo,
+ struct commit *base,
+ struct commit *tip,
+ struct strbuf *out)
+{
+ struct rev_info revs;
+ struct commit *commit;
+ struct strvec args = STRVEC_INIT;
+ int n = 0, ret;
+
+ repo_init_revisions(repo, &revs, NULL);
+ strvec_push(&args, "ignored");
+ strvec_push(&args, "--reverse");
+ strvec_push(&args, "--topo-order");
+ strvec_pushf(&args, "%s..%s", oid_to_hex(&base->object.oid),
+ oid_to_hex(&tip->object.oid));
+ setup_revisions_from_strvec(&args, &revs, NULL);
+
+ if (prepare_revision_walk(&revs) < 0) {
+ ret = error(_("error preparing revisions"));
+ goto out;
+ }
+
+ while ((commit = get_revision(&revs))) {
+ const char *message, *body;
+ struct strbuf one = STRBUF_INIT;
+
+ message = repo_logmsg_reencode(repo, commit, NULL, NULL);
+ find_commit_subject(message, &body);
+ strbuf_addstr(&one, body);
+ strbuf_trim_trailing_newline(&one);
+
+ if (n++)
+ strbuf_addch(out, '\n');
+ strbuf_addbuf(out, &one);
+ strbuf_addch(out, '\n');
+
+ strbuf_release(&one);
+ repo_unuse_commit_buffer(repo, commit, message);
+ }
+
+ ret = 0;
+
+out:
+ reset_revision_walk();
+ release_revisions(&revs);
+ strvec_clear(&args);
+ return ret;
+}
+
static int cmd_history_squash(int argc,
const char **argv,
const char *prefix,
@@ -1071,6 +1121,7 @@ static int cmd_history_squash(int argc,
OPT_END(),
};
struct strbuf reflog_msg = STRBUF_INIT;
+ struct strbuf message = STRBUF_INIT;
struct commit *base, *oldest, *tip, *rewritten;
const struct object_id *base_tree_oid, *tip_tree_oid;
struct commit_list *parents = NULL;
@@ -1091,6 +1142,12 @@ static int cmd_history_squash(int argc,
if (ret < 0)
goto out;
+ if (flags & COMMIT_TREE_EDIT_MESSAGE) {
+ ret = build_squash_message(repo, base, tip, &message);
+ if (ret < 0)
+ goto out;
+ }
+
ret = setup_revwalk(repo, action, tip, &revs);
if (ret < 0)
goto out;
@@ -1099,7 +1156,8 @@ static int cmd_history_squash(int argc,
tip_tree_oid = &repo_get_commit_tree(repo, tip)->object.oid;
commit_list_append(base, &parents);
- ret = commit_tree_ext(repo, "squash", oldest, NULL, parents,
+ ret = commit_tree_ext(repo, "squash", oldest,
+ message.len ? message.buf : NULL, parents,
base_tree_oid, tip_tree_oid, &rewritten, flags);
if (ret < 0) {
ret = error(_("failed writing squashed commit"));
@@ -1120,6 +1178,7 @@ static int cmd_history_squash(int argc,
out:
strbuf_release(&reflog_msg);
+ strbuf_release(&message);
commit_list_free(parents);
release_revisions(&revs);
return ret;
diff --git a/t/t3454-history-squash.sh b/t/t3454-history-squash.sh
index 6c6a75bf00..1edd148295 100755
--- a/t/t3454-history-squash.sh
+++ b/t/t3454-history-squash.sh
@@ -135,6 +135,43 @@ test_expect_success 'preserves authorship of the oldest commit' '
test_cmp expect actual
'
+test_expect_success '--reedit-message offers every folded-in message' '
+ git reset --hard start &&
+ echo b >file &&
+ git add file &&
+ git commit -m "re-one subject" -m "re-one body line" &&
+ test_commit re-two file c &&
+ test_commit re-three file d &&
+
+ write_script editor <<-\EOF &&
+ cp "$1" buffer &&
+ echo combined >"$1"
+ EOF
+ test_set_editor "$(pwd)/editor" &&
+ git history squash --reedit-message start.. &&
+
+ grep "re-one subject" buffer &&
+ grep "re-one body line" buffer &&
+ grep re-two buffer &&
+ grep re-three buffer &&
+ git log --format="%s" -1 >actual &&
+ echo combined >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success '--reedit-message aborts on an empty message' '
+ git reset --hard three &&
+ head_before=$(git rev-parse HEAD) &&
+
+ write_script editor <<-\EOF &&
+ >"$1"
+ EOF
+ test_set_editor "$(pwd)/editor" &&
+ test_must_fail git history squash --reedit-message start.. &&
+
+ test_cmp_rev "$head_before" HEAD
+'
+
test_expect_success '--dry-run predicts the rewrite without performing it' '
git reset --hard three &&
head_before=$(git rev-parse HEAD) &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 3/4] history: add squash subcommand to fold a range
From: Harald Nordgren via GitGitGadget @ 2026-06-18 19:17 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2337.v3.git.git.1781810226.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
Folding a series of commits into one required either an interactive
rebase where each commit after the first was hand-edited to "fixup", or
a "git reset --soft" to the merge base followed by "git commit --amend".
Add "git history squash <revision-range>" to do this directly. It folds
every commit in the range into the oldest one, keeping that commit's
message and authorship and taking the tree of the newest commit, so the
range collapses into a single commit. Commits above the range are
replayed on top of the result.
The range is given as <base>..<tip>, so "git history squash @~3.."
folds the three most recent commits and "git history squash @~5..@~2"
squashes an interior range. A merge inside the range is folded like any
other commit, but the range must have a single base, so a range with
more than one entry point is rejected.
Inspired-by: Sergey Chernov <serega.morph@gmail.com>
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
Documentation/git-history.adoc | 20 ++++
builtin/history.c | 154 ++++++++++++++++++++++++
t/meson.build | 1 +
t/t3454-history-squash.sh | 213 +++++++++++++++++++++++++++++++++
4 files changed, 388 insertions(+)
create mode 100755 t/t3454-history-squash.sh
diff --git a/Documentation/git-history.adoc b/Documentation/git-history.adoc
index 2ba8121795..d3a5ad28a3 100644
--- a/Documentation/git-history.adoc
+++ b/Documentation/git-history.adoc
@@ -11,6 +11,7 @@ SYNOPSIS
git history fixup <commit> [--dry-run] [--update-refs=(branches|head)] [--reedit-message] [--empty=(drop|keep|abort)]
git history reword <commit> [--dry-run] [--update-refs=(branches|head)]
git history split <commit> [--dry-run] [--update-refs=(branches|head)] [--] [<pathspec>...]
+git history squash <revision-range> [--dry-run] [--update-refs=(branches|head)] [--reedit-message]
DESCRIPTION
-----------
@@ -97,6 +98,25 @@ linkgit:gitglossary[7].
It is invalid to select either all or no hunks, as that would lead to
one of the commits becoming empty.
+`squash <revision-range>`::
+ Fold all commits in _<revision-range>_ into the oldest commit of that
+ range. The resulting commit keeps the oldest commit's message and
+ authorship and takes the tree of the range's newest commit, so the
+ whole range collapses into a single commit. Commits above the range
+ are replayed on top of the result.
++
+The range is given in the usual `<base>..<tip>` form, where _<base>_ is
+the commit just below the oldest commit to squash. For example, `git
+history squash @~3..` folds the three most recent commits into one, and
+`git history squash @~5..@~2` squashes an interior range while leaving
+the two newest commits in place.
++
+The oldest commit's message and authorship are preserved by default,
+unless you specify `--reedit-message`. A merge commit inside the range is
+folded like any other, but the range must have a single base, so a range
+that reaches more than one entry point (for example a side branch that
+forked before the range and was later merged into it) is rejected.
+
OPTIONS
-------
diff --git a/builtin/history.c b/builtin/history.c
index 305bde3102..9d9416870f 100644
--- a/builtin/history.c
+++ b/builtin/history.c
@@ -30,6 +30,8 @@
N_("git history reword <commit> [--dry-run] [--update-refs=(branches|head)]")
#define GIT_HISTORY_SPLIT_USAGE \
N_("git history split <commit> [--dry-run] [--update-refs=(branches|head)] [--] [<pathspec>...]")
+#define GIT_HISTORY_SQUASH_USAGE \
+ N_("git history squash <revision-range> [--dry-run] [--update-refs=(branches|head)] [--reedit-message]")
static void change_data_free(void *util, const char *str UNUSED)
{
@@ -973,6 +975,156 @@ out:
return ret;
}
+/*
+ * Resolve a "<base>..<tip>" revision range into the base commit just outside
+ * the range (which becomes the parent of the squashed commit), the oldest
+ * commit contained in the range (whose message the squash reuses), and the
+ * range tip (whose tree becomes the result). A merge inside the range is fine,
+ * but the range must have a single base and must not reach a root commit.
+ */
+static int resolve_squash_range(struct repository *repo,
+ const char *range,
+ struct commit **base_out,
+ struct commit **oldest_out,
+ struct commit **tip_out)
+{
+ struct rev_info revs;
+ struct commit *commit, *base = NULL, *oldest = NULL, *tip = NULL;
+ struct strvec args = STRVEC_INIT;
+ int ret;
+
+ repo_init_revisions(repo, &revs, NULL);
+ strvec_push(&args, "ignored");
+ strvec_push(&args, "--reverse");
+ strvec_push(&args, "--topo-order");
+ strvec_push(&args, "--boundary");
+ strvec_push(&args, range);
+ setup_revisions_from_strvec(&args, &revs, NULL);
+ if (args.nr != 1) {
+ ret = error(_("'%s' does not name a revision range"), range);
+ goto out;
+ }
+
+ if (prepare_revision_walk(&revs) < 0) {
+ ret = error(_("error preparing revisions"));
+ goto out;
+ }
+
+ while ((commit = get_revision(&revs))) {
+ if (commit->object.flags & BOUNDARY) {
+ if (base) {
+ ret = error(_("range '%s' has more than one base; "
+ "cannot squash"), range);
+ goto out;
+ }
+ base = commit;
+ continue;
+ }
+ if (!oldest)
+ oldest = commit;
+ tip = commit;
+ }
+
+ if (!oldest) {
+ ret = error(_("the range '%s' is empty"), range);
+ goto out;
+ }
+
+ if (!base) {
+ ret = error(_("cannot squash the root commit"));
+ goto out;
+ }
+
+ *base_out = base;
+ *oldest_out = oldest;
+ *tip_out = tip;
+ ret = 0;
+
+out:
+ reset_revision_walk();
+ release_revisions(&revs);
+ strvec_clear(&args);
+ return ret;
+}
+
+static int cmd_history_squash(int argc,
+ const char **argv,
+ const char *prefix,
+ struct repository *repo)
+{
+ const char * const usage[] = {
+ GIT_HISTORY_SQUASH_USAGE,
+ NULL,
+ };
+ enum ref_action action = REF_ACTION_DEFAULT;
+ enum commit_tree_flags flags = 0;
+ int dry_run = 0;
+ struct option options[] = {
+ OPT_CALLBACK_F(0, "update-refs", &action, "(branches|head)",
+ N_("control which refs should be updated"),
+ PARSE_OPT_NONEG, parse_ref_action),
+ OPT_BOOL('n', "dry-run", &dry_run,
+ N_("perform a dry-run without updating any refs")),
+ OPT_BIT(0, "reedit-message", &flags,
+ N_("open an editor to modify the commit message"),
+ COMMIT_TREE_EDIT_MESSAGE),
+ OPT_END(),
+ };
+ struct strbuf reflog_msg = STRBUF_INIT;
+ struct commit *base, *oldest, *tip, *rewritten;
+ const struct object_id *base_tree_oid, *tip_tree_oid;
+ struct commit_list *parents = NULL;
+ struct rev_info revs = { 0 };
+ int ret;
+
+ argc = parse_options(argc, argv, prefix, options, usage, 0);
+ if (argc != 1) {
+ ret = error(_("command expects a single revision range"));
+ goto out;
+ }
+ repo_config(repo, git_default_config, NULL);
+
+ if (action == REF_ACTION_DEFAULT)
+ action = REF_ACTION_BRANCHES;
+
+ ret = resolve_squash_range(repo, argv[0], &base, &oldest, &tip);
+ if (ret < 0)
+ goto out;
+
+ ret = setup_revwalk(repo, action, tip, &revs);
+ if (ret < 0)
+ goto out;
+
+ base_tree_oid = &repo_get_commit_tree(repo, base)->object.oid;
+ tip_tree_oid = &repo_get_commit_tree(repo, tip)->object.oid;
+ commit_list_append(base, &parents);
+
+ ret = commit_tree_ext(repo, "squash", oldest, NULL, parents,
+ base_tree_oid, tip_tree_oid, &rewritten, flags);
+ if (ret < 0) {
+ ret = error(_("failed writing squashed commit"));
+ goto out;
+ }
+
+ strbuf_addf(&reflog_msg, "squash: updating %s", argv[0]);
+
+ ret = handle_reference_updates(&revs, action, tip, rewritten,
+ reflog_msg.buf, dry_run,
+ REPLAY_EMPTY_COMMIT_ABORT);
+ if (ret < 0) {
+ ret = error(_("failed replaying descendants"));
+ goto out;
+ }
+
+ ret = 0;
+
+out:
+ strbuf_release(&reflog_msg);
+ commit_list_free(parents);
+ release_revisions(&revs);
+ return ret;
+}
+
int cmd_history(int argc,
const char **argv,
const char *prefix,
@@ -982,6 +1134,7 @@ int cmd_history(int argc,
GIT_HISTORY_FIXUP_USAGE,
GIT_HISTORY_REWORD_USAGE,
GIT_HISTORY_SPLIT_USAGE,
+ GIT_HISTORY_SQUASH_USAGE,
NULL,
};
parse_opt_subcommand_fn *fn = NULL;
@@ -989,6 +1142,7 @@ int cmd_history(int argc,
OPT_SUBCOMMAND("fixup", &fn, cmd_history_fixup),
OPT_SUBCOMMAND("reword", &fn, cmd_history_reword),
OPT_SUBCOMMAND("split", &fn, cmd_history_split),
+ OPT_SUBCOMMAND("squash", &fn, cmd_history_squash),
OPT_END(),
};
diff --git a/t/meson.build b/t/meson.build
index 3219264fe7..d7ae5a46ef 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -399,6 +399,7 @@ integration_tests = [
't3451-history-reword.sh',
't3452-history-split.sh',
't3453-history-fixup.sh',
+ 't3454-history-squash.sh',
't3500-cherry.sh',
't3501-revert-cherry-pick.sh',
't3502-cherry-pick-merge.sh',
diff --git a/t/t3454-history-squash.sh b/t/t3454-history-squash.sh
new file mode 100755
index 0000000000..6c6a75bf00
--- /dev/null
+++ b/t/t3454-history-squash.sh
@@ -0,0 +1,213 @@
+#!/bin/sh
+
+test_description='tests for git-history squash subcommand'
+
+. ./test-lib.sh
+
+test_expect_success 'setup linear history touching two files' '
+ test_commit base file a &&
+ git tag start &&
+ test_commit one other x &&
+ test_commit two file c &&
+ test_commit three file d
+'
+
+test_expect_success 'errors on missing range argument' '
+ test_must_fail git history squash 2>err &&
+ test_grep "command expects a single revision range" err
+'
+
+test_expect_success 'errors on too many arguments' '
+ test_must_fail git history squash start.. HEAD 2>err &&
+ test_grep "command expects a single revision range" err
+'
+
+test_expect_success 'errors on an empty range' '
+ test_must_fail git history squash HEAD..HEAD 2>err &&
+ test_grep "the range .* is empty" err
+'
+
+test_expect_success 'errors when the range includes the root commit' '
+ test_must_fail git history squash HEAD 2>err &&
+ test_grep "cannot squash the root commit" err
+'
+
+test_expect_success 'squashes a range into a single commit without changing the tree' '
+ git reset --hard three &&
+ tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+ git history squash start.. &&
+
+ git rev-list --count start..HEAD >count &&
+ echo 1 >expect &&
+ test_cmp expect count &&
+ test_cmp_rev start HEAD^ &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
+ git log --format="%s" -1 >subject &&
+ echo one >expect &&
+ test_cmp expect subject &&
+ git reflog >reflog &&
+ test_grep "squash: updating" reflog
+'
+
+test_expect_success 'squashes an interior range and replays descendants verbatim' '
+ git reset --hard three &&
+ final_tree=$(git rev-parse HEAD^{tree}) &&
+
+ git history squash start..@~1 &&
+
+ git log --format="%s" start..HEAD >actual &&
+ cat >expect <<-\EOF &&
+ three
+ one
+ EOF
+ test_cmp expect actual &&
+
+ test_cmp_rev start HEAD~2 &&
+ test "$final_tree" = "$(git rev-parse HEAD^{tree})"
+'
+
+test_expect_success 'squashes when the base is the root commit' '
+ git reset --hard three &&
+ root=$(git rev-list --max-parents=0 HEAD) &&
+ tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+ git history squash "$root.." &&
+
+ git rev-list --count "$root..HEAD" >count &&
+ echo 1 >expect &&
+ test_cmp expect count &&
+ test_cmp_rev "$root" HEAD^ &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})"
+'
+
+test_expect_success 'squashing a single-commit range replays the rest' '
+ git reset --hard three &&
+ tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+ git history squash start..@~2 &&
+
+ git log --format="%s" start..HEAD >actual &&
+ cat >expect <<-\EOF &&
+ three
+ two
+ one
+ EOF
+ test_cmp expect actual &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})"
+'
+
+test_expect_success 'reuses the message of a fixup! commit in the range' '
+ git reset --hard start &&
+ test_commit reg1 file b &&
+ git commit --allow-empty -m "fixup! reg1" &&
+ test_commit reg2 file c &&
+
+ git history squash start.. &&
+
+ git log --format="%s" -1 >actual &&
+ echo reg1 >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'keeps the oldest message even if it is a fixup!' '
+ git reset --hard start &&
+ test_commit --no-tag "fixup! something" file b &&
+ test_commit tail file c &&
+
+ git history squash start.. &&
+
+ git log --format="%s" -1 >actual &&
+ echo "fixup! something" >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'preserves authorship of the oldest commit' '
+ git reset --hard start &&
+ GIT_AUTHOR_NAME=Squasher GIT_AUTHOR_EMAIL=squash@example.com \
+ test_commit oldest file b &&
+ test_commit newest file c &&
+
+ git history squash start.. &&
+
+ git log -1 --format="%an <%ae>" >actual &&
+ echo "Squasher <squash@example.com>" >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success '--dry-run predicts the rewrite without performing it' '
+ git reset --hard three &&
+ head_before=$(git rev-parse HEAD) &&
+
+ git history squash --dry-run start.. >out &&
+ grep "^update refs/heads/" out >update &&
+ predicted=$(awk "{print \$3}" update) &&
+ test_cmp_rev "$head_before" HEAD &&
+
+ git history squash start.. &&
+ test "$predicted" = "$(git rev-parse HEAD)"
+'
+
+test_expect_success '--update-refs=head only moves HEAD' '
+ git reset --hard three &&
+ git branch -f other HEAD &&
+ other_before=$(git rev-parse other) &&
+
+ git history squash --update-refs=head start.. &&
+
+ git rev-list --count start..HEAD >count &&
+ echo 1 >expect &&
+ test_cmp expect count &&
+ test_cmp_rev "$other_before" other
+'
+
+test_expect_success '--update-refs=branches moves a branch pointing into the range' '
+ git reset --hard three &&
+ git branch -f mid HEAD~2 &&
+ mid_before=$(git rev-parse mid) &&
+
+ git history squash start..@~1 &&
+
+ test_cmp_rev "$mid_before" mid &&
+ test_commit_message mid -m one
+'
+
+test_expect_success 'squashes a range whose internal merge has a single base' '
+ git reset --hard start &&
+ test_commit before-side file b &&
+ git checkout -b inner-side &&
+ test_commit on-inner-side inner x &&
+ git checkout - &&
+ test_commit after-side file c &&
+ git merge --no-ff -m merge inner-side &&
+ test_commit after-merge file d &&
+ tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+ git history squash start.. &&
+
+ git rev-list --count start..HEAD >count &&
+ echo 1 >expect &&
+ test_cmp expect count &&
+ git log --format="%s" -1 >subject &&
+ echo before-side >expect &&
+ test_cmp expect subject &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
+ test_path_is_file inner
+'
+
+test_expect_success 'refuses to squash a range with more than one base' '
+ git reset --hard start &&
+ head_before=$(git rev-parse HEAD) &&
+ git checkout -b forked-before &&
+ test_commit forked-side fside x &&
+ git checkout - &&
+ test_commit forked-main file b &&
+ git merge --no-ff -m merge forked-before &&
+ merged=$(git rev-parse HEAD) &&
+
+ test_must_fail git history squash forked-main.. 2>err &&
+ test_grep "more than one base" err &&
+ test_cmp_rev "$merged" HEAD
+'
+
+test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 2/4] history: give commit_tree_ext a message template
From: Harald Nordgren via GitGitGadget @ 2026-06-18 19:17 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2337.v3.git.git.1781810226.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
commit_tree_ext() reuses the message of the commit it is handed. A
caller that folds several commits together wants to seed the message
from more than that single commit, so add an optional message_template
parameter. When NULL, the behavior is unchanged.
Pass NULL from the existing fixup and split callers.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
builtin/history.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/builtin/history.c b/builtin/history.c
index f95f26e684..305bde3102 100644
--- a/builtin/history.c
+++ b/builtin/history.c
@@ -101,6 +101,7 @@ enum commit_tree_flags {
static int commit_tree_ext(struct repository *repo,
const char *action,
struct commit *commit_with_message,
+ const char *message_template,
const struct commit_list *parents,
const struct object_id *old_tree,
const struct object_id *new_tree,
@@ -130,13 +131,16 @@ static int commit_tree_ext(struct repository *repo,
original_author = xmemdupz(ptr, len);
find_commit_subject(original_message, &original_body);
+ if (!message_template)
+ message_template = original_body;
+
if (flags & COMMIT_TREE_EDIT_MESSAGE) {
ret = fill_commit_message(repo, old_tree, new_tree,
- original_body, action, &commit_message);
+ message_template, action, &commit_message);
if (ret < 0)
goto out;
} else {
- strbuf_addstr(&commit_message, original_body);
+ strbuf_addstr(&commit_message, message_template);
}
original_extra_headers = read_commit_extra_headers(commit_with_message,
@@ -189,7 +193,7 @@ static int commit_tree_with_edited_message(struct repository *repo,
if (first_parent_tree_oid(repo, original, &parent_tree_oid) < 0)
return -1;
- return commit_tree_ext(repo, action, original, original->parents,
+ return commit_tree_ext(repo, action, original, NULL, original->parents,
&parent_tree_oid, tree_oid, out, COMMIT_TREE_EDIT_MESSAGE);
}
@@ -644,7 +648,7 @@ static int cmd_history_fixup(int argc,
goto out;
if (!skip_commit) {
- ret = commit_tree_ext(repo, "fixup", original, original->parents,
+ ret = commit_tree_ext(repo, "fixup", original, NULL, original->parents,
&original_tree->object.oid, &merge_result.tree->object.oid,
&rewritten, flags);
if (ret < 0) {
@@ -855,7 +859,7 @@ static int split_commit(struct repository *repo,
* The first commit is constructed from the split-out tree. The base
* that shall be diffed against is the parent of the original commit.
*/
- ret = commit_tree_ext(repo, "split-out", original, original->parents, &parent_tree_oid,
+ ret = commit_tree_ext(repo, "split-out", original, NULL, original->parents, &parent_tree_oid,
&split_tree->object.oid, &first_commit, COMMIT_TREE_EDIT_MESSAGE);
if (ret < 0) {
ret = error(_("failed writing first commit"));
@@ -872,7 +876,7 @@ static int split_commit(struct repository *repo,
old_tree_oid = &repo_get_commit_tree(repo, first_commit)->object.oid;
new_tree_oid = &repo_get_commit_tree(repo, original)->object.oid;
- ret = commit_tree_ext(repo, "split-out", original, parents, old_tree_oid,
+ ret = commit_tree_ext(repo, "split-out", original, NULL, parents, old_tree_oid,
new_tree_oid, &second_commit, COMMIT_TREE_EDIT_MESSAGE);
if (ret < 0) {
ret = error(_("failed writing second commit"));
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 1/4] history: extract helper for a commit's parent tree
From: Harald Nordgren via GitGitGadget @ 2026-06-18 19:17 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2337.v3.git.git.1781810226.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
Three places resolve the tree of a commit's first parent, falling back
to the empty tree for a root commit, each repeating the same parse and
oidcpy dance. Extract a first_parent_tree_oid() helper and route the
existing callers through it.
No change in behavior.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
builtin/history.c | 58 +++++++++++++++++++++--------------------------
1 file changed, 26 insertions(+), 32 deletions(-)
diff --git a/builtin/history.c b/builtin/history.c
index 091465a59e..f95f26e684 100644
--- a/builtin/history.c
+++ b/builtin/history.c
@@ -157,6 +157,25 @@ out:
return ret;
}
+static int first_parent_tree_oid(struct repository *repo,
+ struct commit *commit,
+ struct object_id *out)
+{
+ struct commit *parent = commit->parents ? commit->parents->item : NULL;
+
+ if (!parent) {
+ oidcpy(out, repo->hash_algo->empty_tree);
+ return 0;
+ }
+
+ if (repo_parse_commit(repo, parent))
+ return error(_("unable to parse parent commit %s"),
+ oid_to_hex(&parent->object.oid));
+
+ oidcpy(out, &repo_get_commit_tree(repo, parent)->object.oid);
+ return 0;
+}
+
static int commit_tree_with_edited_message(struct repository *repo,
const char *action,
struct commit *original,
@@ -164,21 +183,11 @@ static int commit_tree_with_edited_message(struct repository *repo,
{
struct object_id parent_tree_oid;
const struct object_id *tree_oid;
- struct commit *parent;
tree_oid = &repo_get_commit_tree(repo, original)->object.oid;
- parent = original->parents ? original->parents->item : NULL;
- if (parent) {
- if (repo_parse_commit(repo, parent)) {
- return error(_("unable to parse parent commit %s"),
- oid_to_hex(&parent->object.oid));
- }
-
- parent_tree_oid = repo_get_commit_tree(repo, parent)->object.oid;
- } else {
- oidcpy(&parent_tree_oid, repo->hash_algo->empty_tree);
- }
+ if (first_parent_tree_oid(repo, original, &parent_tree_oid) < 0)
+ return -1;
return commit_tree_ext(repo, action, original, original->parents,
&parent_tree_oid, tree_oid, out, COMMIT_TREE_EDIT_MESSAGE);
@@ -444,18 +453,10 @@ static int commit_became_empty(struct repository *repo,
struct commit *original,
struct tree *result)
{
- struct commit *parent = original->parents ? original->parents->item : NULL;
struct object_id parent_tree_oid;
- if (parent) {
- if (repo_parse_commit(repo, parent))
- return error(_("unable to parse parent of %s"),
- oid_to_hex(&original->object.oid));
-
- parent_tree_oid = repo_get_commit_tree(repo, parent)->object.oid;
- } else {
- oidcpy(&parent_tree_oid, repo->hash_algo->empty_tree);
- }
+ if (first_parent_tree_oid(repo, original, &parent_tree_oid) < 0)
+ return -1;
return oideq(&result->object.oid, &parent_tree_oid);
}
@@ -799,16 +800,9 @@ static int split_commit(struct repository *repo,
struct tree *split_tree;
int ret;
- if (original->parents) {
- if (repo_parse_commit(repo, original->parents->item)) {
- ret = error(_("unable to parse parent commit %s"),
- oid_to_hex(&original->parents->item->object.oid));
- goto out;
- }
-
- parent_tree_oid = *get_commit_tree_oid(original->parents->item);
- } else {
- oidcpy(&parent_tree_oid, repo->hash_algo->empty_tree);
+ if (first_parent_tree_oid(repo, original, &parent_tree_oid) < 0) {
+ ret = -1;
+ goto out;
}
original_commit_tree_oid = get_commit_tree_oid(original);
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 0/4] history: add squash subcommand to fold a range
From: Harald Nordgren via GitGitGadget @ 2026-06-18 19:17 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren
In-Reply-To: <pull.2337.v2.git.git.1781512625.gitgitgadget@gmail.com>
Adds git history squash <revision-range> to fold a range of commits into its
oldest one, reusing that commit's message and replaying any descendants on
top.
Changes in v3:
* Moved the feature out of git rebase and into a new git history squash
<revision-range> subcommand, per the list discussion. git rebase --squash
is dropped.
* Takes an arbitrary range (git history squash @~3.., git history squash
@~5..@~2), folding it into the oldest commit and replaying any
descendants on top.
* Implemented as a single tree operation rather than picking each commit,
so there are no repeated conflict stops (addresses Phillip's efficiency
point).
* A merge inside the range is folded fine, only a range with more than one
base is rejected.
* --reedit-message seeds the editor with every folded-in message, not just
the oldest.
Harald Nordgren (4):
history: extract helper for a commit's parent tree
history: give commit_tree_ext a message template
history: add squash subcommand to fold a range
history: re-edit a squash with every message
Documentation/git-history.adoc | 21 +++
builtin/history.c | 287 ++++++++++++++++++++++++++++-----
t/meson.build | 1 +
t/t3454-history-squash.sh | 250 ++++++++++++++++++++++++++++
4 files changed, 521 insertions(+), 38 deletions(-)
create mode 100755 t/t3454-history-squash.sh
base-commit: 95e20213faefeb95df29277c58ac1980ab68f701
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2337%2FHaraldNordgren%2Frebase-fixup-fold-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2337/HaraldNordgren/rebase-fixup-fold-v3
Pull-Request: https://github.com/git/git/pull/2337
Range-diff vs v2:
1: c55b9cd6f7 < -: ---------- t3415: remove prepare-commit-msg hook after use
2: 22d4276ff5 < -: ---------- rebase: add --squash to fold a range
-: ---------- > 1: 1e31474ef6 history: extract helper for a commit's parent tree
-: ---------- > 2: 498da64046 history: give commit_tree_ext a message template
-: ---------- > 3: 66b2f49fb4 history: add squash subcommand to fold a range
-: ---------- > 4: 43e4270614 history: re-edit a squash with every message
--
gitgitgadget
^ permalink raw reply
* Re: [PATCH] rebase: mention --abort alongside --continue
From: Harald Nordgren @ 2026-06-18 18:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Phillip Wood, Harald Nordgren via GitGitGadget, git
In-Reply-To: <xmqqo6h9z7e6.fsf@gitster.g>
Just an example when working on a different topic:
I rebased with -x to run all the tests, but ran a test that didn't
exist yet on the first commit and ended up in a bad state. Here it
should clearly show the 'git rebase --abort', so I can start over,
it's not something to fix:
```
$ git rebase --keep-base -x 'make -s' -x 'cd t && prove -j8
t3454-history-squash.sh t3453-history-fixup.sh t3452-history-split.sh
t3451-history-reword.sh t3450-history.sh'
Executing: make -s
GIT_VERSION=2.55.0.rc1.20.g1e31474ef6
Executing: cd t && prove -j8 t3454-history-squash.sh
t3453-history-fixup.sh t3452-history-split.sh t3451-history-reword.sh
t3450-history.sh
Cannot detect source of 't3454-history-squash.sh'! at
/System/Library/Perl/5.34/TAP/Parser/IteratorFactory.pm line 256.
...
warning: execution failed: cd t && prove -j8 t3454-history-squash.sh
t3453-history-fixup.sh t3452-history-split.sh t3451-history-reword.sh
t3450-history.sh
You can fix the problem, and then run
git rebase --continue
$ git status
interactive rebase in progress; onto 95e20213fa
Last commands done (3 commands done):
exec make -s
exec cd t && prove -j8 t3454-history-squash.sh
t3453-history-fixup.sh t3452-history-split.sh t3451-history-reword.sh
t3450-history.sh
(see more in file .git/rebase-merge/done)
Next commands to do (9 remaining commands):
pick 498da64046 # history: give commit_tree_ext a message template
exec make -s
(use "git rebase --edit-todo" to view and edit)
You are currently editing a commit while rebasing branch
'rebase-fixup-fold' on '95e20213fa'.
(use "git commit --amend" to amend the current commit)
(use "git rebase --continue" once you are satisfied with your changes)
nothing to commit, working tree clean
```
Harald
On Wed, Jun 17, 2026 at 2:19 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Phillip Wood <phillip.wood123@gmail.com> writes:
>
> >> It is very true that users who know what they are doing and got into
> >> such conflicts are opted to go into such a situation tnat it is
> >> unlikely that they would appreciate a choice to abort.
> >
> > That's not quite what I was trying to say which was that aborting in the
> > case of conflicts is more likely than in the case of a failed exec.
>
> Ah, I misread the intention. And I agree with you that "failed
> test" case is very likely to lead to "further changes/amends" and
> not "aborted rebase".
>
> > So if I've understood we'd print a message explaining what's happened
> > and how to continue followed by a hint about aborting. The message would
> > depend on what problem caused the rebase to stop, but the hint would be
> > the same in each case. That sounds fine to me.
>
> Yeah, and "failed test" would not be one of the problem that would
> invite the hint to "abort". I am OK with that, too. FWIW, I am OK
> if the "you can abort" hint cannot be configured away, either ;-)
>
^ permalink raw reply
* Pinned references?
From: Erik Östlund @ 2026-06-18 18:37 UTC (permalink / raw)
To: git
I'd like to be able to express a reference together with an expected
object ID, for example with strawman syntax like:
refs/tags/v1.2.3?oid=a1b2c3d4
The intended semantics would be that both the reference and object ID
must exist, and Git should fail if the reference does not resolve to the
specified object ID.
Tags are nice because they convey human meaning. Object IDs are nice
because they are immutable. As it is, I often have to choose between the
two, or represent them separately in external tooling.
Is there existing terminology, prior discussion, or an accepted Git-native
approach for this kind of "ref plus expected OID" invariant? I
searched both the Git reference documentation and the mailing list
archives, but couldn't find what I was looking for.
Thanks,
Erik
^ permalink raw reply
* [PATCH 7/7] diffcore-pickaxe: scope -G to the -L tracked range
From: Michael Montalbo via GitGitGadget @ 2026-06-18 18:16 UTC (permalink / raw)
To: git; +Cc: D. Ben Knoble, Michael Montalbo, Michael Montalbo
In-Reply-To: <pull.2152.git.1781806593.gitgitgadget@gmail.com>
From: Michael Montalbo <mmontalbo@gmail.com>
git log -L scopes its diff output to the tracked range, but pickaxe
(-S, -G) still runs in diffcore over the whole-file change, so -L -G
selects a commit whenever the pattern appears in any added or removed
line of the file, even outside the tracked range.
Teach -G to honor the range. diff_grep() already runs an xdiff pass
and greps the +/- lines; route that pass through the line-range filter
so only the tracked range's lines are grepped. Expose the filter as
diff_emit_line_ranges(), a line-range-scoped xdi_diff_outf(), thread
the filepair's line_ranges through the pickaxe callback, and pass it
from pickaxe_match(). Skip scoping under textconv, whose output is not
in the original file's line coordinates.
-G needs only a hit/no-hit answer, so the line-number concerns the
filter handles for patch and check output do not apply here.
-S is left matching the whole file: it counts needle occurrences per
blob rather than grepping the diff, so scoping it needs a different
approach, left to a follow-up. has_changes() takes the range parameter
but ignores it for now.
Document the resulting -L pickaxe scoping: -G is range-scoped, while -S
still matches the whole file.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
Documentation/line-range-options.adoc | 5 +-
diff.c | 15 ++++++
diffcore-pickaxe.c | 37 +++++++++++---
t/t4211-line-log.sh | 72 +++++++++++++++++++++++++--
xdiff-interface.h | 13 +++++
5 files changed, 130 insertions(+), 12 deletions(-)
diff --git a/Documentation/line-range-options.adoc b/Documentation/line-range-options.adoc
index ec92f43ae4..7cfae1499c 100644
--- a/Documentation/line-range-options.adoc
+++ b/Documentation/line-range-options.adoc
@@ -20,6 +20,9 @@
+
Patch formatting options such as `--word-diff`, `--color-moved`,
`--no-prefix`, and whitespace options (`-w`, `-b`) are supported,
-as are pickaxe options (`-S`, `-G`) and `--diff-filter`.
+as are pickaxe options (`-S`, `-G`) and `--diff-filter`. `-G` is
+scoped to the tracked range; `-S` is still evaluated over the whole
+file, so an `-S` query may select a commit for a change outside the
+range.
+
include::line-range-format.adoc[]
diff --git a/diff.c b/diff.c
index 519c513356..a8f346621b 100644
--- a/diff.c
+++ b/diff.c
@@ -2817,6 +2817,21 @@ static int line_range_filter_diff(struct line_range_filter *filter,
return ret;
}
+/*
+ * Expose the in-file line-range filter to callers outside diff.c (e.g.
+ * pickaxe -G); see xdiff-interface.h for the contract.
+ */
+int diff_emit_line_ranges(mmfile_t *one, mmfile_t *two,
+ const struct range_set *ranges,
+ xdiff_emit_line_fn line_fn, void *cb_data,
+ xpparam_t *xpp, xdemitconf_t *xecfg)
+{
+ struct line_range_filter filter;
+
+ line_range_filter_init(&filter, ranges, line_fn, cb_data);
+ return line_range_filter_diff(&filter, one, two, xpp, xecfg);
+}
+
static void pprint_rename(struct strbuf *name, const char *a, const char *b)
{
const char *old_name = a;
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index a52d569911..047b2bf7ac 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -16,7 +16,8 @@
typedef int (*pickaxe_fn)(mmfile_t *one, mmfile_t *two,
struct diff_options *o,
- regex_t *regexp, kwset_t kws);
+ regex_t *regexp, kwset_t kws,
+ const struct range_set *ranges);
struct diffgrep_cb {
regex_t *regexp;
@@ -42,7 +43,8 @@ static int diffgrep_consume(void *priv, char *line, unsigned long len)
static int diff_grep(mmfile_t *one, mmfile_t *two,
struct diff_options *o,
- regex_t *regexp, kwset_t kws UNUSED)
+ regex_t *regexp, kwset_t kws UNUSED,
+ const struct range_set *ranges)
{
struct diffgrep_cb ecbdata;
xpparam_t xpp;
@@ -50,8 +52,11 @@ static int diff_grep(mmfile_t *one, mmfile_t *two,
int ret;
/*
- * We have both sides; need to run textual diff and see if
- * the pattern appears on added/deleted lines.
+ * We have both sides; need to run textual diff and see if the
+ * pattern appears on added/deleted lines. Under -L (ranges set),
+ * forward only the tracked range's lines so the match is scoped.
+ * -G needs only a hit/no-hit answer, so the line-number bookkeeping
+ * the filter does for -L patch and check output is irrelevant here.
*/
memset(&xpp, 0, sizeof(xpp));
memset(&xecfg, 0, sizeof(xecfg));
@@ -65,8 +70,12 @@ static int diff_grep(mmfile_t *one, mmfile_t *two,
* An xdiff error might be our "data->hit" from above. See the
* comment for xdiff_emit_line_fn in xdiff-interface.h
*/
- ret = xdi_diff_outf(one, two, NULL, diffgrep_consume,
- &ecbdata, &xpp, &xecfg);
+ if (ranges)
+ ret = diff_emit_line_ranges(one, two, ranges, diffgrep_consume,
+ &ecbdata, &xpp, &xecfg);
+ else
+ ret = xdi_diff_outf(one, two, NULL, diffgrep_consume,
+ &ecbdata, &xpp, &xecfg);
if (ecbdata.hit)
return 1;
if (ret)
@@ -119,8 +128,13 @@ static unsigned int contains(mmfile_t *mf, regex_t *regexp, kwset_t kws,
static int has_changes(mmfile_t *one, mmfile_t *two,
struct diff_options *o UNUSED,
- regex_t *regexp, kwset_t kws)
+ regex_t *regexp, kwset_t kws,
+ const struct range_set *ranges UNUSED)
{
+ /*
+ * -S counts needle occurrences in each whole blob. Scoping this to
+ * a -L range is left to a follow-up; for now -S ignores the range.
+ */
unsigned int c1 = one ? contains(one, regexp, kws, 0) : 0;
unsigned int c2 = two ? contains(two, regexp, kws, c1 + 1) : 0;
return c1 != c2;
@@ -132,6 +146,7 @@ static int pickaxe_match(struct diff_filepair *p, struct diff_options *o,
struct userdiff_driver *textconv_one = NULL;
struct userdiff_driver *textconv_two = NULL;
mmfile_t mf1, mf2;
+ const struct range_set *ranges;
int ret;
/* ignore unmerged */
@@ -169,7 +184,13 @@ static int pickaxe_match(struct diff_filepair *p, struct diff_options *o,
mf1.size = fill_textconv(o->repo, textconv_one, p->one, &mf1.ptr);
mf2.size = fill_textconv(o->repo, textconv_two, p->two, &mf2.ptr);
- ret = fn(&mf1, &mf2, o, regexp, kws);
+ /*
+ * -L scopes the search to the tracked range, but the range is in
+ * original-file line coordinates that do not map onto textconv
+ * output, so search the whole file when textconv is in play.
+ */
+ ranges = (textconv_one || textconv_two) ? NULL : p->line_ranges;
+ ret = fn(&mf1, &mf2, o, regexp, kws, ranges);
if (textconv_one)
free(mf1.ptr);
diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh
index 9d351aa05f..3d35b20178 100755
--- a/t/t4211-line-log.sh
+++ b/t/t4211-line-log.sh
@@ -722,9 +722,9 @@ test_expect_success '-L with -S filters to string-count changes' '
test_expect_success '-L with -G filters to diff-text matches' '
git checkout parent-oids &&
git log -L:func2:file.c -G "F2 [+] 2" --format= >actual &&
- # -G greps the whole-file diff text, not just the tracked range;
- # combined with -L, this selects commits that both touch func2
- # and have "F2 + 2" in their diff.
+ # -G greps the diff text, and under -L only the lines in the
+ # tracked range (unlike -S above, which searches the whole file);
+ # this selects commits whose change to func2 contains "F2 + 2".
test $(grep -c "^diff --git" actual) = 1 &&
grep "F2 + 2" actual
'
@@ -1110,4 +1110,70 @@ test_expect_success '--check does not report blank-at-eof outside the range' '
test_grep ! "blank line at EOF" actual
'
+test_expect_success '-L -G is scoped to the tracked range' '
+ git checkout --orphan grep-scope &&
+ git reset --hard &&
+ cat >gp.c <<-\EOF &&
+ int func1()
+ {
+ return ALPHA;
+ }
+
+ int func2()
+ {
+ return BETA;
+ }
+ EOF
+ git add gp.c &&
+ test_tick &&
+ git commit -m "add gp.c" &&
+ sed -e "s/ALPHA/ALPHA2/" -e "s/BETA/BETA2/" gp.c >tmp &&
+ mv tmp gp.c &&
+ git commit -a -m "touch both functions" &&
+ # The commit changes ALPHA (func1) and BETA (func2). Tracking func2,
+ # -G BETA matches its in-range change; -G ALPHA must not, since ALPHA
+ # changes only outside the tracked range.
+ git log -L:func2:gp.c -G BETA --format=%s >actual &&
+ test_grep "touch both functions" actual &&
+ git log -L:func2:gp.c -G ALPHA --format=%s >actual &&
+ test_grep ! "touch both functions" actual
+'
+
+test_expect_success '-L -G searches the whole file under textconv' '
+ git checkout --orphan grep-textconv &&
+ git reset --hard &&
+ cat >tc.c <<-\EOF &&
+ int func1()
+ {
+ return F1;
+ }
+
+ int func2()
+ {
+ return F2;
+ }
+ EOF
+ git add tc.c &&
+ test_tick &&
+ git commit -m "add tc.c" &&
+ # One commit changes func1 and func2; MAGIC lands only in the
+ # func2 change, outside func1.
+ sed -e "s/F1/F1 + 1/" -e "s/return F2/return MAGIC/" tc.c >tmp &&
+ mv tmp tc.c &&
+ git commit -a -m "change both funcs" &&
+ echo "tc.c diff=tc" >.gitattributes &&
+
+ # Without a textconv driver, -G is scoped to func1, so MAGIC (only
+ # in the func2 change) does not select the commit.
+ git log -L:func1:tc.c -G MAGIC --format=%s --no-patch >actual &&
+ test_must_be_empty actual &&
+
+ # A textconv driver makes the range (original-file line numbers)
+ # meaningless against the driver output, so -G falls back to the
+ # whole file and MAGIC now selects the commit.
+ git config diff.tc.textconv cat &&
+ git log -L:func1:tc.c -G MAGIC --format=%s --no-patch >actual &&
+ test_grep "change both funcs" actual
+'
+
test_done
diff --git a/xdiff-interface.h b/xdiff-interface.h
index 51c88296ed..71e5dffefb 100644
--- a/xdiff-interface.h
+++ b/xdiff-interface.h
@@ -46,6 +46,19 @@ int xdi_diff_outf(mmfile_t *mf1, mmfile_t *mf2,
xdiff_emit_line_fn line_fn,
void *consume_callback_data,
xpparam_t const *xpp, xdemitconf_t const *xecfg);
+
+struct range_set;
+/*
+ * Like xdi_diff_outf(), but forwards only the lines within the given
+ * (post-image) line ranges to line_fn, as "git log -L" scopes its output.
+ * Returns line_fn's latched return value (so a consumer can signal a hit
+ * with a non-zero return), or non-zero on xdiff failure. Defined in
+ * diff.c (it reuses the line-range filter there).
+ */
+int diff_emit_line_ranges(mmfile_t *mf1, mmfile_t *mf2,
+ const struct range_set *ranges,
+ xdiff_emit_line_fn line_fn, void *cb_data,
+ xpparam_t *xpp, xdemitconf_t *xecfg);
int read_mmfile(mmfile_t *ptr, const char *filename);
void read_mmblob(mmfile_t *ptr, struct object_database *odb,
const struct object_id *oid);
--
gitgitgadget
^ permalink raw reply related
* [PATCH 6/7] diff: support --check with -L line ranges
From: Michael Montalbo via GitGitGadget @ 2026-06-18 18:16 UTC (permalink / raw)
To: git; +Cc: D. Ben Knoble, Michael Montalbo, Michael Montalbo
In-Reply-To: <pull.2152.git.1781806593.gitgitgadget@gmail.com>
From: Michael Montalbo <mmontalbo@gmail.com>
builtin_checkdiff() runs its own xdiff pass to detect whitespace
errors in newly added lines. When -L is active, the check should
be scoped to the tracked line ranges rather than the whole file.
Reuse the line_range_filter to wrap checkdiff_consume(), the same
pattern already used for patch output and diffstat. The filter
forwards only in-range lines for whitespace checking.
checkdiff reports the file line number of each error, which it
normally learns from the hunk header via checkdiff_consume_hunk().
The filter synthesizes its own hunk headers, so give it an optional
hunk callback and route checkdiff_consume_hunk() through it; this
sets the post-image position before the in-range lines are replayed.
Without it the reported line numbers would count from the start of
the range hunk rather than the start of the file.
The trailing blank-at-eof check is a second pass that scans the whole
file via check_blank_at_eof(), so gate its report on the tracked
ranges as well; otherwise a blank line added at end of file is
reported even when it lies outside the range.
Add DIFF_FORMAT_CHECKDIFF to the -L output format allowlist in
setup_revisions() so that -L --check is accepted, and list --check
among the supported formats in the documentation. Add tests covering
that whitespace errors are reported, scoped to the tracked range, and
labeled with the correct file line number, including when two errors
in one range are separated by a gap that would otherwise split into
multiple xdiff hunks.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
Documentation/line-range-options.adoc | 2 +-
diff.c | 65 ++++++++++++++++++-
revision.c | 2 +-
t/t4211-line-log.sh | 92 +++++++++++++++++++++++++++
4 files changed, 156 insertions(+), 5 deletions(-)
diff --git a/Documentation/line-range-options.adoc b/Documentation/line-range-options.adoc
index 1a25f55bb1..ec92f43ae4 100644
--- a/Documentation/line-range-options.adoc
+++ b/Documentation/line-range-options.adoc
@@ -10,7 +10,7 @@
You can specify this option more than once. Implies `--patch`.
Patch output can be suppressed using `--no-patch`.
The following non-patch diff formats are supported: `--raw`,
- `--name-only`, `--name-status`, `--summary`,
+ `--name-only`, `--name-status`, `--summary`, `--check`,
`--stat`, `--numstat`, and `--shortstat`.
The stat formats show range-scoped counts: only lines within
the tracked range are counted. `--dirstat` is not supported
diff --git a/diff.c b/diff.c
index 026fafeb90..519c513356 100644
--- a/diff.c
+++ b/diff.c
@@ -665,6 +665,12 @@ struct emit_callback {
*/
struct line_range_filter {
xdiff_emit_line_fn orig_line_fn;
+ /*
+ * Optional; consumers that report file line numbers (e.g.
+ * checkdiff) need the synthetic hunk header to set their
+ * post-image position before in-range lines are replayed.
+ */
+ xdiff_emit_hunk_fn orig_hunk_fn;
void *orig_cb_data;
const struct range_set *ranges; /* 0-based [start, end) */
unsigned int cur_range; /* index into the range_set */
@@ -2652,6 +2658,17 @@ static void flush_range_hunk(struct line_range_filter *filter)
filter->hunk.new_begin, new_count,
filter->func, filter->funclen);
+ /*
+ * Inform a line-numbering consumer of the post-image position
+ * before replaying lines, mirroring the hunk callback xdiff
+ * would have issued for a non-scoped diff.
+ */
+ if (filter->orig_hunk_fn)
+ filter->orig_hunk_fn(filter->orig_cb_data,
+ filter->hunk.old_begin, old_count,
+ filter->hunk.new_begin, new_count,
+ filter->func, filter->funclen);
+
filter->ret = filter->orig_line_fn(filter->orig_cb_data, hdr.buf, hdr.len);
strbuf_release(&hdr);
@@ -4330,11 +4347,29 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
diff_free_filespec_data(two);
}
+/*
+ * Is the 0-based line index within any of the tracked ranges?
+ * (range_set ranges are 0-based, half-open [start, end).) This is a
+ * one-shot query for a single line and scans; the streaming filter
+ * (line_range_line_fn) uses a forward cursor instead.
+ */
+static int idx_in_ranges(const struct range_set *ranges, long idx)
+{
+ unsigned int i;
+
+ for (i = 0; i < ranges->nr; i++)
+ if (idx >= ranges->ranges[i].start &&
+ idx < ranges->ranges[i].end)
+ return 1;
+ return 0;
+}
+
static void builtin_checkdiff(const char *name_a, const char *name_b,
const char *attr_path,
struct diff_filespec *one,
struct diff_filespec *two,
- struct diff_options *o)
+ struct diff_options *o,
+ const struct range_set *line_ranges)
{
mmfile_t mf1, mf2;
struct checkdiff_t data;
@@ -4374,7 +4409,19 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,
memset(&xecfg, 0, sizeof(xecfg));
xecfg.ctxlen = 1; /* at least one context line */
xpp.flags = 0;
- if (xdi_diff_outf(&mf1, &mf2, checkdiff_consume_hunk,
+
+ if (line_ranges) {
+ struct line_range_filter lr_filter;
+
+ line_range_filter_init(&lr_filter, line_ranges,
+ checkdiff_consume, &data);
+ lr_filter.orig_hunk_fn = checkdiff_consume_hunk;
+
+ if (line_range_filter_diff(&lr_filter, &mf1, &mf2,
+ &xpp, &xecfg))
+ die("unable to generate checkdiff for %s",
+ one->path);
+ } else if (xdi_diff_outf(&mf1, &mf2, checkdiff_consume_hunk,
checkdiff_consume, &data,
&xpp, &xecfg))
die("unable to generate checkdiff for %s", one->path);
@@ -4387,6 +4434,17 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,
check_blank_at_eof(&mf1, &mf2, &ecbdata);
blank_at_eof = ecbdata.blank_at_eof_in_postimage;
+ /*
+ * check_blank_at_eof() scans the whole file; with -L,
+ * keep the report only when its line is in a tracked
+ * range. The error's location is the first trailing
+ * blank line (blank_at_eof, 1-based; ranges 0-based), so
+ * we scope by that line.
+ */
+ if (blank_at_eof && line_ranges &&
+ !idx_in_ranges(line_ranges, blank_at_eof - 1))
+ blank_at_eof = 0;
+
if (blank_at_eof) {
static char *err;
if (!err)
@@ -5179,7 +5237,8 @@ static void run_checkdiff(struct diff_filepair *p, struct diff_options *o)
diff_fill_oid_info(p->one, o->repo->index);
diff_fill_oid_info(p->two, o->repo->index);
- builtin_checkdiff(name, other, attr_path, p->one, p->two, o);
+ builtin_checkdiff(name, other, attr_path, p->one, p->two, o,
+ p->line_ranges);
}
void repo_diff_setup(struct repository *r, struct diff_options *options)
diff --git a/revision.c b/revision.c
index 2c76e15778..7abb287451 100644
--- a/revision.c
+++ b/revision.c
@@ -3195,7 +3195,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
DIFF_FORMAT_RAW | DIFF_FORMAT_NAME |
DIFF_FORMAT_NAME_STATUS | DIFF_FORMAT_SUMMARY |
DIFF_FORMAT_NUMSTAT | DIFF_FORMAT_DIFFSTAT |
- DIFF_FORMAT_SHORTSTAT))))
+ DIFF_FORMAT_SHORTSTAT | DIFF_FORMAT_CHECKDIFF))))
die(_("-L does not support the requested diff format"));
if (revs->expand_tabs_in_log < 0)
diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh
index af37bd532f..9d351aa05f 100755
--- a/t/t4211-line-log.sh
+++ b/t/t4211-line-log.sh
@@ -1018,4 +1018,96 @@ test_expect_success '--summary shows new file on root commit' '
test_grep "create mode 100644 file.c" actual
'
+test_expect_success 'setup for --check test' '
+ git checkout --orphan check-test &&
+ git reset --hard &&
+ cat >check.c <<-\EOF &&
+ void tracked()
+ {
+ return;
+ }
+
+ void other()
+ {
+ return;
+ }
+ EOF
+ git add check.c &&
+ test_tick &&
+ git commit -m "add check.c" &&
+ # Introduce trailing whitespace errors in both functions
+ sed "s/return;/return; /" check.c >check.c.tmp &&
+ mv check.c.tmp check.c &&
+ git commit -a -m "introduce trailing whitespace"
+'
+
+test_expect_success '--check scoped to tracked range with correct file line' '
+ # tracked() trailing whitespace is at check.c:3; report it with the
+ # real file line number, not a count from the start of the range
+ # hunk. other() at check.c:8 is outside the range and is excluded.
+ test_must_fail git log -L:tracked:check.c --check --format= >actual &&
+ test_grep "check.c:3: trailing whitespace" actual &&
+ test_grep ! "check.c:8:" actual
+'
+
+test_expect_success '--check reports each of several tracked ranges' '
+ # Track both functions as separate ranges. Each range is flushed
+ # as its own hunk, so the second error must report its real file
+ # line (check.c:8), not continue the numbering from the first
+ # range (check.c:3).
+ test_must_fail git log -L:tracked:check.c -L:other:check.c \
+ --check --format= >actual &&
+ test_grep "check.c:3: trailing whitespace" actual &&
+ test_grep "check.c:8: trailing whitespace" actual
+'
+
+test_expect_success '--check line numbers stay correct across a gap in one range' '
+ git checkout --orphan check-gap &&
+ git reset --hard &&
+ cat >gap.c <<-\EOF &&
+ void tracked()
+ {
+ int a = 1;
+ int b = 2;
+ int c = 3;
+ int d = 4;
+ int e = 5;
+ int g = 7;
+ return;
+ }
+ EOF
+ git add gap.c &&
+ test_tick &&
+ git commit -m "add gap.c" &&
+ # Two trailing-whitespace errors within one tracked range,
+ # separated by clean lines. ctxlen is inflated to the range span,
+ # so they land in a single xdiff hunk with the gap as context;
+ # both must report their real file line number, with the context
+ # lines between them counted.
+ sed -e "s/int a = 1;/int a = 1; /" -e "s/int g = 7;/int g = 7; /" gap.c >tmp &&
+ mv tmp gap.c &&
+ git commit -a -m "ws errors with a gap" &&
+ test_must_fail git log -L:tracked:gap.c --check --format= >actual &&
+ test_grep "gap.c:3: trailing whitespace" actual &&
+ test_grep "gap.c:8: trailing whitespace" actual
+'
+
+test_expect_success '--check does not report blank-at-eof outside the range' '
+ git checkout --orphan check-eof &&
+ git reset --hard &&
+ printf "void tracked()\n{\n return;\n}\n\nint tail = 1;\n" >eof.c &&
+ git add eof.c &&
+ test_tick &&
+ git commit -m "add eof.c" &&
+ # One commit introduces a trailing-whitespace error inside tracked()
+ # (line 3) and a blank line at end of file (line 7, outside the
+ # range). The blank-at-eof check scans the whole file, so it must be
+ # scoped: report the in-range error, not the out-of-range EOF blank.
+ printf "void tracked()\n{\n return; \n}\n\nint tail = 1;\n\n" >eof.c &&
+ git commit -a -m "ws in range, blank at eof out of range" &&
+ test_must_fail git log -L:tracked:eof.c --check --format= >actual &&
+ test_grep "eof.c:3: trailing whitespace" actual &&
+ test_grep ! "blank line at EOF" actual
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH 5/7] line-log: support diff stat formats with -L
From: Michael Montalbo via GitGitGadget @ 2026-06-18 18:16 UTC (permalink / raw)
To: git; +Cc: D. Ben Knoble, Michael Montalbo, Michael Montalbo
In-Reply-To: <pull.2152.git.1781806593.gitgitgadget@gmail.com>
From: Michael Montalbo <mmontalbo@gmail.com>
Reuse the line_range_filter in builtin_diffstat() to produce
range-scoped statistics. When a filepair carries line_ranges, the
filter wraps diffstat_consume() as its output callback, forwarding only
in-range lines for counting. flush_range_hunk() replays buffered
content through diffstat_consume(), which ignores synthetic @@ headers
since it only counts '+' and '-' lines.
Expand the output format allowlist in setup_revisions() to accept
--stat, --numstat, and --shortstat with -L.
Leave --dirstat out of the allowlist so it is rejected like any other
unsupported format. Its default mode counts each file's whole-file
byte damage via diffcore_count_changes(), outside the line-based
pipeline that the -L filter scopes, so bare --dirstat cannot honor the
tracked range. The --dirstat=lines mode could: it aggregates the same
per-file line counts as --numstat, which -L already scopes. But
accepting only that sub-mode while bare --dirstat keeps erroring is a
confusing split, so the whole format is deferred to a follow-up;
--numstat already reports the exact range-scoped per-file counts.
Also drop "yet" from the generic -L rejection message ("does not
yet support the requested diff format"). Some rejected formats do
not fit a line range at all, so "yet" wrongly implied they are all
just awaiting support.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
Documentation/line-range-options.adoc | 12 ++-
diff.c | 13 ++-
revision.c | 6 +-
t/t4211-line-log.sh | 150 ++++++++++++++++++++++----
4 files changed, 155 insertions(+), 26 deletions(-)
diff --git a/Documentation/line-range-options.adoc b/Documentation/line-range-options.adoc
index 72f639b5e7..1a25f55bb1 100644
--- a/Documentation/line-range-options.adoc
+++ b/Documentation/line-range-options.adoc
@@ -9,10 +9,14 @@
_<start>_ and _<end>_ (or _<funcname>_) must exist in the starting revision.
You can specify this option more than once. Implies `--patch`.
Patch output can be suppressed using `--no-patch`.
- Non-patch diff formats `--raw`, `--name-only`, `--name-status`,
- and `--summary` are supported. Diff stat formats
- (`--stat`, `--numstat`, `--shortstat`, `--dirstat`) are not
- currently implemented.
+ The following non-patch diff formats are supported: `--raw`,
+ `--name-only`, `--name-status`, `--summary`,
+ `--stat`, `--numstat`, and `--shortstat`.
+ The stat formats show range-scoped counts: only lines within
+ the tracked range are counted. `--dirstat` is not supported
+ with `-L`: it summarizes change as each directory's share of
+ the total churn, not as counts for the tracked lines. Use
+ `--numstat` for exact per-file counts within the range.
+
Patch formatting options such as `--word-diff`, `--color-moved`,
`--no-prefix`, and whitespace options (`-w`, `-b`) are supported,
diff --git a/diff.c b/diff.c
index 6233a96bf0..026fafeb90 100644
--- a/diff.c
+++ b/diff.c
@@ -4289,7 +4289,18 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
xecfg.ctxlen = o->context;
xecfg.interhunkctxlen = o->interhunkcontext;
xecfg.flags = XDL_EMIT_NO_HUNK_HDR;
- if (xdi_diff_outf(&mf1, &mf2, NULL,
+
+ if (p->line_ranges) {
+ struct line_range_filter lr_filter;
+
+ line_range_filter_init(&lr_filter, p->line_ranges,
+ diffstat_consume, diffstat);
+
+ if (line_range_filter_diff(&lr_filter, &mf1, &mf2,
+ &xpp, &xecfg))
+ die("unable to generate diffstat for %s",
+ one->path);
+ } else if (xdi_diff_outf(&mf1, &mf2, NULL,
diffstat_consume, diffstat, &xpp, &xecfg))
die("unable to generate diffstat for %s", one->path);
diff --git a/revision.c b/revision.c
index 6a8101e8b7..2c76e15778 100644
--- a/revision.c
+++ b/revision.c
@@ -3193,8 +3193,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
(revs->diffopt.output_format &
~(DIFF_FORMAT_PATCH | DIFF_FORMAT_NO_OUTPUT |
DIFF_FORMAT_RAW | DIFF_FORMAT_NAME |
- DIFF_FORMAT_NAME_STATUS | DIFF_FORMAT_SUMMARY))))
- die(_("-L does not yet support the requested diff format"));
+ DIFF_FORMAT_NAME_STATUS | DIFF_FORMAT_SUMMARY |
+ DIFF_FORMAT_NUMSTAT | DIFF_FORMAT_DIFFSTAT |
+ DIFF_FORMAT_SHORTSTAT))))
+ die(_("-L does not support the requested diff format"));
if (revs->expand_tabs_in_log < 0)
revs->expand_tabs_in_log = revs->expand_tabs_in_log_default;
diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh
index e9691066de..af37bd532f 100755
--- a/t/t4211-line-log.sh
+++ b/t/t4211-line-log.sh
@@ -176,24 +176,15 @@ test_expect_success '--name-status shows status and path' '
test_grep ! "^@@" actual
'
-test_expect_success '--stat is not yet supported with -L' '
- test_must_fail git log -L1,24:b.c --stat 2>err &&
- test_grep "does not yet support" err
-'
-
-test_expect_success '--numstat is not yet supported with -L' '
- test_must_fail git log -L1,24:b.c --numstat 2>err &&
- test_grep "does not yet support" err
-'
-
-test_expect_success '--shortstat is not yet supported with -L' '
- test_must_fail git log -L1,24:b.c --shortstat 2>err &&
- test_grep "does not yet support" err
-'
-
-test_expect_success '--dirstat is not yet supported with -L' '
+test_expect_success '--dirstat is not supported with -L' '
+ # --dirstat is not supported with -L: its default mode measures
+ # whole-file change, not the tracked lines, and the
+ # --dirstat=lines variant is deferred too, so both forms are
+ # rejected like any other unsupported format.
test_must_fail git log -L1,24:b.c --dirstat 2>err &&
- test_grep "does not yet support" err
+ test_grep "does not support" err &&
+ test_must_fail git log -L1,24:b.c --dirstat=lines 2>err &&
+ test_grep "does not support" err
'
test_expect_success 'setup for checking fancy rename following' '
@@ -887,9 +878,9 @@ test_expect_success '-L with -S suppresses non-matching commits' '
test_cmp expect actual
'
-test_expect_success '--full-diff is not yet supported with -L' '
+test_expect_success '--full-diff is not supported with -L' '
test_must_fail git log -L1,24:b.c --full-diff 2>err &&
- test_grep "does not yet support" err
+ test_grep "does not support" err
'
test_expect_success '-L --oneline has no extra blank line before diff' '
@@ -900,6 +891,127 @@ test_expect_success '-L --oneline has no extra blank line before diff' '
test_grep "^diff --git" line2
'
+test_expect_success 'setup for stat range-scoping tests' '
+ git checkout --orphan stat-scoping &&
+ git reset --hard &&
+ cat >file.c <<-\EOF &&
+ int func1()
+ {
+ return F1;
+ }
+
+ int func2()
+ {
+ return F2;
+ }
+ EOF
+ git add file.c &&
+ test_tick &&
+ git commit -m "Add func1() and func2()" &&
+
+ # Modify both functions in a single commit so that
+ # whole-file stats differ from range-scoped stats.
+ sed -e "s/F1/F1 + 1/" -e "s/F2/F2 + 2/" file.c >tmp &&
+ mv tmp file.c &&
+ git commit -a -m "Modify both functions"
+'
+
+test_expect_success '--numstat counts only lines in tracked range' '
+ # "Modify both functions" changes one line in func1 and one in
+ # func2. Whole-file numstat would show 2 added, 2 deleted.
+ # Range-scoped numstat for func2 should show only 1 and 1.
+ git log -L:func2:file.c --numstat --format=%s -1 >actual &&
+ test_grep "Modify both functions" actual &&
+ test_grep "^1 1 file.c$" actual &&
+ test_grep ! "^diff --git" actual
+'
+
+test_expect_success '--numstat counts only additions for root commit' '
+ # Root commit creates both func1 (4 lines) and func2 (4 lines).
+ # Whole-file numstat would show 9 lines added. Range-scoped
+ # numstat for func2 should show only 4.
+ git log -L:func2:file.c --numstat --format=%s >actual &&
+ test_grep "Add func1() and func2()" actual &&
+ test_grep "^4 0 file.c$" actual &&
+ test_grep ! "^diff --git" actual
+'
+
+test_expect_success '--stat counts only lines in tracked range' '
+ git log -L:func2:file.c --stat --format=%s -1 >actual &&
+ test_grep "Modify both functions" actual &&
+ test_grep "file.c |" actual &&
+ test_grep "1 insertion" actual &&
+ test_grep "1 deletion" actual &&
+ test_grep ! "^diff --git" actual
+'
+
+test_expect_success '--shortstat counts only lines in tracked range' '
+ # --shortstat prints only the summary line: no per-file "file.c |"
+ # line. Counts are range-scoped as for --numstat above.
+ git log -L:func2:file.c --shortstat --format=%s -1 >actual &&
+ test_grep "Modify both functions" actual &&
+ test_grep "1 insertion" actual &&
+ test_grep "1 deletion" actual &&
+ test_grep ! "file.c |" actual &&
+ test_grep ! "^diff --git" actual
+'
+
+test_expect_success '--numstat across renames and multiple commits' '
+ # parallel-change carries the tracked function f across an a.c -> b.c
+ # rename and a merge of two parallel histories. With -M, --numstat
+ # follows the rename and reports range-scoped (not whole-file)
+ # added/removed counts for f per commit; the file column flips from
+ # b.c to a.c at the rename as the walk goes back in time. Commits
+ # that do not change the range of f emit no row (the merge and the
+ # pure file-move produce nothing), so there are fewer rows than
+ # commits.
+ git checkout parallel-change &&
+ git log -M -L ":f:b.c" --format= --numstat >actual &&
+ cat >expect <<-\EOF &&
+ 1 1 b.c
+ 1 1 a.c
+ 1 1 a.c
+ 1 1 a.c
+ 1 0 a.c
+ 13 0 a.c
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success '-L multiple ranges with --numstat excludes untracked change' '
+ git checkout --orphan multi-range &&
+ git reset --hard &&
+ cat >m.c <<-\EOF &&
+ int func1()
+ {
+ return F1;
+ }
+
+ int func2()
+ {
+ return F2;
+ }
+
+ int func3()
+ {
+ return F3;
+ }
+ EOF
+ git add m.c &&
+ test_tick &&
+ git commit -m "add m.c" &&
+ # Change all three functions but track only func1 and func2.
+ # Whole-file numstat would be 3 3; a 2 2 result proves the
+ # untracked func3 change is excluded and the two ranges just sum.
+ sed -e "s/F1/F1 + 1/" -e "s/F2/F2 + 2/" -e "s/F3/F3 + 3/" m.c >tmp &&
+ mv tmp m.c &&
+ git commit -a -m "Modify all three functions" &&
+ git log -L:func1:m.c -L:func2:m.c --numstat --format=%s -1 >actual &&
+ test_grep "Modify all three functions" actual &&
+ test_grep "^2 2 m.c$" actual &&
+ test_grep ! "^3 3 m.c$" actual
+'
+
test_expect_success '--summary shows new file on root commit' '
git checkout parent-oids &&
git log -L:func2:file.c --summary --format= >actual &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH 4/7] diff: extract a line-range diff helper for reuse
From: Michael Montalbo via GitGitGadget @ 2026-06-18 18:16 UTC (permalink / raw)
To: git; +Cc: D. Ben Knoble, Michael Montalbo, Michael Montalbo
In-Reply-To: <pull.2152.git.1781806593.gitgitgadget@gmail.com>
From: Michael Montalbo <mmontalbo@gmail.com>
builtin_diff() open-codes the line-range filter setup and teardown
around its xdi_diff_outf() call: zero the struct, point it at the
output callback, inflate ctxlen to the largest range span so each range
yields a single xdiff hunk, run the diff, flush the trailing range
hunk, and release the buffer. The upcoming -L stat and check formats
need the same sequence.
Extract line_range_filter_init() for the setup and a
line_range_filter_diff() helper that prepares the xdiff config the
filter needs, runs an initialized filter through xdi_diff_outf(),
flushes the final range hunk, and releases it, returning the latched
error. The helper inflates ctxlen to the largest range span so each
range yields a single xdiff hunk, and clears XDL_EMIT_NO_HUNK_HDR so
the hunk headers the filter seeds its position from are always emitted.
Folding both into the helper keeps these invariants, which the filter's
position tracking relies on, in a single place for every consumer.
builtin_diff() now does init + line_range_filter_diff(); the next two
patches reuse them in builtin_diffstat() and builtin_checkdiff()
instead of repeating the boilerplate.
No behavior change: builtin_diff() leaves XDL_EMIT_NO_HUNK_HDR unset,
so clearing it is a no-op until the suppressing consumers arrive.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
diff.c | 100 +++++++++++++++++++++++++++++++++++----------------------
1 file changed, 61 insertions(+), 39 deletions(-)
diff --git a/diff.c b/diff.c
index 9751bb6798..6233a96bf0 100644
--- a/diff.c
+++ b/diff.c
@@ -2580,6 +2580,18 @@ static int quick_consume(void *priv, char *line UNUSED, unsigned long len UNUSED
return 1;
}
+static void line_range_filter_init(struct line_range_filter *filter,
+ const struct range_set *ranges,
+ xdiff_emit_line_fn line_fn,
+ void *cb_data)
+{
+ memset(filter, 0, sizeof(*filter));
+ filter->orig_line_fn = line_fn;
+ filter->orig_cb_data = cb_data;
+ filter->ranges = ranges;
+ strbuf_init(&filter->hunk.lines, 0);
+}
+
/*
* Begin a range hunk at the first in-range line. Its position fixes the
* hunk's begins, taken from the two image cursors before they advance:
@@ -2744,6 +2756,50 @@ static int line_range_line_fn(void *priv, char *line, unsigned long len)
return filter->ret;
}
+/*
+ * Run an xdiff pass through an initialized line-range filter, flush the
+ * final range hunk, and release the filter. Inflates ctxlen to the largest
+ * range span first, so that every change within a single range lands in one
+ * xdiff hunk and the inter-change context is emitted; the filter then clips
+ * back to range boundaries. The optimal ctxlen depends on where changes fall
+ * within the range, which is only known after xdiff runs, so the max span is
+ * the upper bound that guarantees correctness in a single pass. Every
+ * consumer (patch, diffstat, check) relies on one xdiff hunk per range, so
+ * this lives here rather than at each call site. Also clears
+ * XDL_EMIT_NO_HUNK_HDR: the filter seeds its per-image position from the hunk
+ * headers, so a consumer that otherwise suppresses them (diffstat) still gets
+ * them here. Returns non-zero if xdiff or any forwarded callback failed.
+ */
+static int line_range_filter_diff(struct line_range_filter *filter,
+ mmfile_t *mf1, mmfile_t *mf2,
+ xpparam_t *xpp, xdemitconf_t *xecfg)
+{
+ const struct range_set *ranges = filter->ranges;
+ long max_span = 0;
+ unsigned int i;
+ int ret;
+
+ for (i = 0; i < ranges->nr; i++) {
+ long span = ranges->ranges[i].end - ranges->ranges[i].start;
+ if (span > max_span)
+ max_span = span;
+ }
+ if (max_span > xecfg->ctxlen)
+ xecfg->ctxlen = max_span;
+
+ /* the filter seeds its per-image position from hunk headers */
+ xecfg->flags &= ~XDL_EMIT_NO_HUNK_HDR;
+
+ ret = xdi_diff_outf(mf1, mf2, line_range_hunk_fn,
+ line_range_line_fn, filter, xpp, xecfg);
+ if (!ret) {
+ flush_range_hunk(filter);
+ ret = filter->ret;
+ }
+ strbuf_release(&filter->hunk.lines);
+ return ret;
+}
+
static void pprint_rename(struct strbuf *name, const char *a, const char *b)
{
const char *old_name = a;
@@ -4108,49 +4164,15 @@ static void builtin_diff(const char *name_a,
xdi_diff_outf(&mf1, &mf2, NULL, quick_consume,
&ecbdata, &xpp, &xecfg);
} else if (line_ranges) {
- struct line_range_filter lr_state;
- unsigned int i;
- long max_span = 0;
+ struct line_range_filter lr_filter;
- memset(&lr_state, 0, sizeof(lr_state));
- lr_state.orig_line_fn = fn_out_consume;
- lr_state.orig_cb_data = &ecbdata;
- lr_state.ranges = line_ranges;
- strbuf_init(&lr_state.hunk.lines, 0);
-
- /*
- * Inflate ctxlen so that all changes within
- * any single range are merged into one xdiff
- * hunk and the inter-change context is emitted.
- * The callback clips back to range boundaries.
- *
- * The optimal ctxlen depends on where changes
- * fall within the range, which is only known
- * after xdiff runs; the max range span is the
- * upper bound that guarantees correctness in a
- * single pass.
- */
- for (i = 0; i < line_ranges->nr; i++) {
- long span = line_ranges->ranges[i].end -
- line_ranges->ranges[i].start;
- if (span > max_span)
- max_span = span;
- }
- if (max_span > xecfg.ctxlen)
- xecfg.ctxlen = max_span;
-
- if (xdi_diff_outf(&mf1, &mf2,
- line_range_hunk_fn,
- line_range_line_fn,
- &lr_state, &xpp, &xecfg))
- die("unable to generate diff for %s",
- one->path);
+ line_range_filter_init(&lr_filter, line_ranges,
+ fn_out_consume, &ecbdata);
- flush_range_hunk(&lr_state);
- if (lr_state.ret)
+ if (line_range_filter_diff(&lr_filter, &mf1, &mf2,
+ &xpp, &xecfg))
die("unable to generate diff for %s",
one->path);
- strbuf_release(&lr_state.hunk.lines);
} else if (xdi_diff_outf(&mf1, &mf2, NULL, fn_out_consume,
&ecbdata, &xpp, &xecfg))
die("unable to generate diff for %s", one->path);
--
gitgitgadget
^ permalink raw reply related
* [PATCH 3/7] diff: emit -L hunk headers via xdiff's formatter
From: Michael Montalbo via GitGitGadget @ 2026-06-18 18:16 UTC (permalink / raw)
To: git; +Cc: D. Ben Knoble, Michael Montalbo, Michael Montalbo
In-Reply-To: <pull.2152.git.1781806593.gitgitgadget@gmail.com>
From: Michael Montalbo <mmontalbo@gmail.com>
The line-range filter builds its own "@@ -<old> +<new> @@" header for
each range hunk. For a side with no lines (count 0, such as the old
side of a pure insertion), the begin should be the number of the line
before the change, per the convention git diff and xdl_emit_hunk_hdr()
follow. The hand-rolled code's begin was one too high; in t4211 this
produced
@@ -25,0 +18,9 @@
an old begin of 25 in a 24-line file, where git diff would give 24.
Stop hand-rolling the header. flush_range_hunk() now formats it through
xdiff's own emitter: a new xdiff_emit_hunk_header() helper wraps
xdl_emit_hunk_hdr(), the function that produces every other diff's hunk
headers. The count-0 begin is then correct by construction, and as a
side effect -L headers match git diff exactly, including its omission of
a count of 1 ("@@ -22 +22 @@" rather than "@@ -22,1 +22,1 @@").
xdiff's hunk callback already hands line_range_hunk_fn() a count-0 begin
decremented, so undo that when seeding the cursors and let the formatter
re-apply the convention once, at emit time.
The off-by-one predates this series, and the two regenerated fixtures
reach it from different origins: no-assertion-error has carried it since
its test was added in ab60c693a2 (line-log: fix assertion error,
2025-08-18), while vanishes-early acquired it when 86e986f166 (line-log:
route -L output through the standard diff pipeline) reshaped its tracked
line into a pure insertion. vanishes-early also drops its count-1
counts.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
diff.c | 27 +++++++++++-------------
t/t4211/sha1/expect.no-assertion-error | 2 +-
t/t4211/sha1/expect.vanishes-early | 6 +++---
t/t4211/sha256/expect.no-assertion-error | 2 +-
t/t4211/sha256/expect.vanishes-early | 6 +++---
xdiff-interface.c | 19 +++++++++++++++++
xdiff-interface.h | 15 +++++++++++++
7 files changed, 54 insertions(+), 23 deletions(-)
diff --git a/diff.c b/diff.c
index ee765d7ac2..9751bb6798 100644
--- a/diff.c
+++ b/diff.c
@@ -2636,14 +2636,9 @@ static void flush_range_hunk(struct line_range_filter *filter)
return;
}
- strbuf_addf(&hdr, "@@ -%ld,%ld +%ld,%ld @@",
- filter->hunk.old_begin, old_count,
- filter->hunk.new_begin, new_count);
- if (filter->funclen > 0) {
- strbuf_addch(&hdr, ' ');
- strbuf_add(&hdr, filter->func, filter->funclen);
- }
- strbuf_addch(&hdr, '\n');
+ xdiff_emit_hunk_header(&hdr, filter->hunk.old_begin, old_count,
+ filter->hunk.new_begin, new_count,
+ filter->func, filter->funclen);
filter->ret = filter->orig_line_fn(filter->orig_cb_data, hdr.buf, hdr.len);
strbuf_release(&hdr);
@@ -2668,19 +2663,21 @@ static void flush_range_hunk(struct line_range_filter *filter)
}
static void line_range_hunk_fn(void *data,
- long old_begin, long old_nr UNUSED,
- long new_begin, long new_nr UNUSED,
+ long old_begin, long old_nr,
+ long new_begin, long new_nr,
const char *func, long funclen)
{
struct line_range_filter *filter = data;
/*
- * When count > 0, begin is 1-based. When count == 0, begin is
- * adjusted down by 1 by xdl_emit_hunk_hdr(), but no lines of
- * that type will arrive, so the value is unused.
+ * Seed the per-image line cursors from the hunk header's begins. For
+ * a side with no lines (count 0), xdiff's callback has already moved
+ * its begin to the line before the change, so add one back to recover
+ * the true 1-based start. xdiff_emit_hunk_header() reapplies that -1
+ * when the clipped hunk is emitted.
*/
- filter->lno_in_postimage = new_begin;
- filter->lno_in_preimage = old_begin;
+ filter->lno_in_postimage = new_nr ? new_begin : new_begin + 1;
+ filter->lno_in_preimage = old_nr ? old_begin : old_begin + 1;
if (funclen > 0) {
if (funclen > (long)sizeof(filter->func))
diff --git a/t/t4211/sha1/expect.no-assertion-error b/t/t4211/sha1/expect.no-assertion-error
index 54c568f273..95faf51a7b 100644
--- a/t/t4211/sha1/expect.no-assertion-error
+++ b/t/t4211/sha1/expect.no-assertion-error
@@ -8,7 +8,7 @@ diff --git a/b.c b/b.c
index bf79c2f..27c829c 100644
--- a/b.c
+++ b/b.c
-@@ -25,0 +18,9 @@
+@@ -24,0 +18,9 @@
+long f(long x)
+{
+ int s = 0;
diff --git a/t/t4211/sha1/expect.vanishes-early b/t/t4211/sha1/expect.vanishes-early
index a413ad3659..e4b1a201d5 100644
--- a/t/t4211/sha1/expect.vanishes-early
+++ b/t/t4211/sha1/expect.vanishes-early
@@ -8,7 +8,7 @@ diff --git a/a.c b/a.c
index 0b9cae5..5de3ea4 100644
--- a/a.c
+++ b/a.c
-@@ -23,0 +24,1 @@ int main ()
+@@ -22,0 +24 @@ int main ()
+/* incomplete lines are bad! */
commit 100b61a6f2f720f812620a9d10afb3a960ccb73c
@@ -21,7 +21,7 @@ diff --git a/a.c b/a.c
index 5e709a1..0b9cae5 100644
--- a/a.c
+++ b/a.c
-@@ -22,1 +22,1 @@ int main ()
+@@ -22 +22 @@ int main ()
-}
+}
\ No newline at end of file
@@ -37,5 +37,5 @@ new file mode 100644
index 0000000..444e415
--- /dev/null
+++ b/a.c
-@@ -0,0 +20,1 @@
+@@ -0,0 +20 @@
+}
diff --git a/t/t4211/sha256/expect.no-assertion-error b/t/t4211/sha256/expect.no-assertion-error
index c25f2ce19c..815d27f7f1 100644
--- a/t/t4211/sha256/expect.no-assertion-error
+++ b/t/t4211/sha256/expect.no-assertion-error
@@ -8,7 +8,7 @@ diff --git a/b.c b/b.c
index 69cb69c..a0d566e 100644
--- a/b.c
+++ b/b.c
-@@ -25,0 +18,9 @@
+@@ -24,0 +18,9 @@
+long f(long x)
+{
+ int s = 0;
diff --git a/t/t4211/sha256/expect.vanishes-early b/t/t4211/sha256/expect.vanishes-early
index bc33b963dc..263fc9eaac 100644
--- a/t/t4211/sha256/expect.vanishes-early
+++ b/t/t4211/sha256/expect.vanishes-early
@@ -8,7 +8,7 @@ diff --git a/a.c b/a.c
index e4fa1d8..62c1fc2 100644
--- a/a.c
+++ b/a.c
-@@ -23,0 +24,1 @@ int main ()
+@@ -22,0 +24 @@ int main ()
+/* incomplete lines are bad! */
commit 29f32ac3141c48b22803e5c4127b719917b67d0f8ca8c5248bebfa2a19f7da10
@@ -21,7 +21,7 @@ diff --git a/a.c b/a.c
index d325124..e4fa1d8 100644
--- a/a.c
+++ b/a.c
-@@ -22,1 +22,1 @@ int main ()
+@@ -22 +22 @@ int main ()
-}
+}
\ No newline at end of file
@@ -37,5 +37,5 @@ new file mode 100644
index 0000000..9f550c3
--- /dev/null
+++ b/a.c
-@@ -0,0 +20,1 @@
+@@ -0,0 +20 @@
+}
diff --git a/xdiff-interface.c b/xdiff-interface.c
index 5ee2b96d0a..32e04630ee 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -91,6 +91,25 @@ static int xdiff_outf(void *priv_, mmbuffer_t *mb, int nbuf)
return 0;
}
+static int strbuf_out_line(void *priv, mmbuffer_t *mb, int nbuf)
+{
+ struct strbuf *out = priv;
+ int i;
+ for (i = 0; i < nbuf; i++)
+ strbuf_add(out, mb[i].ptr, mb[i].size);
+ return 0;
+}
+
+void xdiff_emit_hunk_header(struct strbuf *out,
+ long old_begin, long old_count,
+ long new_begin, long new_count,
+ const char *func, long funclen)
+{
+ xdemitcb_t ecb = { .priv = out, .out_line = strbuf_out_line };
+ xdl_emit_hunk_hdr(old_begin, old_count, new_begin, new_count,
+ func, funclen, &ecb);
+}
+
/*
* Trim down common substring at the end of the buffers,
* but end on a complete line.
diff --git a/xdiff-interface.h b/xdiff-interface.h
index ce54e1c0e0..51c88296ed 100644
--- a/xdiff-interface.h
+++ b/xdiff-interface.h
@@ -76,4 +76,19 @@ int xdiff_compare_lines(const char *l1, long s1,
*/
unsigned long xdiff_hash_string(const char *s, size_t len, long flags);
+struct strbuf;
+
+/*
+ * Append a unified-diff hunk header to `out`, e.g.
+ * "@@ -<old> +<new> @@ func\n". The header comes from wrapping xdiff's
+ * own hunk-header emitter, so it matches what a normal diff would
+ * produce for these begins and counts. For a side with no lines
+ * (count 0) the begin is the line before the change, and a count of 1
+ * is omitted.
+ */
+void xdiff_emit_hunk_header(struct strbuf *out,
+ long old_begin, long old_count,
+ long new_begin, long new_count,
+ const char *func, long funclen);
+
#endif
--
gitgitgadget
^ permalink raw reply related
* [PATCH 2/7] diff: simplify the line-range filter by classifying removals immediately
From: Michael Montalbo via GitGitGadget @ 2026-06-18 18:16 UTC (permalink / raw)
To: git; +Cc: D. Ben Knoble, Michael Montalbo, Michael Montalbo
In-Reply-To: <pull.2152.git.1781806593.gitgitgadget@gmail.com>
From: Michael Montalbo <mmontalbo@gmail.com>
The filter buffered '-' lines in a pending_rm strbuf, deferring their
classification until a '+' or ' ' line revealed the post-image
position. That buffering is unnecessary: a removal occupies no
post-image line, so it does not advance lno_in_postimage, and xdiff
emits removals before additions within a change. A '-' therefore
arrives while lno_in_postimage already holds the index the following
'+'/' ' will occupy, and can be classified against the ranges as it
arrives.
The buffering also hid a bug: flush_range_hunk() drained pending_rm into
the range hunk whenever the hunk was active, even after lno_in_postimage
had advanced past the tracked range, so a deletion just after the
tracked function leaked into the patch. Classifying each line as it
arrives removes the pending_rm buffer, the discard_pending_rm() helper,
three struct fields, and makes that bug impossible by construction.
With every line classified on arrival, the buffered lines are the
hunk's single source of truth, so the old/new counts need not be kept
alongside them: flush_range_hunk() derives the counts (and whether the
hunk holds any change) from the buffer when it builds the header. Drop
the per-line counting and the old_count, new_count, and has_changes
fields; there is no longer a second tally that could fall out of sync
with the buffer.
Add begin_range_hunk() to open the accumulator at the first in-range
line, seeding both begins from the live image cursors, as the
counterpart to flush_range_hunk(). With the counting gone too,
line_range_line_fn() now only appends an in-range line.
Document the coordinate model: a block comment on struct
line_range_filter states it (the pre/post-image cursors, the 0-based
idx_in_postimage, removals classified by the following line) with a
worked example.
Add tests for the leaked trailing deletion this fixes, the symmetric
leading-deletion case, and the filter's range boundaries (a change at
the first and last line of a range, and a pure in-range deletion).
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
diff.c | 215 ++++++++++++++++++++++++--------------------
t/t4211-line-log.sh | 125 ++++++++++++++++++++++++++
2 files changed, 243 insertions(+), 97 deletions(-)
diff --git a/diff.c b/diff.c
index 1e043c959f..ee765d7ac2 100644
--- a/diff.c
+++ b/diff.c
@@ -610,18 +610,58 @@ struct emit_callback {
};
/*
- * State for the line-range callback wrappers that sit between
- * xdi_diff_outf() and fn_out_consume(). xdiff produces a normal,
- * unfiltered diff; the wrappers intercept each hunk header and line,
- * track post-image position, and forward only lines that fall within
- * the requested ranges. Contiguous in-range lines are collected into
- * range hunks and flushed with a synthetic @@ header so that
- * fn_out_consume() sees well-formed unified-diff fragments.
+ * Line-range filter: scopes "git log -L" output to the tracked ranges.
*
- * Removal lines ('-') cannot be classified by post-image position, so
- * they are buffered in pending_rm until the next '+' or ' ' line
- * reveals whether they precede an in-range line (flush into range hunk) or
- * an out-of-range line (discard).
+ * It sits between xdi_diff_outf() and an output callback (fn_out_consume,
+ * diffstat_consume, checkdiff_consume). xdiff produces a normal diff; the
+ * filter forwards only the lines inside the requested ranges, collecting
+ * contiguous in-range lines into a "range hunk" emitted with a synthetic
+ * @@ header so the callback sees well-formed unified-diff fragments.
+ *
+ * A diff describes the change from a pre-image to a post-image. Each
+ * line is context (' ', in both), a removal ('-', pre-image only), or
+ * an addition ('+', post-image only). -L tracks ranges in the
+ * post-image, so a line is in range by its post-image position.
+ *
+ * Two 1-based cursors track the next line in each image, named as in
+ * struct emit_callback and seeded from the xdiff hunk header:
+ *
+ * lno_in_postimage advances on '+' and ' ' (lines in the post-image)
+ * lno_in_preimage advances on '-' and ' ' (lines in the pre-image)
+ *
+ * Ranges are 0-based half-open [start, end), so a line is tested at the
+ * 0-based index idx_in_postimage = lno_in_postimage - 1.
+ *
+ * A '-' is not present in the post-image, so it has no post-image line
+ * number of its own. Since it does not advance lno_in_postimage, it is
+ * classified at the idx_in_postimage that the following '+'/' ' will
+ * occupy. xdiff emits a change's removals before its additions, so that
+ * index is already known when the '-' arrives.
+ *
+ * The synthetic "@@ -<old> +<new> @@" header has two sides, old (the
+ * pre-image) and new (the post-image), matching the xdiff_emit_hunk_fn
+ * callback; the hunk.old_begin / hunk.new_begin fields below hold those
+ * begins, and flush_range_hunk() derives the counts from the buffered
+ * lines.
+ *
+ * Example, tracking post-image line 2 (range [1, 2)) of:
+ *
+ * pre-image post-image
+ * 1 a 1 a
+ * 2 b 2 X (b -> X)
+ * 3 c 3 c
+ *
+ * classify each line by idx_in_postimage. The pre and post columns
+ * are each cursor's value while that line is classified, i.e. before
+ * the line advances them (pre = lno_in_preimage,
+ * post = lno_in_postimage, idx = idx_in_postimage):
+ * ' a' pre 1 post 1 idx 0 -> before start, skip
+ * '-b' pre 2 post 2 idx 1 -> keep (removal)
+ * '+X' pre 3 post 2 idx 1 -> keep (addition)
+ * ' c' pre 3 post 3 idx 2 -> past end, flush
+ *
+ * -b and +X share idx = 1 because -b did not advance lno_in_postimage;
+ * both land in the range hunk, flushed when ' c' crosses the range end.
*/
struct line_range_filter {
xdiff_emit_line_fn orig_line_fn;
@@ -640,20 +680,18 @@ struct line_range_filter {
char func[80];
long funclen;
- /* The range hunk being accumulated for the current range. */
+ /*
+ * The range hunk being accumulated. At most one is live at a time:
+ * it is flushed and reset as the cursor leaves each range (and once
+ * more at end of diff), then reused for the next range.
+ */
struct {
struct strbuf lines; /* buffered in-range diff lines */
- long old_begin, old_count;
- long new_begin, new_count;
+ long old_begin;
+ long new_begin;
int active;
- int has_changes; /* any '+' or '-' line? */
} hunk;
- /* Removal lines not yet known to be in-range */
- struct strbuf pending_rm;
- int pending_rm_count;
- long pending_rm_pre_begin; /* pre-image line of first pending */
-
int ret; /* latched error from orig_line_fn */
};
@@ -2542,26 +2580,48 @@ static int quick_consume(void *priv, char *line UNUSED, unsigned long len UNUSED
return 1;
}
-static void discard_pending_rm(struct line_range_filter *filter)
+/*
+ * Begin a range hunk at the first in-range line. Its position fixes the
+ * hunk's begins, taken from the two image cursors before they advance:
+ * new_begin from the post-image, old_begin from the pre-image. The line
+ * counts are not tracked here; flush_range_hunk() derives them from the
+ * buffered lines.
+ */
+static void begin_range_hunk(struct line_range_filter *filter)
{
- strbuf_reset(&filter->pending_rm);
- filter->pending_rm_count = 0;
+ filter->hunk.active = 1;
+ filter->hunk.new_begin = filter->lno_in_postimage;
+ filter->hunk.old_begin = filter->lno_in_preimage;
+ strbuf_reset(&filter->hunk.lines);
}
static void flush_range_hunk(struct line_range_filter *filter)
{
struct strbuf hdr = STRBUF_INIT;
const char *p, *end;
+ long old_count = 0, new_count = 0;
+ int has_changes = 0;
if (!filter->hunk.active || filter->ret)
return;
- /* Drain any pending removal lines into the range hunk */
- if (filter->pending_rm_count) {
- strbuf_addbuf(&filter->hunk.lines, &filter->pending_rm);
- filter->hunk.old_count += filter->pending_rm_count;
- filter->hunk.has_changes = 1;
- discard_pending_rm(filter);
+ /*
+ * Derive the hunk's geometry from the buffered lines: a ' '
+ * counts on both sides, a '-' on the old side, a '+' on the new.
+ * A '-' or '+' marks a real change; the "\ No newline at end of
+ * file" marker (line[0] == '\\') counts on neither side.
+ */
+ p = filter->hunk.lines.buf;
+ end = p + filter->hunk.lines.len;
+ while (p < end) {
+ const char *eol = memchr(p, '\n', end - p);
+ if (*p == ' ' || *p == '-')
+ old_count++;
+ if (*p == ' ' || *p == '+')
+ new_count++;
+ if (*p == '-' || *p == '+')
+ has_changes = 1;
+ p = eol ? eol + 1 : end;
}
/*
@@ -2570,15 +2630,15 @@ static void flush_range_hunk(struct line_range_filter *filter)
* ctxlen causes xdiff to emit context covering a range that
* has no changes in this commit.
*/
- if (!filter->hunk.has_changes) {
+ if (!has_changes) {
filter->hunk.active = 0;
strbuf_reset(&filter->hunk.lines);
return;
}
strbuf_addf(&hdr, "@@ -%ld,%ld +%ld,%ld @@",
- filter->hunk.old_begin, filter->hunk.old_count,
- filter->hunk.new_begin, filter->hunk.new_count);
+ filter->hunk.old_begin, old_count,
+ filter->hunk.new_begin, new_count);
if (filter->funclen > 0) {
strbuf_addch(&hdr, ' ');
strbuf_add(&hdr, filter->func, filter->funclen);
@@ -2618,11 +2678,6 @@ static void line_range_hunk_fn(void *data,
* When count > 0, begin is 1-based. When count == 0, begin is
* adjusted down by 1 by xdl_emit_hunk_hdr(), but no lines of
* that type will arrive, so the value is unused.
- *
- * Any pending removal lines from the previous xdiff hunk are
- * intentionally left in pending_rm: the line callback will
- * flush or discard them when the next content line reveals
- * whether the removals precede in-range content.
*/
filter->lno_in_postimage = new_begin;
filter->lno_in_preimage = old_begin;
@@ -2638,88 +2693,56 @@ static void line_range_hunk_fn(void *data,
static int line_range_line_fn(void *priv, char *line, unsigned long len)
{
struct line_range_filter *filter = priv;
- const struct range *cur;
- long idx_in_postimage, cur_pre;
+ long idx_in_postimage;
+ int in_range;
if (filter->ret)
return filter->ret;
- if (line[0] == '-') {
- if (!filter->pending_rm_count)
- filter->pending_rm_pre_begin = filter->lno_in_preimage;
- filter->lno_in_preimage++;
- strbuf_add(&filter->pending_rm, line, len);
- filter->pending_rm_count++;
- return filter->ret;
- }
-
if (line[0] == '\\') {
- if (filter->pending_rm_count)
- strbuf_add(&filter->pending_rm, line, len);
- else if (filter->hunk.active)
+ if (filter->hunk.active)
strbuf_add(&filter->hunk.lines, line, len);
- /* otherwise outside tracked range; drop silently */
return filter->ret;
}
- if (line[0] != '+' && line[0] != ' ')
+ if (line[0] != '+' && line[0] != ' ' && line[0] != '-')
BUG("unexpected diff line type '%c'", line[0]);
+ /*
+ * idx_in_postimage is this line's 0-based post-image index (see the model on
+ * struct line_range_filter). The cursors are advanced only after
+ * the line is classified, so a '-' is tested at the same idx_in_postimage as
+ * the '+'/' ' that follows it.
+ */
idx_in_postimage = filter->lno_in_postimage - 1;
- cur_pre = filter->lno_in_preimage; /* save before advancing for context lines */
- filter->lno_in_postimage++;
- if (line[0] == ' ')
- filter->lno_in_preimage++;
- /* Advance past ranges we've passed */
+ /* Retire ranges we have passed, flushing the one we leave. */
while (filter->cur_range < filter->ranges->nr &&
idx_in_postimage >= filter->ranges->ranges[filter->cur_range].end) {
if (filter->hunk.active)
flush_range_hunk(filter);
- discard_pending_rm(filter);
filter->cur_range++;
}
- /* Past all ranges */
- if (filter->cur_range >= filter->ranges->nr) {
- discard_pending_rm(filter);
- return filter->ret;
- }
+ in_range = filter->cur_range < filter->ranges->nr &&
+ idx_in_postimage >= filter->ranges->ranges[filter->cur_range].start &&
+ idx_in_postimage < filter->ranges->ranges[filter->cur_range].end;
- cur = &filter->ranges->ranges[filter->cur_range];
+ if (in_range) {
+ if (!filter->hunk.active)
+ begin_range_hunk(filter);
- /* Before current range */
- if (idx_in_postimage < cur->start) {
- discard_pending_rm(filter);
- return filter->ret;
+ strbuf_add(&filter->hunk.lines, line, len);
}
- /* In range so start a new range hunk if needed */
- if (!filter->hunk.active) {
- filter->hunk.active = 1;
- filter->hunk.has_changes = 0;
- filter->hunk.new_begin = idx_in_postimage + 1;
- filter->hunk.old_begin = filter->pending_rm_count
- ? filter->pending_rm_pre_begin : cur_pre;
- filter->hunk.old_count = 0;
- filter->hunk.new_count = 0;
- strbuf_reset(&filter->hunk.lines);
- }
-
- /* Flush pending removals into range hunk */
- if (filter->pending_rm_count) {
- strbuf_addbuf(&filter->hunk.lines, &filter->pending_rm);
- filter->hunk.old_count += filter->pending_rm_count;
- filter->hunk.has_changes = 1;
- discard_pending_rm(filter);
- }
-
- strbuf_add(&filter->hunk.lines, line, len);
- filter->hunk.new_count++;
- if (line[0] == '+')
- filter->hunk.has_changes = 1;
- else
- filter->hunk.old_count++;
+ /*
+ * Advance each image's cursor: a line present in that image (see
+ * the model) consumes one of its line numbers.
+ */
+ if (line[0] != '-')
+ filter->lno_in_postimage++;
+ if (line[0] != '+')
+ filter->lno_in_preimage++;
return filter->ret;
}
@@ -4097,7 +4120,6 @@ static void builtin_diff(const char *name_a,
lr_state.orig_cb_data = &ecbdata;
lr_state.ranges = line_ranges;
strbuf_init(&lr_state.hunk.lines, 0);
- strbuf_init(&lr_state.pending_rm, 0);
/*
* Inflate ctxlen so that all changes within
@@ -4132,7 +4154,6 @@ static void builtin_diff(const char *name_a,
die("unable to generate diff for %s",
one->path);
strbuf_release(&lr_state.hunk.lines);
- strbuf_release(&lr_state.pending_rm);
} else if (xdi_diff_outf(&mf1, &mf2, NULL, fn_out_consume,
&ecbdata, &xpp, &xecfg))
die("unable to generate diff for %s", one->path);
diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh
index ca4eb7bbc7..e9691066de 100755
--- a/t/t4211-line-log.sh
+++ b/t/t4211-line-log.sh
@@ -738,6 +738,131 @@ test_expect_success '-L with -G filters to diff-text matches' '
grep "F2 + 2" actual
'
+test_expect_success 'setup for trailing deletion test' '
+ git checkout --orphan trailing-del &&
+ git reset --hard &&
+ cat >file.c <<-\EOF &&
+ void tracked()
+ {
+ return 1;
+ }
+ // trailing comment
+ EOF
+ git add file.c &&
+ test_tick &&
+ git commit -m "add file with trailing comment" &&
+ # Modify tracked() AND delete the trailing comment in
+ # one commit, so the commit touches the tracked range
+ # and is not filtered out by the revision walker.
+ cat >file.c <<-\EOF &&
+ void tracked()
+ {
+ return 2;
+ }
+ EOF
+ git commit -a -m "modify tracked and delete trailing comment"
+'
+
+test_expect_success '-L does not include deletions past end of tracked range' '
+ git log -L:tracked:file.c --format= -1 -p >actual &&
+ # The trailing comment deletion is outside the tracked
+ # range and should not appear in the patch output.
+ test_grep "return 2" actual &&
+ test_grep ! "trailing comment" actual
+'
+
+test_expect_success '-L includes leading deletions resolved by in-range line' '
+ git checkout --orphan leading-del &&
+ git reset --hard &&
+ cat >file.c <<-\EOF &&
+ // leading comment
+ void tracked()
+ {
+ return 1;
+ }
+ EOF
+ git add file.c &&
+ test_tick &&
+ git commit -m "add file with leading comment" &&
+ cat >file.c <<-\EOF &&
+ void tracked()
+ {
+ return 2;
+ }
+ EOF
+ git commit -a -m "modify tracked and delete leading comment" &&
+ git log -L:tracked:file.c --format= -1 -p >actual &&
+ # The leading comment deletion is resolved by the next
+ # non-removal line (void tracked), which is in range: a
+ # removal is classified by the position of the following
+ # line, so it joins the range that line falls in.
+ test_grep "return 2" actual &&
+ test_grep "leading comment" actual
+'
+
+test_expect_success 'setup for line-range filter edge cases' '
+ git checkout --orphan filter-edge &&
+ git reset --hard &&
+ cat >file.c <<-\EOF &&
+ void before()
+ {
+ return 0;
+ }
+
+ void tracked()
+ {
+ int a = 1;
+ int b = 2;
+ int c = 3;
+ return a + b + c;
+ }
+
+ void after()
+ {
+ return 9;
+ }
+ EOF
+ git add file.c &&
+ test_tick &&
+ git commit -m "initial"
+'
+
+test_expect_success '-L change at exact first line of range' '
+ git checkout filter-edge &&
+ # Change the function signature (first line of range)
+ sed "s/void tracked/int tracked/" file.c >tmp &&
+ mv tmp file.c &&
+ git commit -a -m "change first line" &&
+ git log -L:tracked:file.c -p --format=%s -1 >actual &&
+ test_grep "change first line" actual &&
+ test_grep "+int tracked" actual &&
+ test_grep "\\-void tracked" actual
+'
+
+test_expect_success '-L change at exact last line of range' '
+ git checkout filter-edge &&
+ git reset --hard HEAD~1 &&
+ # Change the closing brace line (last line of range)
+ sed "s/^}$/} \/\/ end tracked/" file.c >tmp &&
+ mv tmp file.c &&
+ git commit -a -m "change last line" &&
+ git log -L:tracked:file.c -p --format=%s -1 >actual &&
+ test_grep "change last line" actual &&
+ test_grep "end tracked" actual
+'
+
+test_expect_success '-L pure deletion in range (no additions)' '
+ git checkout filter-edge &&
+ git reset --hard HEAD~1 &&
+ # Delete a line inside tracked() without adding anything
+ sed "/int c/d" file.c >tmp &&
+ mv tmp file.c &&
+ git commit -a -m "pure deletion" &&
+ git log -L:tracked:file.c -p --format=%s -1 >actual &&
+ test_grep "pure deletion" actual &&
+ test_grep "\\-.*int c" actual
+'
+
test_expect_success '-L with --diff-filter=M excludes root commit' '
git checkout parent-oids &&
git log -L:func2:file.c --diff-filter=M --format=%s --no-patch >actual &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH 1/7] diff: rename and group the line-range filter for clarity
From: Michael Montalbo via GitGitGadget @ 2026-06-18 18:16 UTC (permalink / raw)
To: git; +Cc: D. Ben Knoble, Michael Montalbo, Michael Montalbo
In-Reply-To: <pull.2152.git.1781806593.gitgitgadget@gmail.com>
From: Michael Montalbo <mmontalbo@gmail.com>
The line-range filter that mm/line-log-cleanup added uses names that
obscure its model. The cursors lno_post/lno_pre and the index lno_0
share an lno_ prefix but conflate the pre/post-image axis with the
0-based/1-based axis, the hunk state is a flat set of rhunk_* fields,
and the filter-state pointer is just s.
The filter bridges two layers of diff.c, and its fields already used
each layer's vocabulary, but in cryptic abbreviations. Spell them out
to the form the rest of the file uses, so that the patches that follow
can simplify and fix it with those clearer names in place:
- lno_post/lno_pre -> lno_in_postimage/lno_in_preimage, the
line-number cursors, matching the counters in struct emit_callback
- lno_0 -> idx_in_postimage, the 0-based range index
- the hunk-header geometry stays old/new (old_begin, new_begin, and
counts) to match the xdiff_emit_hunk_fn callback and the
"@@ -<old> +<new> @@" header it feeds, but moves from flat rhunk_*
fields into a "hunk" sub-struct, so accesses read
filter->hunk.old_begin
- flush_rhunk -> flush_range_hunk
- the filter-state pointer in each callback: s -> filter
Also rename the struct line_range_callback to line_range_filter: it is
a filter over xdiff output, not merely a callback.
No behavior change.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
diff.c | 192 +++++++++++++++++++++++++++++----------------------------
1 file changed, 97 insertions(+), 95 deletions(-)
diff --git a/diff.c b/diff.c
index 5a584fa1d5..1e043c959f 100644
--- a/diff.c
+++ b/diff.c
@@ -623,15 +623,15 @@ struct emit_callback {
* reveals whether they precede an in-range line (flush into range hunk) or
* an out-of-range line (discard).
*/
-struct line_range_callback {
+struct line_range_filter {
xdiff_emit_line_fn orig_line_fn;
void *orig_cb_data;
const struct range_set *ranges; /* 0-based [start, end) */
unsigned int cur_range; /* index into the range_set */
/* Post/pre-image line counters (1-based, set from hunk headers) */
- long lno_post;
- long lno_pre;
+ long lno_in_postimage;
+ long lno_in_preimage;
/*
* Function name from most recent xdiff hunk header;
@@ -640,12 +640,14 @@ struct line_range_callback {
char func[80];
long funclen;
- /* Range hunk being accumulated for the current range */
- struct strbuf rhunk;
- long rhunk_old_begin, rhunk_old_count;
- long rhunk_new_begin, rhunk_new_count;
- int rhunk_active;
- int rhunk_has_changes; /* any '+' or '-' lines? */
+ /* The range hunk being accumulated for the current range. */
+ struct {
+ struct strbuf lines; /* buffered in-range diff lines */
+ long old_begin, old_count;
+ long new_begin, new_count;
+ int active;
+ int has_changes; /* any '+' or '-' line? */
+ } hunk;
/* Removal lines not yet known to be in-range */
struct strbuf pending_rm;
@@ -2540,26 +2542,26 @@ static int quick_consume(void *priv, char *line UNUSED, unsigned long len UNUSED
return 1;
}
-static void discard_pending_rm(struct line_range_callback *s)
+static void discard_pending_rm(struct line_range_filter *filter)
{
- strbuf_reset(&s->pending_rm);
- s->pending_rm_count = 0;
+ strbuf_reset(&filter->pending_rm);
+ filter->pending_rm_count = 0;
}
-static void flush_rhunk(struct line_range_callback *s)
+static void flush_range_hunk(struct line_range_filter *filter)
{
struct strbuf hdr = STRBUF_INIT;
const char *p, *end;
- if (!s->rhunk_active || s->ret)
+ if (!filter->hunk.active || filter->ret)
return;
/* Drain any pending removal lines into the range hunk */
- if (s->pending_rm_count) {
- strbuf_addbuf(&s->rhunk, &s->pending_rm);
- s->rhunk_old_count += s->pending_rm_count;
- s->rhunk_has_changes = 1;
- discard_pending_rm(s);
+ if (filter->pending_rm_count) {
+ strbuf_addbuf(&filter->hunk.lines, &filter->pending_rm);
+ filter->hunk.old_count += filter->pending_rm_count;
+ filter->hunk.has_changes = 1;
+ discard_pending_rm(filter);
}
/*
@@ -2568,22 +2570,22 @@ static void flush_rhunk(struct line_range_callback *s)
* ctxlen causes xdiff to emit context covering a range that
* has no changes in this commit.
*/
- if (!s->rhunk_has_changes) {
- s->rhunk_active = 0;
- strbuf_reset(&s->rhunk);
+ if (!filter->hunk.has_changes) {
+ filter->hunk.active = 0;
+ strbuf_reset(&filter->hunk.lines);
return;
}
strbuf_addf(&hdr, "@@ -%ld,%ld +%ld,%ld @@",
- s->rhunk_old_begin, s->rhunk_old_count,
- s->rhunk_new_begin, s->rhunk_new_count);
- if (s->funclen > 0) {
+ filter->hunk.old_begin, filter->hunk.old_count,
+ filter->hunk.new_begin, filter->hunk.new_count);
+ if (filter->funclen > 0) {
strbuf_addch(&hdr, ' ');
- strbuf_add(&hdr, s->func, s->funclen);
+ strbuf_add(&hdr, filter->func, filter->funclen);
}
strbuf_addch(&hdr, '\n');
- s->ret = s->orig_line_fn(s->orig_cb_data, hdr.buf, hdr.len);
+ filter->ret = filter->orig_line_fn(filter->orig_cb_data, hdr.buf, hdr.len);
strbuf_release(&hdr);
/*
@@ -2591,18 +2593,18 @@ static void flush_rhunk(struct line_range_callback *s)
* The cast discards const because xdiff_emit_line_fn takes
* char *, though fn_out_consume does not modify the buffer.
*/
- p = s->rhunk.buf;
- end = p + s->rhunk.len;
- while (!s->ret && p < end) {
+ p = filter->hunk.lines.buf;
+ end = p + filter->hunk.lines.len;
+ while (!filter->ret && p < end) {
const char *eol = memchr(p, '\n', end - p);
unsigned long line_len = eol ? (unsigned long)(eol - p + 1)
: (unsigned long)(end - p);
- s->ret = s->orig_line_fn(s->orig_cb_data, (char *)p, line_len);
+ filter->ret = filter->orig_line_fn(filter->orig_cb_data, (char *)p, line_len);
p += line_len;
}
- s->rhunk_active = 0;
- strbuf_reset(&s->rhunk);
+ filter->hunk.active = 0;
+ strbuf_reset(&filter->hunk.lines);
}
static void line_range_hunk_fn(void *data,
@@ -2610,7 +2612,7 @@ static void line_range_hunk_fn(void *data,
long new_begin, long new_nr UNUSED,
const char *func, long funclen)
{
- struct line_range_callback *s = data;
+ struct line_range_filter *filter = data;
/*
* When count > 0, begin is 1-based. When count == 0, begin is
@@ -2622,104 +2624,104 @@ static void line_range_hunk_fn(void *data,
* flush or discard them when the next content line reveals
* whether the removals precede in-range content.
*/
- s->lno_post = new_begin;
- s->lno_pre = old_begin;
+ filter->lno_in_postimage = new_begin;
+ filter->lno_in_preimage = old_begin;
if (funclen > 0) {
- if (funclen > (long)sizeof(s->func))
- funclen = sizeof(s->func);
- memcpy(s->func, func, funclen);
+ if (funclen > (long)sizeof(filter->func))
+ funclen = sizeof(filter->func);
+ memcpy(filter->func, func, funclen);
}
- s->funclen = funclen;
+ filter->funclen = funclen;
}
static int line_range_line_fn(void *priv, char *line, unsigned long len)
{
- struct line_range_callback *s = priv;
+ struct line_range_filter *filter = priv;
const struct range *cur;
- long lno_0, cur_pre;
+ long idx_in_postimage, cur_pre;
- if (s->ret)
- return s->ret;
+ if (filter->ret)
+ return filter->ret;
if (line[0] == '-') {
- if (!s->pending_rm_count)
- s->pending_rm_pre_begin = s->lno_pre;
- s->lno_pre++;
- strbuf_add(&s->pending_rm, line, len);
- s->pending_rm_count++;
- return s->ret;
+ if (!filter->pending_rm_count)
+ filter->pending_rm_pre_begin = filter->lno_in_preimage;
+ filter->lno_in_preimage++;
+ strbuf_add(&filter->pending_rm, line, len);
+ filter->pending_rm_count++;
+ return filter->ret;
}
if (line[0] == '\\') {
- if (s->pending_rm_count)
- strbuf_add(&s->pending_rm, line, len);
- else if (s->rhunk_active)
- strbuf_add(&s->rhunk, line, len);
+ if (filter->pending_rm_count)
+ strbuf_add(&filter->pending_rm, line, len);
+ else if (filter->hunk.active)
+ strbuf_add(&filter->hunk.lines, line, len);
/* otherwise outside tracked range; drop silently */
- return s->ret;
+ return filter->ret;
}
if (line[0] != '+' && line[0] != ' ')
BUG("unexpected diff line type '%c'", line[0]);
- lno_0 = s->lno_post - 1;
- cur_pre = s->lno_pre; /* save before advancing for context lines */
- s->lno_post++;
+ idx_in_postimage = filter->lno_in_postimage - 1;
+ cur_pre = filter->lno_in_preimage; /* save before advancing for context lines */
+ filter->lno_in_postimage++;
if (line[0] == ' ')
- s->lno_pre++;
+ filter->lno_in_preimage++;
/* Advance past ranges we've passed */
- while (s->cur_range < s->ranges->nr &&
- lno_0 >= s->ranges->ranges[s->cur_range].end) {
- if (s->rhunk_active)
- flush_rhunk(s);
- discard_pending_rm(s);
- s->cur_range++;
+ while (filter->cur_range < filter->ranges->nr &&
+ idx_in_postimage >= filter->ranges->ranges[filter->cur_range].end) {
+ if (filter->hunk.active)
+ flush_range_hunk(filter);
+ discard_pending_rm(filter);
+ filter->cur_range++;
}
/* Past all ranges */
- if (s->cur_range >= s->ranges->nr) {
- discard_pending_rm(s);
- return s->ret;
+ if (filter->cur_range >= filter->ranges->nr) {
+ discard_pending_rm(filter);
+ return filter->ret;
}
- cur = &s->ranges->ranges[s->cur_range];
+ cur = &filter->ranges->ranges[filter->cur_range];
/* Before current range */
- if (lno_0 < cur->start) {
- discard_pending_rm(s);
- return s->ret;
+ if (idx_in_postimage < cur->start) {
+ discard_pending_rm(filter);
+ return filter->ret;
}
/* In range so start a new range hunk if needed */
- if (!s->rhunk_active) {
- s->rhunk_active = 1;
- s->rhunk_has_changes = 0;
- s->rhunk_new_begin = lno_0 + 1;
- s->rhunk_old_begin = s->pending_rm_count
- ? s->pending_rm_pre_begin : cur_pre;
- s->rhunk_old_count = 0;
- s->rhunk_new_count = 0;
- strbuf_reset(&s->rhunk);
+ if (!filter->hunk.active) {
+ filter->hunk.active = 1;
+ filter->hunk.has_changes = 0;
+ filter->hunk.new_begin = idx_in_postimage + 1;
+ filter->hunk.old_begin = filter->pending_rm_count
+ ? filter->pending_rm_pre_begin : cur_pre;
+ filter->hunk.old_count = 0;
+ filter->hunk.new_count = 0;
+ strbuf_reset(&filter->hunk.lines);
}
/* Flush pending removals into range hunk */
- if (s->pending_rm_count) {
- strbuf_addbuf(&s->rhunk, &s->pending_rm);
- s->rhunk_old_count += s->pending_rm_count;
- s->rhunk_has_changes = 1;
- discard_pending_rm(s);
+ if (filter->pending_rm_count) {
+ strbuf_addbuf(&filter->hunk.lines, &filter->pending_rm);
+ filter->hunk.old_count += filter->pending_rm_count;
+ filter->hunk.has_changes = 1;
+ discard_pending_rm(filter);
}
- strbuf_add(&s->rhunk, line, len);
- s->rhunk_new_count++;
+ strbuf_add(&filter->hunk.lines, line, len);
+ filter->hunk.new_count++;
if (line[0] == '+')
- s->rhunk_has_changes = 1;
+ filter->hunk.has_changes = 1;
else
- s->rhunk_old_count++;
+ filter->hunk.old_count++;
- return s->ret;
+ return filter->ret;
}
static void pprint_rename(struct strbuf *name, const char *a, const char *b)
@@ -4086,7 +4088,7 @@ static void builtin_diff(const char *name_a,
xdi_diff_outf(&mf1, &mf2, NULL, quick_consume,
&ecbdata, &xpp, &xecfg);
} else if (line_ranges) {
- struct line_range_callback lr_state;
+ struct line_range_filter lr_state;
unsigned int i;
long max_span = 0;
@@ -4094,7 +4096,7 @@ static void builtin_diff(const char *name_a,
lr_state.orig_line_fn = fn_out_consume;
lr_state.orig_cb_data = &ecbdata;
lr_state.ranges = line_ranges;
- strbuf_init(&lr_state.rhunk, 0);
+ strbuf_init(&lr_state.hunk.lines, 0);
strbuf_init(&lr_state.pending_rm, 0);
/*
@@ -4125,11 +4127,11 @@ static void builtin_diff(const char *name_a,
die("unable to generate diff for %s",
one->path);
- flush_rhunk(&lr_state);
+ flush_range_hunk(&lr_state);
if (lr_state.ret)
die("unable to generate diff for %s",
one->path);
- strbuf_release(&lr_state.rhunk);
+ strbuf_release(&lr_state.hunk.lines);
strbuf_release(&lr_state.pending_rm);
} else if (xdi_diff_outf(&mf1, &mf2, NULL, fn_out_consume,
&ecbdata, &xpp, &xecfg))
--
gitgitgadget
^ permalink raw reply related
* [PATCH 0/7] line-log: range-scope stat, check, and -G under -L
From: Michael Montalbo via GitGitGadget @ 2026-06-18 18:16 UTC (permalink / raw)
To: git; +Cc: D. Ben Knoble, Michael Montalbo
This series extends git log -L so that more of its diff output and commit
selection honor the tracked line ranges: the diff stat formats and --check
are supported with range-scoped results, and the -G pickaxe is scoped to the
tracked range.
It builds on top of the mm/line-log-cleanup topic [1], which integrated -L
with the standard log output pipeline and taught it the non-patch formats
--raw, --name-only, --name-status, and --summary.
With these patches the following also honor the tracked range:
* --stat, --numstat, --shortstat: counts cover only the lines inside the
tracked range, not the whole file.
* --check: whitespace errors are reported only for added lines inside the
tracked range, with the correct file line numbers.
* -G: a commit is selected only when the pattern appears on an
added/removed line inside the tracked range, rather than anywhere in the
file.
The --dirstat format is deliberately rejected. Its default mode reports each
directory's share of the total churn as a percentage, computed from
whole-file byte damage (via diffcore_count_changes(), outside the line-based
pipeline that -L scopes), so bare --dirstat cannot honor the tracked range.
The --dirstat=lines mode could: it aggregates the same per-file line counts
as --numstat, which -L already scopes. But supporting only that sub-mode
while bare --dirstat still errors is a confusing split, so the whole format
is left to a follow-up; --numstat already gives the exact range-scoped
per-file counts.
-S is left matching the whole file. Unlike -G, it counts needle occurrences
per blob rather than grepping the diff, so scoping it to a range needs a
different approach; that is left to a follow-up. Patch 7, which scopes -G,
also updates the -L documentation to note the -S/-G distinction, so the
whole-file behavior of -S is not mistaken for the range-scoped behavior
around it.
Patches 1-3 are independent of the new formats: they fix two bugs in the
existing -L patch output (a leaked deletion and an off-by-one hunk header),
bring its hunk headers in line with git diff's format, and clarify the
line-range filter mm/line-log-cleanup added, whose names obscured its model
(cryptic lno_ cursors conflating the pre/post-image and 0/1-based axes, a
flat hunk-state struct, and a one-letter state pointer (s)). The two bugs
may be a hint that the model could use clarification, so patch 1 renames and
groups the filter state and patch 2 documents the model, before the fixes
that read against it. Patches 4-7 then build the new formats on top:
* Patch 1: rename and group the filter for clarity. Spell the cryptic names
out to the file's own forms: the line-number cursors to
lno_in_preimage/lno_in_postimage (as in struct emit_callback) and the
range index to idx_in_postimage, while the hunk geometry stays old/new
(the xdiff_emit_hunk_fn convention) and moves into a sub-struct. Name the
filter pointer (filter) and rename the struct to line_range_filter and
the flush helper to flush_range_hunk. No behavior change.
* Patch 2: simplify the filter by classifying removals as they arrive,
dropping the pending_rm buffer and a latent flush_range_hunk() bug that
leaked deletions just past the range. Make the buffered lines the hunk's
single source of truth: flush_range_hunk() derives the counts from them
rather than tracking them per line, dropping three more fields. Document
the model with a block comment and worked example, and add
begin_range_hunk() as the counterpart to flush_range_hunk(). (This
simplification was submitted by itself previously [2] but did not
advance, so it is re-included here.)
* Patch 3: stop hand-rolling the synthetic hunk header and emit it through
xdiff's own formatter via a new xdiff_emit_hunk_header() helper. The
hand-rolled code put a count-0 side's begin one too high (the convention
is the line before the change); routing through xdl_emit_hunk_hdr() fixes
that by construction and, as a side effect, makes -L headers match git
diff exactly, including its omission of a count of 1. Regenerate the two
affected fixtures.
* Patch 4: extract a line_range_filter_diff() helper that folds the
filter's two preconditions into one place: inflate ctxlen to the largest
range span so every change within a range lands in a single xdiff hunk,
and clear XDL_EMIT_NO_HUNK_HDR so the hunk headers the filter reads are
always emitted (its position tracking relies on both). It then runs an
initialized filter through xdiff, flushes the final range hunk, and
releases it; use it in builtin_diff(). The stat, check, and -G patches
that reuse it inherit both.
* Patch 5: reuse the filter in builtin_diffstat() for the stat formats,
extend the -L output-format allowlist, and reject --dirstat.
* Patch 6: reuse the filter in builtin_checkdiff() and extend the allowlist
for --check. The separate blank-at-eof pass scans the whole file, so
scope its report to the tracked ranges too.
* Patch 7: scope -G to the tracked range. Expose the filter as
diff_emit_line_ranges() and grep only the tracked range's lines,
threading the filepair's line_ranges through the pickaxe callback. -S is
left whole-file, and the -L documentation is updated to note that -G is
range-scoped while -S still matches the whole file.
[1]
https://lore.kernel.org/git/pull.2094.v3.git.1780001267.gitgitgadget@gmail.com/
[2]
https://lore.kernel.org/git/pull.2099.git.1777230630020.gitgitgadget@gmail.com/
Michael Montalbo (7):
diff: rename and group the line-range filter for clarity
diff: simplify the line-range filter by classifying removals
immediately
diff: emit -L hunk headers via xdiff's formatter
diff: extract a line-range diff helper for reuse
line-log: support diff stat formats with -L
diff: support --check with -L line ranges
diffcore-pickaxe: scope -G to the -L tracked range
Documentation/line-range-options.adoc | 17 +-
diff.c | 491 ++++++++++++++---------
diffcore-pickaxe.c | 37 +-
revision.c | 6 +-
t/t4211-line-log.sh | 439 +++++++++++++++++++-
t/t4211/sha1/expect.no-assertion-error | 2 +-
t/t4211/sha1/expect.vanishes-early | 6 +-
t/t4211/sha256/expect.no-assertion-error | 2 +-
t/t4211/sha256/expect.vanishes-early | 6 +-
xdiff-interface.c | 19 +
xdiff-interface.h | 28 ++
11 files changed, 826 insertions(+), 227 deletions(-)
base-commit: ea97ad8d017de0c9037451a78008a0fd60abea0c
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2152%2Fmmontalbo%2Fmm%2Fline-log-stat-formats-followup-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2152/mmontalbo/mm/line-log-stat-formats-followup-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2152
--
gitgitgadget
^ permalink raw reply
* Re: [PATCH v3 15/17] odb/source-packed: stub out remaining functions
From: Junio C Hamano @ 2026-06-18 17:59 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Karthik Nayak, Justin Tobler
In-Reply-To: <20260617-pks-odb-source-packed-v3-15-b5c7583cd795@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
Just FYI (i.e. nothing wrong in this patch)
> +static int odb_source_packed_write_object(struct odb_source *source UNUSED,
> + const void *buf UNUSED,
> + unsigned long len UNUSED,
The type of this parameter will become size_t via another topic in
flight; I prepared an evil merge to address it (otherwise winbuild
would barf, as expected).
-- >8 --
Author: Junio C Hamano <gitster@pobox.com>
Date: Thu Jun 18 10:49:10 2026 -0700
merge-fix po/hash-object-size-t vs ps/odb-source-packed
diff --git a/odb/source-packed.c b/odb/source-packed.c
index 42c28fba0e..decc81aa52 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -503,7 +503,7 @@ static int odb_source_packed_freshen_object(struct odb_source *source,
static int odb_source_packed_write_object(struct odb_source *source UNUSED,
const void *buf UNUSED,
- unsigned long len UNUSED,
+ size_t len UNUSED,
enum object_type type UNUSED,
struct object_id *oid UNUSED,
struct object_id *compat_oid UNUSED,
^ permalink raw reply related
* Re: [PATCH v15 0/7] branch: delete-merged
From: Harald Nordgren @ 2026-06-18 17:53 UTC (permalink / raw)
To: phillip.wood
Cc: Harald Nordgren via GitGitGadget, git, Kristoffer Haugsbakk,
Johannes Sixt
In-Reply-To: <feac3d8b-e291-48e8-ac73-3b1f5321799b@gmail.com>
> > I received the same feedback from Junio before, so I'm not unaware of
> > this problem. I am trying to slow down. I often prepare the work as
> > soon as I get some comments -- I'm on paternity leave so I have a lot
> > of time when the baby is sleeping --
>
> Congratulations - I hope the baby is sleeping at night as well in the day!
Thanks! It's our third, so hopefully we got the hang of it now.
He sleeps -- some of the time.
> > then I actively hold off on
> > sending to not overload the rest of you. But at the same time I think
> > it's valuable to keep up a certain pace. It's a balancing act.
> It is worth waiting for the discussion to settle on each round, I'll try
> and be clear when I've finished looking at each revision. I'm sure other
> folks would appreciate you looking at their patches and commenting on
> them while you're waiting for feedback on yours, especially the GSoC
> project students.
That's a good point!
Harald
^ permalink raw reply
* Re: [PATCH] help: prompt user to run corrected command on typo
From: Justin Tobler @ 2026-06-18 17:48 UTC (permalink / raw)
To: calicomills; +Cc: git, gitster
In-Reply-To: <6a340006.60da1a74.20db39.8f57@mx.google.com>
On 26/06/18 07:26AM, calicomills wrote:
> From 0dc9e5c4593611b75e7003e8fdbea9370524c05b Mon Sep 17 00:00:00 2001
> From: calicomills <jishnuck26@gmail.com>
> Date: Thu, 18 Jun 2026 19:47:12 +0530
> Subject: [PATCH] help: prompt user to run corrected command on typo
>
> When a user mistypes a git command and there is exactly one similar
> command, git currently prints a suggestion but exits, requiring the
> user to retype the corrected command manually.
>
> Instead, when stdin and stderr are both connected to a terminal and
> there is a single best match, prompt the user with:
>
> Did you mean 'git checkout neo'? [y/N]
>
> The full corrected invocation (command + original arguments) is shown
> in the prompt so the user knows exactly what will run. Answering 'y'
> re-executes git with the corrected command and all original arguments.
> Answering anything else exits as before.
Isn't this already possible via setting `help.autoCorrect=prompt` in the
config? For example:
git -c help.autoCorrect=prompt comit --allow-empty -m init
seems to already do exactly what is proposed here.
-Justin
^ permalink raw reply
* Re: [PATCH] checkout: add --fetch to fetch remote before resolving start-point
From: D. Ben Knoble @ 2026-06-18 17:47 UTC (permalink / raw)
To: Harald Nordgren; +Cc: Harald Nordgren via GitGitGadget, git
In-Reply-To: <CAHwyqnU4xuw9ZDjarWKKua_s1Qywt07GyP1kJO2HM+XQTcE8hA@mail.gmail.com>
Hi Harald,
On Thu, Jun 18, 2026 at 8:36 AM Harald Nordgren
<haraldnordgren@gmail.com> wrote:
>
> Hi Ben!
>
> Trying to shore up some support for this topic. How do you feel about this now?
>
>
> Harald
I haven't followed the series too closely; it sounds like the code is
in decent shape.
I stand by my thought that this could be convenient, but that's not
much one way or another in terms of how it fits in overall with Git's
vision.
There are some conveniences I recommend universally. This one requires
caveats, as pointed out by others, depending on the project, so… from
that point of view I wonder if keeping "fetch + switch" as 2 distinct
steps is better? I truly don't know.
--
D. Ben Knoble
^ permalink raw reply
* Re: [PATCH] completion: zsh: support completion after "git -C <path>"
From: D. Ben Knoble @ 2026-06-18 17:43 UTC (permalink / raw)
To: Lutz Lengemann via GitGitGadget; +Cc: git, Lutz Lengemann, Junio C Hamano
In-Reply-To: <pull.2155.git.1781710256081.gitgitgadget@gmail.com>
[apologies in advance for the strange format below]
On Wed, Jun 17, 2026 at 11:37 AM Lutz Lengemann via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Lutz Lengemann <lutz@lengemann.net>
>
> The zsh completion wrapper (__git_zsh_main) did not handle the global -C
> option, so "git -C <path> <command> <TAB>" offered nothing and could not
> complete a command's arguments.
>
> Three things are needed to make it work, all scoped to -C:
>
> - Add -C to the _arguments specification, so completion no longer stops
> at it.
>
> - Advance __git_cmd_idx past any leading "-C <path>" options. The index
> is hard-coded to 1, i.e. the command is assumed to be the first
> argument; with -C present the command sits two words later for each
> -C, so the bash helpers otherwise look at the wrong word and produce
> nothing.
>
> - Collect the -C paths into __git_C_args, as __git_main does. The bash
> helpers run git to resolve aliases and list refs; without the -C
> paths they run in the current directory, so completion fails whenever
> the cwd is not the target repository or the command is an alias.
>
> With these, "git -C <path> <command> <TAB>" completes the command, its
> options and its arguments, including outside the repository, through
> aliases, and with repeated -C options.
>
> Signed-off-by: Lutz Lengemann <lutz@lengemann.net>
> ---
> completion: zsh: support completion after "git -C "
>
> This patch is intentionally scoped to -C, but the underlying problem is
> more general. The zsh wrapper hard-codes __git_cmd_idx=1, i.e. it
> assumes the command is always the first argument. That assumption breaks
> argument completion after any global option that precedes the command,
> not just -C — e.g. --git-dir, --work-tree, --namespace, -c, and
> -p/--paginate. After those, git <opt> <command> <TAB> currently
> completes the command name but not its arguments.
>
> The same approach generalizes cleanly: instead of skipping only leading
> -C options, walk all leading global options and their arguments to
> locate the command and its true index (mirroring the option scan in
> __git_main in git-completion.bash), while collecting -C into
> __git_C_args and --git-dir into __git_dir as today.
>
> I kept this revision narrow for reviewability and because git -C is the
> case where I miss the completion, but I'm happy to extend it to cover
> the other global options in a follow-up (or fold it into this patch) if
> that's preferred.
See Junio's review for whether we should expand in this patch or a follow-up.
In reply to Junio:
> [the new handling only knows about -C]
> Doesn't it want to do something similar to what __git_main in
> git-completion.bash does at the beginning, namely, this part?
Yeah, we probably do want to skip over -c, etc. (I see some support for
--bare and --git-dir, but not skipping over it.) Still, this patch makes
things no worse in that regard, and improves the situation for -C
AFAICT.
In reply to Lutz:
> + local -a __git_C_args
> + local -i i=2
> +
> + while [[ ${orig_words[i]} == -C ]]; do
> + __git_C_args+=(-C ${orig_words[i+1]})
> + (( __git_cmd_idx += 2 ))
> + (( i += 2 ))
> + done
I don't see either of these 2 local variables used anywhere else…
…well, except the Bash completion helpers, I suppose. But we mark these
local, so how do they propagate to the other functions?
Still, I was able to try this out with the somewhat hacky
zsh # new shell :)
# absolute path important
autoload -Uz $PWD/contrib/completion/git-completion.zsh
compdef git-completion.zsh git
git -C <tab>
and it does prioritize directories there (though I still get a listing
of files afterwards, so the screen is taken up by that gigantic listing
in git.git, for example).
By the way, I've realized that "git -<tab>" has the same problem (a
giant list of files after the other option completions), and worse has
some _funky_ output!
git -<tab> # without patch
(option)
--bare
--exec-path
--git-dir
--help
--html-path
--info-path
--man-path
--namespace
--no-pager
--no-replace-objects
--paginate
--version
--work-tree
-p
# treat the repository as a bare repository
# path to where your core git programs are installed
# set the path to the repository
# prints the synopsis and a list of the most commonly used commands
# print the path where gits HTML documentation is installed
# print the path where the Info files are installed
# print the manpath (see `man(1)`) for the man pages
# set the git namespace
# do not pipe git output into a pager
# do not use replacement refs to replace git objects
# pipe all output into less
# prints the git suite version
# set the path to the working tree
[ed: the above block repeats twice more before the (file) listing below]
(file)
[…]
Here's the output of _complete_help (^Xh by default) in both situations,
in case that helps to understand either the extra files listing (1) in
the example further back or the issue with single letter options (2)
just mentioned:
1: tags in context :completion::complete:git::
option-C-1 (_arguments __git_zsh_main _git git-completion.zsh)
use-compctl (_default _git git-completion.zsh)
globbed-files (_files _default _git git-completion.zsh)
tags in context :completion::complete:git:option-C-1:
directories (_directories _arguments __git_zsh_main _git
git-completion.zsh)
globbed-files (_files _directories _arguments __git_zsh_main _git
git-completion.zsh)
all-files (_files _directories _arguments __git_zsh_main _git
git-completion.zsh)
2: tags in context :completion::complete:git::
argument-1 options (_arguments __git_zsh_main _git)
use-compctl (_default _git)
globbed-files (_files _default _git)
tags in context :completion::complete:git:argument-1:
common-commands alias-commands all-commands (__git_zsh_main _git)
common-commands (__git_zsh_cmd_common
__git_zsh_main _git)
alias-commands (__git_zsh_cmd_alias
__git_zsh_main _git)
all-commands (__git_zsh_cmd_all
__git_zsh_main _git)
tags in context :completion::complete:git:options:
options (_arguments __git_zsh_main _git)
> + '*-C[run as if git was started in <path>]: :_directories' \
We should probably note in the log message that the _directories
completion will not account for previous -C; that is, after typing
git -C dir -C <tab>
we will complete directories in ".", not "dir". That's probably a
reasonable limitation for now, but I think we could do _slightly_ better
by using a state "->dir" or something, accumulating the current prefix,
and passing that to _directories as a prefix with -W (see _path_files in
zshcompsys, which _directories delegates to via _files, IIUC).
--
D. Ben Knoble
^ permalink raw reply
* [PATCH] doc: detail LMAP binary format specification
From: irsalshydiq @ 2026-04-16 5:55 UTC (permalink / raw)
To: git; +Cc: irsalshydiq
The experimental Rust implementation in 'loose.rs' introduces a new
binary format called 'LMAP' for mapping between different object ID
formats (e.g., SHA-1 and SHA-256).
However, the technical documentation for this format was missing from
the 'Documentation/technical/' directory, making it difficult for
C developers to understand the format's layout and logic.
Add 'Documentation/technical/loose-object-map.adoc' to provide a bit-by-bit
specification of the 'LMAP' format and register it in the build systems.
Signed-off-by: irsalshydiq <ichalprov@gmail.com>
---
Documentation/Makefile | 1 +
Documentation/technical/loose-object-map.adoc | 72 +++++++++++++++++++
Documentation/technical/meson.build | 1 +
3 files changed, 74 insertions(+)
create mode 100644 Documentation/technical/loose-object-map.adoc
diff --git a/Documentation/Makefile b/Documentation/Makefile
index 2699f0b24a..8ad908d62c 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -126,6 +126,7 @@ TECH_DOCS += technical/directory-rename-detection
TECH_DOCS += technical/hash-function-transition
TECH_DOCS += technical/large-object-promisors
TECH_DOCS += technical/long-running-process-protocol
+TECH_DOCS += technical/loose-object-map
TECH_DOCS += technical/multi-pack-index
TECH_DOCS += technical/packfile-uri
TECH_DOCS += technical/pack-heuristics
diff --git a/Documentation/technical/loose-object-map.adoc b/Documentation/technical/loose-object-map.adoc
new file mode 100644
index 0000000000..6e8dbd6c6f
--- /dev/null
+++ b/Documentation/technical/loose-object-map.adoc
@@ -0,0 +1,72 @@
+Loose Object Map (LMAP) format
+============================
+
+The loose object map file (LMAP) provides a way to map between different object
+ID formats (e.g., SHA-1 and SHA-256) for loose objects. It is designed for
+efficient lookup and storage of these mappings.
+
+All multi-byte integers are in network byte order (big-endian).
+
+== File Layout
+
+- A header (20 bytes)
+- An Object Format Table (16 bytes per format)
+- A Trailer Offset (8 bytes)
+- Data Sections (variable length, 4-byte aligned)
+- A Trailer (variable length, defined by hash algorithm)
+
+=== Header
+
+- 4-byte signature: `LMAP`
+- 4-byte version number: The current version is 1.
+- 4-byte header size: The total size of the header, including the Object Format Table and Trailer Offset.
+- 4-byte number of items: The number of object IDs mapped in this file.
+- 4-byte number of object formats: The number of different hash algorithms supported in this file (minimum 2).
+
+=== Object Format Table
+
+For each object format (as specified in the header), there is a 16-byte entry:
+
+- 4-byte Format ID: The identifier for the hash algorithm (e.g., `0x73686131` for SHA-1, `0x73323536` for SHA-256).
+- 4-byte Shortened Length: The minimum number of bytes needed to unambiguously identify an object ID in this format within this file.
+- 8-byte Data Offset: The absolute offset from the beginning of the file to the start of the data section for this format.
+
+=== Trailer Offset
+
+- 8-byte Trailer Offset: The absolute offset from the beginning of the file to the start of the Trailer.
+
+=== Data Sections
+
+Each object format has a corresponding data section starting at the offset provided in the Object Format Table. Each data section is aligned to a 4-byte boundary.
+
+==== Format 1 (Storage Format) Data Section
+
+The first format listed is considered the "storage" or "main" format. Its data section contains:
+
+1. **Shortened Index**: `(number of items) * (shortened length)` bytes.
+ This table contains the first `shortened length` bytes of each object ID, sorted lexicographically. This allows for binary search lookup.
+
+2. **Full OID Table**: `(number of items) * (hash length)` bytes.
+ The full object IDs for the storage format, in the same order as the Shortened Index.
+
+3. **Metadata Table**: `(number of items) * 4` bytes.
+ A table of 32-bit integers representing the type of each object:
+ - 0: Reserved (e.g., null OID, empty tree/blob)
+ - 1: Loose Object
+ - 2: Shallow Commit
+ - 3: Submodule Commit
+
+==== Subsequent Format (Compatibility) Data Section
+
+For each subsequent format, the data section contains:
+
+1. **Shortened Index**: Similar to the storage format, but for the compatibility algorithm's OIDs.
+
+2. **Full OID Table**: The full object IDs in the compatibility algorithm.
+
+3. **Mapping Table**: `(number of items) * 4` bytes.
+ A table of 32-bit integers. Each entry at index `i` provides the index in the **storage format's** tables that corresponds to this compatibility object ID.
+
+=== Trailer
+
+- Variable length: The hash of all preceding bytes in the file, calculated using the main hash algorithm.
diff --git a/Documentation/technical/meson.build b/Documentation/technical/meson.build
index ec07088c57..dc1249f9aa 100644
--- a/Documentation/technical/meson.build
+++ b/Documentation/technical/meson.build
@@ -16,6 +16,7 @@ articles = [
'large-object-promisors.adoc',
'long-running-process-protocol.adoc',
'multi-pack-index.adoc',
+ 'loose-object-map.adoc',
'packfile-uri.adoc',
'pack-heuristics.adoc',
'parallel-checkout.adoc',
--
2.50.1 (Apple Git-155)
^ permalink raw reply related
* Re: [PATCH] zlib: properly clamp to uLong
From: Junio C Hamano @ 2026-06-18 17:03 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin
In-Reply-To: <pull.2153.git.1781790619424.gitgitgadget@gmail.com>
The original in js/objects-larger-than-4gb-on-windows says things
like:
+ s->z.total_in = (uLong)(s->total_in & ULONG_MAX_VALUE);
+ s->z.total_out = (uLong)(s->total_out & ULONG_MAX_VALUE);
Your patch ...
> +static inline uLong zlib_uLong_cap(size_t s)
> +{
> + return s < ULONG_MAX_VALUE ? (uLong)s : ULONG_MAX_VALUE;
> +}
> +
> static void zlib_pre_call(git_zstream *s)
> {
> s->z.next_in = s->next_in;
> s->z.next_out = s->next_out;
> - s->z.total_in = (uLong)(s->total_in & ULONG_MAX_VALUE);
> - s->z.total_out = (uLong)(s->total_out & ULONG_MAX_VALUE);
> + s->z.total_in = zlib_uLong_cap(s->total_in);
> + s->z.total_out = zlib_uLong_cap(s->total_out);
... is an obvious fix for that.
> @@ -60,7 +65,7 @@ static void zlib_post_call(git_zstream *s, int status)
> * We track our own totals and verify only the low bits match.
> */
> if ((s->z.total_out & ULONG_MAX_VALUE) !=
> - ((s->total_out + bytes_produced) & ULONG_MAX_VALUE))
> + ((zlib_uLong_cap(s->total_out) + bytes_produced) & ULONG_MAX_VALUE))
> BUG("total_out mismatch");
Because we now clamp (not "taking lower bits of") s->total_out to a
value between 0..4GB and store it in s->z.total_out in pre-call, let
zlib do its thing that increments s->z.total_out modulo 4GB, and we
clamp the s->total_out (before incrementing) the same way in
post_call here, both sides of "!=" above even out. But the comment
before this comparison that claims that "we ... verify only the low
bits match" is a bit off the reality, I suspect.
> @@ -68,7 +73,7 @@ static void zlib_post_call(git_zstream *s, int status)
> */
> if (status != Z_NEED_DICT &&
> (s->z.total_in & ULONG_MAX_VALUE) !=
> - ((s->total_in + bytes_consumed) & ULONG_MAX_VALUE))
> + ((zlib_uLong_cap(s->total_in) + bytes_consumed) & ULONG_MAX_VALUE))
> BUG("total_in mismatch");
>
> s->total_out += bytes_produced;
>
> base-commit: 7a094d68a27e321a99c8ab6b700909e503904bd9
^ permalink raw reply
* Re: [PATCH v2 7/8] refs: fix recursing `get_main_ref_store()` with "onbranch" config
From: Jeff King @ 2026-06-18 16:40 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Karthik Nayak
In-Reply-To: <20260615-b4-pks-refs-avoid-chdir-notify-reparent-v2-7-f4854aa99859@pks.im>
On Mon, Jun 15, 2026 at 03:56:53PM +0200, Patrick Steinhardt wrote:
> When we have an "onbranch" condition we need to ask the reference
> database whether HEAD currently points at the configured branch. This
> unfortunately creates a chicken-and-egg problem:
>
> - The reference database needs to read the configuration so that it
> can configure itself.
>
> - The configuration needs to construct a reference database to fully
> parse all of its conditionals.
>
> The way we handle this is by simply excluding "onbranch" conditionals
> when we haven't yet configured the reference database.
My gut feeling upon reading this is that some part of the config reading
is being done wrong to create this chicken-and-egg situation.
I'd expect the ref database config (like the ref format) to be read not
through the regular config subsystem, but via read_repository_format()
and friends. And while that does build on the regular config code, it
should never enable includes at all. So includeIf.onbranch:foo.path is
just another uninteresting config key to it.
In other words, there should be two passes over the config file: one to
load basic repository information (and not respect includes), and one to
actually load what we think of as user-visible config[1].
And it seems to work. If I do this:
diff --git a/config.c b/config.c
index 45144f73c5..343af2cf9a 100644
--- a/config.c
+++ b/config.c
@@ -303,7 +303,7 @@ static int include_by_branch(struct config_include_data *data,
const char *refname, *shortname;
if (!data->repo || data->repo->ref_storage_format == REF_STORAGE_FORMAT_UNKNOWN)
- return 0;
+ BUG("chicken and egg");
refname = refs_resolve_ref_unsafe(get_main_ref_store(data->repo),
"HEAD", 0, NULL, &flags);
and then:
git config includeIf.onbranch:main.path alt-config
git config -f .git/alt-config foo.bar baz
git config foo.bar
then we correctly read the value without triggering this code path.
Looking back at the last commit that touched include_by_branch(), the
problem does not appear to be about a chicken-and-egg at all, though. It
is about reading config with includes when there is _no_ repository at
all. I.e., this:
git config -f main-config includeIf.onbranch:main.path alt-config
git config -f alt-config foo.bar baz
GIT_DIR=/does/not/exist git.compile config --include -f main-config foo.bar
will trigger that BUG() marker, and quietly returning "no match" (like
the current code does) is the right thing.
Looking below...
> The consequence is that we have recursion:
>
> 1. We call `get_main_ref_store()`.
>
> 2. We don't yet have a reference store, so we call `ref_store_init()`.
>
> 3. We parse the configuration required for the reference store.
>
> 4. We eventually end up in `include_by_branch()`.
>
> 5. We have already configured the reference storage format, so we end
> up calling `get_main_ref_store()` again.
Ah, the culprit seems to be ref_store_init() calling into the regular
config parser via repo_settings_get_log_all_ref_updates(). But that
feels weird to me. Either:
1. It is application config that should not be something we need to
load in order to initialize the backend. We could lazy-load it
later, or rely on higher level code to set the option.
2. It is crucial to the ref backend functioning, in which case we
ought to be reading it alongside core.repositoryFormatVersion, etc.
-Peff
^ permalink raw reply related
* Re: [PATCH v2] SubmittingPatches: address design critiques
From: Junio C Hamano @ 2026-06-18 16:26 UTC (permalink / raw)
To: Kristoffer Haugsbakk; +Cc: git
In-Reply-To: <95cd81dc-baea-4318-9f01-6a795f8eb5bb@app.fastmail.com>
"Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:
> You can imagine someone from group number 1 who is *not* in group number
> 3 use a weekend to implement something. But then when it is submitted it
> turns out that is a very “centralized CVS” idea which doesn’t fit into
> git(1) at all. That’s easily spotted by group number 3 by just looking
> at the proposed docs or design. Now that group number 1 individual might
> just have a bunch of code that is dead weight for any proper Git
> workflow.
That depends on how obviously wrong the idea is. If your proposal
is to write another CVS into Git, that may be too obvious it may not
fly, but the thing is, "proposals" that get the canned response you
quoted are often vague enough that crucial details that divide
"iffy" and "obviously wrong" are missing.
One way to make these proposals sufficiently clear to allow
reviewers to tell the difference is with a code that builds. There
may be other ways, but that is one obvious way to start a meaningful
discussion.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox