* [RFC PATCH 0/3] diff: pair edited lines inside moved blocks
From: Keita Oda @ 2026-05-27 4:23 UTC (permalink / raw)
To: git; +Cc: Keita ODA
From: Keita ODA <ainsophyao@gmail.com>
This is an RFC for a review aid, not a proposed final UI or option name.
The motivation is the gap between --word-diff and --color-moved.
--word-diff is very useful when the line-level diff already found useful
old/new line pairs. --color-moved is useful when moved lines are exact
matches. But when a block is moved and one line inside the block is edited,
the small edit can be buried in a large delete/add region.
That case matters for review. A one-line move is usually easy to inspect by
eye. A ten-line moved block with a one-character change inside it is harder
to audit. A small synthetic permission-table example in patch 3 uses this
shape:
-#define PERM_RESOURCE_EXPORT 0x0008
+#define PERM_RESOURCE_EXPORT 0x0001
That particular toy example is not meant to show something that
--color-moved cannot see. It is meant to make the review question small:
can Git expose "this moved line was also edited" in a lightweight way?
The real-world cases below are less about proving that existing modes are
blind, and more about making the row-to-row correspondence explicit enough
that the small edits are easy to check.
This series adds an opt-in prototype, --word-diff-align, that post-processes
the emitted diff symbols and tries to pair similar deleted and inserted lines.
It does not change the underlying diff algorithm, patch semantics, apply, or
merge behavior.
The prototype is deliberately language-agnostic. It does not parse source
code or build an AST; it only tokenizes diff lines into small text tokens and
scores local token overlap. This keeps the experiment applicable to code,
tests, generated tables, documentation, and other text files.
The prototype is intentionally split into three pieces:
* patch 1 adds the candidate retrieval and line-pair scoring, and exposes
selected pairs with an RFC/debug comment;
* patch 2 adds a small RFC-only renderer that inserts word-diff-like
markers on the selected pairs, so that the recovered pairs are easier to
inspect;
* patch 3 adds a focused test case.
The current prototype is still larger than I would like, but the split keeps
the experimental pieces visible. The full series is about 1000 inserted
lines; roughly 800 lines are option plumbing, tokenization, candidate
retrieval, scoring, pair selection, and debug comments, while about 200 lines
are temporary rendering code for review.
The scoring model is:
S = W + aL
where W is a 5-line-window token overlap score and L is a center-line token
LCS score. A small 64-bit window fingerprint is used only as a candidate
retrieval index; candidate pairs are scored again before they are selected.
Tokens repeated in the surrounding small window carry less weight for the
center-line score, which is a local-IDF-like approximation. This keeps tokens
such as "import" or "#define" from overwhelming the line-specific identifier.
Some real-world examples that motivated the prototype:
* CPython opcode/metadata renumbering, where many table rows stay logically
paired but their numeric values shift;
* CPython test parameterization rewrites such as tuple rows becoming
dict(input=..., expected=...) rows;
* Git's own expected-output tables, where a column width change adds spaces
across many rows and a row insertion shifts the surrounding context;
* Git's own remote.c refactoring, where extracted helper code has small
identifier changes.
As a rough trigger-rate sanity check, I ran the prototype over 5734 changed
file pairs sampled from recent Git, CPython, and Rust history. The stricter
"crossing and edited" signal, which ignores the many adjacent row pairs and
looks for pairs that cross another recovered pair, appeared in 739 file pairs
(about 13%). This is not a gold-label quality number, but it suggests that
the mode is not only triggering on the synthetic test.
A small manual review found both clear wins and loose matches.
I found the problem easiest to inspect with four-way comparisons:
* git diff --histogram
* git diff --histogram --word-diff=plain
* git diff --histogram --color-moved=blocks
* git diff --histogram --word-diff-align
I put a small set of rendered four-way examples here:
https://oda.github.io/git-diff-rfc-examples/rfc-word-diff-align/
These links are supplemental; the patch series is intended to be readable
without them.
Known limitations:
* the UI/debug output is not final;
* generated or boilerplate-heavy hunks, especially Rust generated test
updates, can still produce loose matches;
* one-line long-distance pairs are often less useful than block-level pairs;
* the prototype intentionally gives local and remote pairs similar treatment
for now, to make the recovered pairings visible for discussion;
* thresholds and tie-breaking are still experimental.
The question for this RFC is whether this kind of language-agnostic line-pair
annotation is worth pursuing in core, and if so whether it should be shaped as
word-diff plumbing, a color-moved extension, or a separate opt-in mode.
Keita ODA (3):
diff: add word-diff-align line pairing
diff: render word-diff-align pairs for RFC review
t4034: cover moved-and-edited word diff alignment
diff.c | 996 +++++++++++++++++++++++++++++++++++++++++-
diff.h | 1 +
t/t4034-diff-words.sh | 46 ++
3 files changed, 1035 insertions(+), 8 deletions(-)
--
2.39.3 (Apple Git-146)
^ permalink raw reply
* Re: git mv after the fact
From: Tim Tassonis @ 2026-05-27 3:27 UTC (permalink / raw)
To: Junio C Hamano, Chris Torek; +Cc: Frieder Hannenheim, git
In-Reply-To: <xmqqy0h5lfa0.fsf@gitster.g>
Yeah, and while we're at at it: why not another patch for
git rm
file_i_deleted_but_didnt_tell_git_and_dont_want_an_error_message_because_thats_offensive
because that's always very rude, too, telling me to have to use
git rm -f
Because I also am very sensitive and don't like to be told I fucked up
and have to be more specific about what I actually want. That's just
toxic, man.
On 5/27/26 05:09, Junio C Hamano wrote:
> Chris Torek <chris.torek@gmail.com> writes:
>
>> On Tue, May 26, 2026 at 6:18 AM Frieder Hannenheim <mail@fhannenheim.net> wrote:
>>> I'd like to propose a new flag for git mv, that updates the index
>>> like git mv normally would but does not move the file. ...
>>
>> You may already know this, but technically no flag is needed:
>> you can just "git add" the new name and "git rm" the old one,
>> with the same effect.
>
> Correct.
>
>> A flag for "git mv" would be convenient (and slightly more
>> efficient, not in terms of storage but in terms of CPU time
>> spent discovering that the contents under the new name
>> already exist in the object database).
>
> May be convenient, but I do not get the "efficient" part. Do you
> mean that for two operations "add" and "rm", you need to spend two
> index writes plus one file contents hash, as opposed to one index
> rite without having to do any contents hash?
>
>> But Git will discover
>> the rename on its own in the usual way regardless of how
>> you get to that point.
>
> This is not incorrect per-se, but it is a confusing thing to say to
> somebody who does not know the equivalence of "mv" and "rm + add".
> It would not be clear to them that you are not talking about what
> happens during "mv" or "rm + add", but about what happend during
> "git log -M", "git diff -M", etc.
>
> There is "git rm --cached" that can be used to recover from an
> "oops, I removed the file from the filesystem without telling Git".
>
> $ date >new-file.txt
> $ git add new-file.txt
> $ rm new-file.txt
> $ git rm --cached new-file.txt
>
> I think the requested "feature" is not all that outrageous. It
> would be a similar value as a morning-after correction measure for
> "oops, I moved the file in the filesystem without telling Git".
>
> $ date >old-file.txt
> $ git add old-file.txt
> $ mv old-file.txt new-file.txt
> $ git mv --cached old-file.txt new-file.txt
>
> Thanks.
>
>
--
decentral.ch - IT Stuff
Tim Tassonis
Badenerstrasse 219
8003 Zürich
stuff@decentral.ch
+41 79 229 36 17
^ permalink raw reply
* Re: [PATCH v2] completion: hide dotfiles for selected path completion
From: Junio C Hamano @ 2026-05-27 3:22 UTC (permalink / raw)
To: Zakariyah Ali via GitGitGadget; +Cc: git, Zakariyah Ali
In-Reply-To: <pull.2311.v2.git.git.1779808987825.gitgitgadget@gmail.com>
"Zakariyah Ali via GitGitGadget" <gitgitgadget@gmail.com> writes:
> This matches standard shell filename completion behavior, where dotfiles
> are hidden by default unless the user starts their input with a dot.
OK, with this rationale added, I no longer have problem with the
proposed new behaviour.
As I'm not going to give a serious review on the patch body itself,
I would really appreciate somebody more knowledgeable on the
existing bach completion code than I am to take a look.
Thanks.
^ permalink raw reply
* Re: git mv after the fact
From: Chris Torek @ 2026-05-27 3:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Frieder Hannenheim, git
In-Reply-To: <xmqqy0h5lfa0.fsf@gitster.g>
> Chris Torek <chris.torek@gmail.com> writes:
>> A flag for "git mv" would be convenient (and slightly moreefficient ...
>
On Tue, May 26, 2026 at 8:09 PM Junio C Hamano <gitster@pobox.com> wrote:
> May be convenient, but I do not get the "efficient" part.
A normal `git mv` renames the index entry and the file in the working
tree without running `git add` on the *contents*, so there's no new hash
computation. Presumably a `git mv --after foo bar` would do the same: verify
that there is no existing `bar` in the index, that there is an existing `foo` in
the index, and that there is no `foo` but there is a `bar` in the working tree,
and then it would rename (add-and-remove, really, because of sorting)
the index entry, without scanning the working tree contents.
In other words, we skip reading the 3 terabyte file, or whatever.
Anyway, comparing to `git rm --cached`:
> I think the requested "feature" is not all that outrageous. It
> would be a similar value as a morning-after correction measure for
> "oops, I moved the file in the filesystem without telling Git".
I agree, but I also don't see it as valuable enough to bother
writing a proper implementation.
I did write it up as a shell script though, long ago. Adding it here
via gmail would mess with white space so I'll just provide a link
to the file on GitHub:
https://github.com/chris3torek/scripts/blob/master/git-mv-after
Chris
^ permalink raw reply
* Re: git mv after the fact
From: Junio C Hamano @ 2026-05-27 3:09 UTC (permalink / raw)
To: Chris Torek; +Cc: Frieder Hannenheim, git
In-Reply-To: <CAPx1Gvd9+z0th9whCbcA60_bWproPp+kwp3qDmhQOe4G=0=E6A@mail.gmail.com>
Chris Torek <chris.torek@gmail.com> writes:
> On Tue, May 26, 2026 at 6:18 AM Frieder Hannenheim <mail@fhannenheim.net> wrote:
>> I'd like to propose a new flag for git mv, that updates the index
>> like git mv normally would but does not move the file. ...
>
> You may already know this, but technically no flag is needed:
> you can just "git add" the new name and "git rm" the old one,
> with the same effect.
Correct.
> A flag for "git mv" would be convenient (and slightly more
> efficient, not in terms of storage but in terms of CPU time
> spent discovering that the contents under the new name
> already exist in the object database).
May be convenient, but I do not get the "efficient" part. Do you
mean that for two operations "add" and "rm", you need to spend two
index writes plus one file contents hash, as opposed to one index
rite without having to do any contents hash?
> But Git will discover
> the rename on its own in the usual way regardless of how
> you get to that point.
This is not incorrect per-se, but it is a confusing thing to say to
somebody who does not know the equivalence of "mv" and "rm + add".
It would not be clear to them that you are not talking about what
happens during "mv" or "rm + add", but about what happend during
"git log -M", "git diff -M", etc.
There is "git rm --cached" that can be used to recover from an
"oops, I removed the file from the filesystem without telling Git".
$ date >new-file.txt
$ git add new-file.txt
$ rm new-file.txt
$ git rm --cached new-file.txt
I think the requested "feature" is not all that outrageous. It
would be a similar value as a morning-after correction measure for
"oops, I moved the file in the filesystem without telling Git".
$ date >old-file.txt
$ git add old-file.txt
$ mv old-file.txt new-file.txt
$ git mv --cached old-file.txt new-file.txt
Thanks.
^ permalink raw reply
* Re: [PATCH v2 0/9] doc: interpret-trailers: explain key format
From: Kristoffer Haugsbakk @ 2026-05-26 21:45 UTC (permalink / raw)
To: D. Ben Knoble
Cc: Junio C Hamano, git, Christian Couder, jackmanb, Linus Arver
In-Reply-To: <0faba437-31cf-4004-adaf-2dfcd2274a5b@app.fastmail.com>
On Tue, May 26, 2026, at 23:42, Kristoffer Haugsbakk wrote:
>>[snip]
>>
>> I’ve read through the below a few times, and I don’t really have much
>> to add for now :) I think that’s a fine improvement.
>>
>> Whether you roll that into this patch series or wait until the dust
>> settles is up to you.
>
> Many thanks!
Sorry. I forgot to add: the plan right now is to weave it into this series.
^ permalink raw reply
* Re: [PATCH v2 0/9] doc: interpret-trailers: explain key format
From: Kristoffer Haugsbakk @ 2026-05-26 21:42 UTC (permalink / raw)
To: D. Ben Knoble
Cc: Junio C Hamano, git, Christian Couder, jackmanb, Linus Arver
In-Reply-To: <4DD440D4-145A-4A9E-ACBA-8E6ACFA231D1@gmail.com>
On Tue, May 26, 2026, at 23:34, Ben Knoble wrote:
>> Le 24 mai 2026 à 08:41, Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com> a écrit :
>>
>> On Mon, May 11, 2026, at 21:23, D. Ben Knoble wrote:
>>> Overall looks good to me. Repeating a few points throughout the doc
>>> might create headaches if format restrictions are changed, but I think
>>> they are essential points worth repeating for now.
>>
>> Thanks for taking a look again. :)
>
> Thank you for working on it :)
>
>>[snip]
>>
>> Here is a draft of this part of the doc. I have tried to consolidate
>> these two “By default” paragrahs and be more explicit about what “the
>> trailer” is. I have included one unchanged paragraph before and after
>> for context.
>
> I’ve read through the below a few times, and I don’t really have much
> to add for now :) I think that’s a fine improvement.
>
> Whether you roll that into this patch series or wait until the dust
> settles is up to you.
Many thanks!
^ permalink raw reply
* Re: [PATCH v2 0/9] doc: interpret-trailers: explain key format
From: Ben Knoble @ 2026-05-26 21:34 UTC (permalink / raw)
To: Kristoffer Haugsbakk
Cc: Junio C Hamano, git, Christian Couder, jackmanb, Linus Arver
In-Reply-To: <fc1f8149-98c2-48e5-9725-08cc21696cb2@app.fastmail.com>
> Le 24 mai 2026 à 08:41, Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com> a écrit :
>
> On Mon, May 11, 2026, at 21:23, D. Ben Knoble wrote:
>> Overall looks good to me. Repeating a few points throughout the doc
>> might create headaches if format restrictions are changed, but I think
>> they are essential points worth repeating for now.
>
> Thanks for taking a look again. :)
Thank you for working on it :)
>>> [snip]
>>> @@ -81,19 +87,25 @@ trailer.sign.key "Signed-off-by: "
>>> in your configuration, you only need to specify `--trailer="sign: foo"`
>>> on the command line instead of `--trailer="Signed-off-by: foo"`.
>>>
>>> -By default the new trailer will appear at the end of all the existing
>>> -trailers. If there is no existing trailer, the new trailer will appear
>>> -at the end of the input. A blank line will be added before the new
>>> -trailer if there isn't one already.
>>> +By default the new trailer will appear at the end of the trailer block.
>>> +A trailer block will be created with only that trailer if a trailer
>>> +block does not already exist. Recall that a trailer block needs to be
>>> +preceded by a blank line, so a blank line (specifically an empty line)
>>> +will be inserted before the new trailer block in that case.
>>
>> [not strictly related to this patch, but while we're here…]
>>
>> Even in context, I find the original (and new) paragraph somewhat
>> jarring. In "the new trailer," there's no antecedent for "the
>> trailer", so which new trailer are we talking about? The previous
>> paragraph is about "<key-alias>es" for --trailer="<key>: value".
>>
>> We _could_ move this paragraph up one, so that it follows the
>> paragraph on trailers being appended when given with --trailer.
>>
>> Either way, adjusting "the new trailer" to "a new trailer" might feel
>> better to me. Other suggestions welcome.
>
> The paragraph about new trailers originally came right after the
> separated-by sentence:[1]
>
> By default, a '<token>=<value>' or '<token>:<value>' [...]
>
> ------------------------------------------------
> token: value
> ------------------------------------------------
>
> This means that the trimmed <token> and <value> will be separated by
> `': '` (one colon followed by one space).
>
> By default the new trailer will appear [...]
>
> † 1: dfd66ddf (Documentation: add documentation for 'git
> interpret-trailers', 2014-10-13)
>
> Nine years later in [2], a “For convenience, <token>” was added to that *existing paragraph:
>
> [...]
> `': '` (one colon followed by one space). For convenience, the <token> can be a
> shortened string key (e.g., "sign") instead of the full string which should
> appear before the separator on the output (e.g., "Signed-off-by"). This can be
> configured using the 'trailer.<token>.key' configuration variable.
>
> By default the new trailer will appear at the end [...]
>
> † 2: eda2c44c (doc: trailer: mention 'key' in DESCRIPTION, 2023-06-15)
>
> A little later in [3], that part was split into its own paragraph—and
> expanded into two more blocks (source block and paragraph):
>
> [...] <key> and <value> will be separated by `': '` (one colon followed
> by one space).
>
> For convenience, a <keyAlias> can be configured to [...]
>
> ------------------------------------------------
> key: value
> ------------------------------------------------
>
> in your configuration, [...]
>
> By default the new trailer will appear at the end [...]
>
> † 3: 6ccbc667 (trailer doc: <token> is a <key> or <keyAlias>, not both,
> 2023-09-07)
>
>> We _could_ move this paragraph up one, so that it follows the
>> paragraph on trailers being appended when given with --trailer.
>
> But going back to commit [1], there are two paragraphs that talk about
> how “By default” the new trailer will be appended to the end:
>
> By default, a '<token>=<value>' or '<token>:<value>' argument given
> using `--trailer` will be appended after the existing trailers only if
> the last trailer has a different (<token>, <value>) pair (or if there
> is no existing trailer). The <token> and <value> parts will be trimmed
> to remove starting and trailing whitespace, and the resulting trimmed
> <token> and <value> will appear in the message like this:
>
> ------------------------------------------------
> token: value
> ------------------------------------------------
>
> This means that the trimmed <token> and <value> will be separated by
> `': '` (one colon followed by one space).
>
> By default the new trailer will appear at the end of all the existing
> trailers. If there is no existing trailer, the new trailer will appear
> after the commit message part of the ouput, and, if there is no line
> with only spaces at the end of the commit message part, one blank line
> will be added before the new trailer.
>
> These two seem to overlap? They both talk about appending. Why does one
> talk about how specifically <token>/<key> and <value> will be treated
> when appended, then a later paragraph *also* says that it will be
> appended?
>
> Here is a draft of this part of the doc. I have tried to consolidate
> these two “By default” paragrahs and be more explicit about what “the
> trailer” is. I have included one unchanged paragraph before and after
> for context.
I’ve read through the below a few times, and I don’t really have much to add for now :) I think that’s a fine improvement.
Whether you roll that into this patch series or wait until the dust settles is up to you.
> ***
>
> Some configuration variables control the way the `--trailer` arguments
> are applied to each input and the way any existing trailer in
> the input is changed. They also make it possible to
> automatically add some trailers.
>
> Let's consider new trailers added with `--trailer`.
> By default, the new trailer will appear at the end of the trailer block.
> Also by default, this new trailer will only be added
> if the last trailer is different to it.
> A trailer block will be created with only that trailer if a trailer
> block does not already exist. Recall that a trailer block needs to be
> preceded by a blank line, so a blank line (specifically an empty line)
> will be inserted before the new trailer block in that case.
>
> More concretely, this is how the new trailer is added: a `<key>=<value>`
> or `<key>:<value>` argument given using `--trailer` will be appended
> after the existing trailers. The _<key>_ and _<value>_ parts will be
> trimmed to remove starting and trailing whitespace, and the resulting
> trimmed _<key>_ and _<value>_ will appear in the output like this:
>
> ------------------------------------------------
> key: value
> ------------------------------------------------
>
> This means that the trimmed _<key>_ and _<value>_ will be separated by
> "`:`{nbsp}" (one colon followed by one space).
>
> ***
>
>> [snip]
>>> -a group of one or more lines that (i) is all trailers, or (ii) contains at
>>> -least one Git-generated or user-configured trailer and consists of at
>>> +Existing trailers are extracted from the input by looking for the
>>> +trailer block. Concretely, that is a group of one or more lines that (i)
>>> +is all trailers, or (ii) contains at least one Git-generated or
>>> +user-configured trailer and consists of at
>>> [snip]
^ permalink raw reply
* Re: [PATCH v3] git-jump: pick a mode automatically when invoked without arguments
From: Erik Cervin Edin @ 2026-05-26 21:33 UTC (permalink / raw)
To: git
In-Reply-To: <20260522052821.GC861761@coredump.intra.peff.net>
On 26/05/22 01:28AM, Jeff King wrote:
> On Thu, May 21, 2026 at 01:45:09PM +0000, Greg Hurrell via GitGitGadget wrote:
>
> > * Don't both teaching "auto" to select "ws" mode, because it is always
> > subsumed by "diff".
>
> Dropping the "ws" mode from auto makes sense to me. It could be slotted
> in between "merge" and "diff" (a whitespace problem always implies a
> diff, but a diff does not always imply a whitespace problem). But would
> that actually be useful?
When I originally proposed the idea of a third branch, there was a
subtle difference -- it was a git diff --cached --check.
On 26/05/14 05:40PM, Erik Cervin Edin wrote:
> If we're going to teach git-jump how to be more clever about where to jump,
> does it also make sense to bake `git jump ws` into this?
>
> elif ! git diff --cached --check >/dev/null 2>&1; then
> mode_ws --cached "$@"
Ever so often I come across file with a diff --check offending
white-space (often a missing newline at the end of some file) and
because I have a commit hook set up, I have to go looking for where that
error is. In this particular case it's always a staged change, and then
a *staged* white space problem doesn't imply a diff.
This happens rarely enough that I haven't internalized that I can use
"git jump diff --check --cached" and it takes me a while to navigate to
the offending files.
But the main suggestion was really considering the possibility to
expand this beyond these two auto jumps in the future -- I'm not sure
an auto jump that goes looking for staged white spaces issues would be
useful to anyone in practise and at this stage I thinks it's best to
drop it.
^ permalink raw reply
* Re: git mv after the fact
From: Ben Knoble @ 2026-05-26 21:29 UTC (permalink / raw)
To: Chris Torek; +Cc: Frieder Hannenheim, git
In-Reply-To: <CAPx1Gvd9+z0th9whCbcA60_bWproPp+kwp3qDmhQOe4G=0=E6A@mail.gmail.com>
> Le 26 mai 2026 à 12:46, Chris Torek <chris.torek@gmail.com> a écrit :
>
> On Tue, May 26, 2026 at 6:18 AM Frieder Hannenheim <mail@fhannenheim.net> wrote:
>> I'd like to propose a new flag for git mv, that updates the index
>> like git mv normally would but does not move the file. ...
>
> You may already know this, but technically no flag is needed:
> you can just "git add" the new name and "git rm" the old one,
> with the same effect.
Or indeed, « git add old new » should also work, I think.
> A flag for "git mv" would be convenient (and slightly more
> efficient, not in terms of storage but in terms of CPU time
> spent discovering that the contents under the new name
> already exist in the object database). But Git will discover
> the rename on its own in the usual way regardless of how
> you get to that point.
>
> Chris
^ permalink raw reply
* Re: [PATCH 0/5] git son: add command to create independent child repositories
From: Ben Knoble @ 2026-05-26 21:27 UTC (permalink / raw)
To: Evan Haque via GitGitGadget; +Cc: git, Evan Haque
In-Reply-To: <pull.2122.git.1779814052.gitgitgadget@gmail.com>
> Le 26 mai 2026 à 13:08, Evan Haque via GitGitGadget <gitgitgadget@gmail.com> a écrit :
>
>
> Motivation
> ==========
>
> When spinning off a new project that is related to an existing repository,
> there is no built-in way to create a child repository that maintains a link
> back to its parent without the tight coupling of submodules. Submodules pin
> the child to a specific commit and require the parent to track the child in
> its index, which is too heavyweight when the child is meant to be fully
> independent.
>
> The typical workflow today is manual: git init, git remote add, update
> .gitignore — three steps that are easy to forget or get wrong. git son
> automates this and establishes a lightweight convention for the parent-child
> relationship: a remote named parent in the child, and nothing in the parent
> except an ignore rule.
I don’t really understand the motivation, but if your goal is to create another repo with the current one as a remote, how does something like
git clone . child
help you? (I’m pretty sure you can even set the remote name to « parent » if you wish.)
You also didn’t mention worktrees or subtrees, which might be useful for you.
^ permalink raw reply
* Re: [PATCH/RFC 1/5] replay: support replaying 2-parent merges
From: Kristoffer Haugsbakk @ 2026-05-26 21:15 UTC (permalink / raw)
To: Koji Nakamaru, git; +Cc: Elijah Newren, Patrick Steinhardt, Johannes Schindelin
In-Reply-To: <034ab0f83822e6db67baa423d9fcb753b12b5ac8.1778107405.git.gitgitgadget@gmail.com>
On Thu, May 7, 2026, at 00:43, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>[snip]
> diff --git a/replay.c b/replay.c
>[snip]
> +out:
> + free(ancestor_name);
> + free_commit_list(parent_bases);
> + free_commit_list(replayed_bases);
`free_commit_list` is deprecated in favor of `commit_list_free` since
52882024 (Merge branch 'ps/commit-list-functions-renamed', 2026-02-13).
> + merge_finalize(&remerge_opt, &remerge_res);
> + merge_finalize(&new_merge_opt, &new_merge_res);
> + return picked;
> }
>[snip]
^ permalink raw reply
* [PATCH v2 2/2] restore: avoid sparse index expansion
From: Derrick Stolee via GitGitGadget @ 2026-05-26 20:26 UTC (permalink / raw)
To: git; +Cc: gitster, Derrick Stolee, Derrick Stolee
In-Reply-To: <pull.2121.v2.git.1779827195.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
Teach update_some() to handle sparse directory entries at the tree
level rather than expanding the entire sparse index. When iterating a
source tree during checkout/restore operations:
- If a directory matches a sparse directory entry with the same OID,
skip it entirely (no change needed).
- If the OID differs and we are in non-overlay mode (e.g., restore
--staged), update the sparse directory entry's OID in place. This
is semantically correct because non-overlay mode removes paths not
in the source tree anyway.
- In overlay mode (e.g., checkout <tree> -- .), fall through to
recursive descent so individual file entries are preserved
correctly.
Also switch from index_name_pos() to index_name_pos_sparse() for
individual file lookups to avoid triggering ensure_full_index() when
the file is already individually tracked in the index.
Update the test expectation in t1092 to assert that 'restore --staged'
no longer expands the sparse index.
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
builtin/checkout.c | 65 +++++++++++++++++++++---
t/t1092-sparse-checkout-compatibility.sh | 8 +--
2 files changed, 63 insertions(+), 10 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 1345e8574a..86e23a07b1 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -31,6 +31,7 @@
#include "revision.h"
#include "sequencer.h"
#include "setup.h"
+#include "sparse-index.h"
#include "strvec.h"
#include "submodule.h"
#include "symlinks.h"
@@ -141,15 +142,65 @@ static int post_checkout_hook(struct commit *old_commit, struct commit *new_comm
return run_hooks_opt(the_repository, "post-checkout", &opt);
}
+/*
+ * Handle a tree object and determine if we need to recurse into the
+ * tree (READ_TREE_RECURSIVE) or skip it (0).
+ */
+static int try_update_sparse_directory(const struct object_id *oid,
+ struct strbuf *base,
+ const char *pathname,
+ int overlay_mode)
+{
+ struct strbuf dirpath = STRBUF_INIT;
+ struct cache_entry *old;
+ int pos, result = READ_TREE_RECURSIVE;
+
+ if (!the_repository->index->sparse_index)
+ return result;
+
+ strbuf_addbuf(&dirpath, base);
+ strbuf_addstr(&dirpath, pathname);
+ strbuf_addch(&dirpath, '/');
+
+ pos = index_name_pos_sparse(the_repository->index,
+ dirpath.buf, dirpath.len);
+ if (pos < 0)
+ goto cleanup;
+
+ old = the_repository->index->cache[pos];
+ if (!S_ISSPARSEDIR(old->ce_mode))
+ goto cleanup;
+
+ if (oideq(oid, &old->oid)) {
+ /* Tree content already matches; no need to descend. */
+ result = 0;
+ } else if (!overlay_mode) {
+ /*
+ * In non-overlay mode (e.g., restore --staged), replace the
+ * sparse directory OID directly since files not present in
+ * the source tree should be removed anyway.
+ */
+ oidcpy(&old->oid, oid);
+ old->ce_flags |= CE_UPDATE;
+ result = 0;
+ }
+
+cleanup:
+ strbuf_release(&dirpath);
+ return result;
+}
+
static int update_some(const struct object_id *oid, struct strbuf *base,
- const char *pathname, unsigned mode, void *context UNUSED)
+ const char *pathname, unsigned mode, void *context)
{
int len;
struct cache_entry *ce;
int pos;
+ int overlay_mode = context ? *((int *)context) : 1;
if (S_ISDIR(mode))
- return READ_TREE_RECURSIVE;
+ return try_update_sparse_directory(oid, base, pathname,
+ overlay_mode);
len = base->len + strlen(pathname);
ce = make_empty_cache_entry(the_repository->index, len);
@@ -165,7 +216,7 @@ static int update_some(const struct object_id *oid, struct strbuf *base,
* entry in place. Whether it is UPTODATE or not, checkout_entry will
* do the right thing.
*/
- pos = index_name_pos(the_repository->index, ce->name, ce->ce_namelen);
+ pos = index_name_pos_sparse(the_repository->index, ce->name, ce->ce_namelen);
if (pos >= 0) {
struct cache_entry *old = the_repository->index->cache[pos];
if (ce->ce_mode == old->ce_mode &&
@@ -182,10 +233,11 @@ static int update_some(const struct object_id *oid, struct strbuf *base,
return 0;
}
-static int read_tree_some(struct tree *tree, const struct pathspec *pathspec)
+static int read_tree_some(struct tree *tree, const struct pathspec *pathspec,
+ int overlay_mode)
{
read_tree(the_repository, tree,
- pathspec, update_some, NULL);
+ pathspec, update_some, &overlay_mode);
/* update the index with the given tree's info
* for all args, expanding wildcards, and exit
@@ -580,7 +632,8 @@ static int checkout_paths(const struct checkout_opts *opts,
return error(_("index file corrupt"));
if (opts->source_tree)
- read_tree_some(opts->source_tree, &opts->pathspec);
+ read_tree_some(opts->source_tree, &opts->pathspec,
+ opts->overlay_mode);
if (opts->merge)
unmerge_index(the_repository->index, &opts->pathspec, CE_MATCHED);
diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
index d69434e7ab..8186da5c88 100755
--- a/t/t1092-sparse-checkout-compatibility.sh
+++ b/t/t1092-sparse-checkout-compatibility.sh
@@ -2608,19 +2608,19 @@ test_expect_success 'restore --staged with wildcards' '
test_all_match git diff --cached
'
-test_expect_success 'sparse-index is expanded: restore --staged' '
+test_expect_success 'sparse-index is not expanded: restore --staged' '
init_repos &&
git -C sparse-index checkout -b restore-staged-exp base &&
git -C sparse-index reset --soft update-folder1 &&
- ensure_expanded restore --staged .
+ ensure_not_expanded restore --staged .
'
-test_expect_success 'sparse-index is expanded: restore --source --staged' '
+test_expect_success 'sparse-index is not expanded: restore --source --staged' '
init_repos &&
git -C sparse-index checkout -b restore-source-staged base &&
- ensure_expanded restore --source update-folder1 --staged .
+ ensure_not_expanded restore --source update-folder1 --staged .
'
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 1/2] t1092: test 'git restore' with sparse index
From: Derrick Stolee via GitGitGadget @ 2026-05-26 20:26 UTC (permalink / raw)
To: git; +Cc: gitster, Derrick Stolee, Derrick Stolee
In-Reply-To: <pull.2121.v2.git.1779827195.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
A user reported that 'git restore --staged .' causes the sparse index to
expand. This is somewhat natural because the '.' pathspec means 'check
every path'. However, the restore will not update paths marked with the
SKIP_WORKTREE bit, so we shouldn't need to process such entries.
For now, establish the current behavior, including the sparse index
expansion, in the t1092 test case as a baseline.
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
t/t1092-sparse-checkout-compatibility.sh | 50 ++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
index d98cb4ac11..d69434e7ab 100755
--- a/t/t1092-sparse-checkout-compatibility.sh
+++ b/t/t1092-sparse-checkout-compatibility.sh
@@ -2573,4 +2573,54 @@ test_expect_success 'sparse-index is not expanded: merge-ours' '
ensure_not_expanded merge -s ours merge-right
'
+test_expect_success 'restore --staged with sparse definition' '
+ init_repos &&
+
+ # Stage changes within the sparse definition
+ test_all_match git checkout -b restore-staged-1 base &&
+ test_all_match git reset --soft update-deep &&
+ test_all_match git restore --staged . &&
+ test_all_match git status --porcelain=v2 &&
+ test_all_match git diff --cached
+'
+
+test_expect_success 'restore --staged with outside sparse definition' '
+ init_repos &&
+
+ # Stage changes that include paths outside the sparse definition.
+ # Although the working tree differs between full and sparse checkouts
+ # after restore, the state of the index should be the same.
+ test_all_match git checkout -b restore-staged-2 base &&
+ test_all_match git reset --soft update-folder1 &&
+ test_sparse_match git restore --staged . &&
+ git -C full-checkout restore --staged . &&
+ test_all_match git ls-files -s -- folder1 &&
+ test_all_match git diff --cached -- folder1
+'
+
+test_expect_success 'restore --staged with wildcards' '
+ init_repos &&
+
+ test_all_match git checkout -b restore-staged-3 base &&
+ test_all_match git reset --soft update-deep &&
+ test_all_match git restore --staged "deep/*" &&
+ test_all_match git status --porcelain=v2 &&
+ test_all_match git diff --cached
+'
+
+test_expect_success 'sparse-index is expanded: restore --staged' '
+ init_repos &&
+
+ git -C sparse-index checkout -b restore-staged-exp base &&
+ git -C sparse-index reset --soft update-folder1 &&
+ ensure_expanded restore --staged .
+'
+
+test_expect_success 'sparse-index is expanded: restore --source --staged' '
+ init_repos &&
+
+ git -C sparse-index checkout -b restore-source-staged base &&
+ ensure_expanded restore --source update-folder1 --staged .
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 0/2] restore: better integrate with sparse index
From: Derrick Stolee via GitGitGadget @ 2026-05-26 20:26 UTC (permalink / raw)
To: git; +Cc: gitster, Derrick Stolee
In-Reply-To: <pull.2121.git.1779644412.gitgitgadget@gmail.com>
There's still a long tail of situations where Git expands a sparse index
in-memory in order to operate on blob path entries instead of intelligently
handling trees. I was recently alerted to one such case with git restore
--staged -- ..
The basic idea here is that the pathspec . signals that all paths matter,
but what we want to do across those pathspecs will ignore the expanded blob
paths with the SKIP_WORKTREE bit, so we should avoid expanding the tree when
we can.
This series has two patches: first a test to demonstrate the baseline
behavior of git restore across different sparsity cases as well as
demonstrate that the index is currently expanded. The second patch includes
the fix and maintains the same end-to-end behavior with the only change
being the performance improvement from not expanding the sparse index.
Update in v2
============
The logic around handling a tree entry is extracted to a helper method,
making the diff easier to read.
Thanks, -Stolee
Derrick Stolee (2):
t1092: test 'git restore' with sparse index
restore: avoid sparse index expansion
builtin/checkout.c | 65 +++++++++++++++++++++---
t/t1092-sparse-checkout-compatibility.sh | 50 ++++++++++++++++++
2 files changed, 109 insertions(+), 6 deletions(-)
base-commit: aec3f587505a472db67e9462d0702e7d463a449d
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2121%2Fderrickstolee%2Frestore-sparse-index-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2121/derrickstolee/restore-sparse-index-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2121
Range-diff vs v1:
1: 7c56d03830 = 1: 7c56d03830 t1092: test 'git restore' with sparse index
2: 47542cbd42 ! 2: 88f5d26a33 restore: avoid sparse index expansion
@@ builtin/checkout.c
#include "submodule.h"
#include "symlinks.h"
@@ builtin/checkout.c: static int post_checkout_hook(struct commit *old_commit, struct commit *new_comm
+ return run_hooks_opt(the_repository, "post-checkout", &opt);
}
++/*
++ * Handle a tree object and determine if we need to recurse into the
++ * tree (READ_TREE_RECURSIVE) or skip it (0).
++ */
++static int try_update_sparse_directory(const struct object_id *oid,
++ struct strbuf *base,
++ const char *pathname,
++ int overlay_mode)
++{
++ struct strbuf dirpath = STRBUF_INIT;
++ struct cache_entry *old;
++ int pos, result = READ_TREE_RECURSIVE;
++
++ if (!the_repository->index->sparse_index)
++ return result;
++
++ strbuf_addbuf(&dirpath, base);
++ strbuf_addstr(&dirpath, pathname);
++ strbuf_addch(&dirpath, '/');
++
++ pos = index_name_pos_sparse(the_repository->index,
++ dirpath.buf, dirpath.len);
++ if (pos < 0)
++ goto cleanup;
++
++ old = the_repository->index->cache[pos];
++ if (!S_ISSPARSEDIR(old->ce_mode))
++ goto cleanup;
++
++ if (oideq(oid, &old->oid)) {
++ /* Tree content already matches; no need to descend. */
++ result = 0;
++ } else if (!overlay_mode) {
++ /*
++ * In non-overlay mode (e.g., restore --staged), replace the
++ * sparse directory OID directly since files not present in
++ * the source tree should be removed anyway.
++ */
++ oidcpy(&old->oid, oid);
++ old->ce_flags |= CE_UPDATE;
++ result = 0;
++ }
++
++cleanup:
++ strbuf_release(&dirpath);
++ return result;
++}
++
static int update_some(const struct object_id *oid, struct strbuf *base,
- const char *pathname, unsigned mode, void *context UNUSED)
+ const char *pathname, unsigned mode, void *context)
@@ builtin/checkout.c: static int post_checkout_hook(struct commit *old_commit, str
int pos;
+ int overlay_mode = context ? *((int *)context) : 1;
-- if (S_ISDIR(mode))
-+ if (S_ISDIR(mode)) {
-+ /*
-+ * If this directory exists as a sparse directory entry in
-+ * the index, we can handle it at the tree level without
-+ * descending into individual files.
-+ */
-+ if (the_repository->index->sparse_index) {
-+ struct strbuf dirpath = STRBUF_INIT;
-+
-+ strbuf_addbuf(&dirpath, base);
-+ strbuf_addstr(&dirpath, pathname);
-+ strbuf_addch(&dirpath, '/');
-+
-+ pos = index_name_pos_sparse(the_repository->index,
-+ dirpath.buf, dirpath.len);
-+ if (pos >= 0) {
-+ struct cache_entry *old =
-+ the_repository->index->cache[pos];
-+ if (S_ISSPARSEDIR(old->ce_mode)) {
-+ if (oideq(oid, &old->oid)) {
-+ strbuf_release(&dirpath);
-+ return 0;
-+ }
-+ if (!overlay_mode) {
-+ /*
-+ * In non-overlay mode (e.g.,
-+ * restore --staged), we can
-+ * replace the sparse dir OID
-+ * directly since files not in
-+ * the source tree should be
-+ * removed anyway.
-+ */
-+ oidcpy(&old->oid, oid);
-+ old->ce_flags |= CE_UPDATE;
-+ strbuf_release(&dirpath);
-+ return 0;
-+ }
-+ }
-+ }
-+ strbuf_release(&dirpath);
-+ }
- return READ_TREE_RECURSIVE;
-+ }
+ if (S_ISDIR(mode))
+- return READ_TREE_RECURSIVE;
++ return try_update_sparse_directory(oid, base, pathname,
++ overlay_mode);
len = base->len + strlen(pathname);
ce = make_empty_cache_entry(the_repository->index, len);
--
gitgitgadget
^ permalink raw reply
* Re: [PATCH v4] config: improve diagnostic for "set" with missing value
From: Harald Nordgren @ 2026-05-26 19:24 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget; +Cc: git, Kristoffer Haugsbakk
In-Reply-To: <pull.2302.v4.git.git.1779823288005.gitgitgadget@gmail.com>
I forgot to update the PR description on GitHub, it should have read:
- Diagnose the 1-arg set form (explicit and implicit) directly: report
the missing value, and suggest the split form only when the prefix
before `=` is a valid key.
- Did not act on Junio's secondary suggestion to reword the 2-arg
`error: invalid key: <key>`, fix seemed to become too big.
Harald
On Tue, May 26, 2026 at 9:21 PM Harald Nordgren via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> "git config set pull.rebase=false" currently fails with "wrong
> number of arguments", and the implicit form "git config
> pull.rebase=false" fails with "invalid key". Neither points at
> the real problem: the value is missing.
>
> Report that directly, and when the argument has the shape
> "<valid-key>=<value>", also suggest the split form:
>
> $ git config set pull.rebase=false
> error: missing value to set to the variable 'pull.rebase=false'
> hint: did you mean "git config set pull.rebase false"?
>
> When the prefix before "=" is not a valid key, drop the hint:
>
> $ git config set foo=bar
> error: missing value to set to a variable with an invalid name 'foo=bar'
>
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
> ---
> config: suggest the correct form when key contains "="
>
> * Skip the hint when the inferred value contains whitespace, so git
> config set pull.rebase=false "hello world" no longer suggests a
> malformed command.
> * Replace the inline actions == 0 check with a named actions_implicit
> flag, simplfied the code.
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2302%2FHaraldNordgren%2Fconfig-hint-equals-key-v4
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2302/HaraldNordgren/config-hint-equals-key-v4
> Pull-Request: https://github.com/git/git/pull/2302
>
> Range-diff vs v3:
>
> 1: 6b9d66361d ! 1: 780b99409c config: suggest the correct form when key contains "=" in set context
> @@ Metadata
> Author: Harald Nordgren <haraldnordgren@gmail.com>
>
> ## Commit message ##
> - config: suggest the correct form when key contains "=" in set context
> + config: improve diagnostic for "set" with missing value
>
> - A user who types "git config pull.rebase=false" gets only "error:
> - invalid key: pull.rebase=false" with no clue what went wrong.
> + "git config set pull.rebase=false" currently fails with "wrong
> + number of arguments", and the implicit form "git config
> + pull.rebase=false" fails with "invalid key". Neither points at
> + the real problem: the value is missing.
>
> - Emit a "did you mean ..." hint suggesting the split form. Restrict it
> - to plausible-set contexts ("git config set", bare "git config <key>",
> - and their 2-arg forms); explicit "get"/"unset" keep the existing error.
> + Report that directly, and when the argument has the shape
> + "<valid-key>=<value>", also suggest the split form:
>
> - "=" is legal inside a subsection, so only fire when "=" lands after
> - the last ".". When the user supplied a separate value, use it in the
> - suggestion instead of the suffix after "=":
> + $ git config set pull.rebase=false
> + error: missing value to set to the variable 'pull.rebase=false'
> + hint: did you mean "git config set pull.rebase false"?
>
> - $ git config set pull.rebase=false true
> - error: invalid key: pull.rebase=false
> - hint: did you mean "git config set pull.rebase true"?
> + When the prefix before "=" is not a valid key, drop the hint:
> +
> + $ git config set foo=bar
> + error: missing value to set to a variable with an invalid name 'foo=bar'
>
> - Signed-off-by: Harald Nordgren <harald.nordgren@kostdoktorn.se>
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
>
> ## builtin/config.c ##
> @@ builtin/config.c: static void check_argc(int argc, int min, int max)
> exit(129);
> }
>
> -+static void advise_setting_with_equals(const char *key, const char *value)
> ++static int is_valid_key(const char *key)
> +{
> + const char *last_dot = strrchr(key, '.');
> -+ const char *eq;
> +
> -+ if (!last_dot)
> -+ return;
> -+ eq = strchr(last_dot + 1, '=');
> -+ if (!eq)
> -+ return;
> -+ if (!value)
> -+ value = eq + 1;
> -+ if (!*value || strpbrk(value, " \t\n"))
> -+ return;
> -+ advise(_("did you mean \"git config set %.*s %s\"?"),
> -+ (int)(eq - key), key, value);
> ++ return last_dot && isalpha(last_dot[1]);
> ++}
> ++
> ++static NORETURN void die_missing_set_value(const char *arg)
> ++{
> ++ const char *last_dot = strrchr(arg, '.');
> ++ const char *eq = last_dot ? strchr(last_dot + 1, '=') : NULL;
> ++ char *prefix = eq ? xstrndup(arg, eq - arg) : NULL;
> ++
> ++ if (prefix && is_valid_key(prefix)) {
> ++ error(_("missing value to set to the variable '%s'"), arg);
> ++ advise(_("did you mean \"git config set %s %s\"?"),
> ++ prefix, eq + 1);
> ++ } else if (is_valid_key(arg)) {
> ++ error(_("missing value to set to the variable '%s'"), arg);
> ++ } else {
> ++ error(_("missing value to set to a variable with an invalid name '%s'"),
> ++ arg);
> ++ }
> ++ free(prefix);
> ++ exit(129);
> +}
> +
> static void show_config_origin(const struct config_display_options *opts,
> @@ builtin/config.c: static int cmd_config_set(int argc, const char **argv, const c
>
> argc = parse_options(argc, argv, prefix, opts, builtin_config_set_usage,
> PARSE_OPT_STOP_AT_NON_OPTION);
> -+ if (argc == 1 && strchr(argv[0], '=')) {
> -+ error(_("wrong number of arguments, should be 2"));
> -+ advise_setting_with_equals(argv[0], NULL);
> -+ exit(129);
> -+ }
> ++ if (argc == 1)
> ++ die_missing_set_value(argv[0]);
> check_argc(argc, 2, 2);
>
> if ((flags & CONFIG_FLAGS_FIXED_VALUE) && !value_pattern)
> -@@ builtin/config.c: static int cmd_config_set(int argc, const char **argv, const char *prefix,
> - error(_("cannot overwrite multiple values with a single value\n"
> - " Use --value=<pattern>, --append or --all to change %s."), argv[0]);
> - }
> -+ if (ret == CONFIG_INVALID_KEY)
> -+ advise_setting_with_equals(argv[0], argv[1]);
> -
> - location_options_release(&location_opts);
> - free(comment);
> @@ builtin/config.c: static int cmd_config_actions(int argc, const char **argv, const char *prefix)
> };
> char *value = NULL, *comment = NULL;
> @@ builtin/config.c: static int cmd_config_actions(int argc, const char **argv, con
> case 1: actions = ACTION_GET; break;
> case 2: actions = ACTION_SET; break;
> @@ builtin/config.c: static int cmd_config_actions(int argc, const char **argv, const char *prefix)
> - if (ret == CONFIG_NOTHING_SET)
> - error(_("cannot overwrite multiple values with a single value\n"
> - " Use a regexp, --add or --replace-all to change %s."), argv[0]);
> -+ else if (ret == CONFIG_INVALID_KEY)
> -+ advise_setting_with_equals(argv[0], argv[1]);
> - }
> - else if (actions == ACTION_SET_ALL) {
> - check_write(&location_opts.source);
> -@@ builtin/config.c: static int cmd_config_actions(int argc, const char **argv, const char *prefix)
> - check_argc(argc, 1, 2);
> - ret = get_value(&location_opts, &display_opts, argv[0], argv[1],
> - 0, flags);
> -+ if (ret == CONFIG_INVALID_KEY && actions_implicit)
> -+ advise_setting_with_equals(argv[0], NULL);
> - }
> - else if (actions == ACTION_GET_ALL) {
> - check_argc(argc, 1, 2);
> + error(_("no action specified"));
> + exit(129);
> + }
> ++ if (actions_implicit && argc == 1) {
> ++ const char *last_dot = strrchr(argv[0], '.');
> ++ if (last_dot && strchr(last_dot + 1, '='))
> ++ die_missing_set_value(argv[0]);
> ++ }
> + if (display_opts.omit_values &&
> + !(actions == ACTION_LIST || actions == ACTION_GET_REGEXP)) {
> + error(_("--name-only is only applicable to --list or --get-regexp"));
>
> ## t/t1300-config.sh ##
> @@ t/t1300-config.sh: test_expect_success 'invalid key' '
> test_must_fail git config inval.2key blabla
> '
>
> -+test_expect_success 'misplaced "=" in key: bare 1-arg form hints' '
> -+ test_must_fail git config pull.rebase=false 2>err &&
> -+ test_grep "invalid key: pull\\.rebase=false" err &&
> ++test_expect_success 'set with 1 arg of "key=value": valid key suggests split form' '
> ++ test_must_fail git config set pull.rebase=false 2>err &&
> ++ test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
> + test_grep "did you mean .git config set pull\\.rebase false." err
> +'
> +
> -+test_expect_success 'misplaced "=" in key: bare 2-arg form uses given value' '
> -+ test_must_fail git config pull.rebase=false true 2>err &&
> -+ test_grep "did you mean .git config set pull\\.rebase true." err
> -+'
> -+
> -+test_expect_success 'misplaced "=" in key: set subcommand uses given value' '
> -+ test_must_fail git config set pull.rebase=false true 2>err &&
> -+ test_grep "did you mean .git config set pull\\.rebase true." err
> -+'
> -+
> -+test_expect_success 'misplaced "=" in key: set with single arg hints' '
> -+ test_must_fail git config set pull.rebase=false 2>err &&
> -+ test_grep "wrong number of arguments" err &&
> ++test_expect_success 'set with 1 arg of "key=value": implicit form suggests split form' '
> ++ test_must_fail git config pull.rebase=false 2>err &&
> ++ test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
> + test_grep "did you mean .git config set pull\\.rebase false." err
> +'
> +
> -+test_expect_success 'misplaced "=" in key: explicit --get does not hint' '
> -+ test_must_fail git config --get pull.rebase=false 2>err &&
> -+ test_grep "invalid key: pull\\.rebase=false" err &&
> ++test_expect_success 'set with 1 arg of "key=value": invalid key does not suggest split form' '
> ++ test_must_fail git config set foo=bar 2>err &&
> ++ test_grep "missing value to set to a variable with an invalid name .foo=bar." err &&
> + test_grep ! "did you mean" err
> +'
> +
> -+test_expect_success 'misplaced "=" in key: get subcommand does not hint' '
> -+ test_must_fail git config get pull.rebase=false 2>err &&
> ++test_expect_success 'set with 1 arg: variable name starting with digit is invalid' '
> ++ test_must_fail git config set foo.1bar=baz 2>err &&
> ++ test_grep "missing value to set to a variable with an invalid name .foo\\.1bar=baz." err &&
> + test_grep ! "did you mean" err
> +'
> +
> -+test_expect_success 'misplaced "=" in key: unset subcommand does not hint' '
> -+ test_must_fail git config unset pull.rebase=false 2>err &&
> ++test_expect_success 'set with 1 arg of valid key reports missing value' '
> ++ test_must_fail git config set pull.rebase 2>err &&
> ++ test_grep "missing value to set to the variable .pull\\.rebase." err &&
> + test_grep ! "did you mean" err
> +'
> +
> -+test_expect_success 'misplaced "=" in key: value with whitespace skips hint' '
> -+ test_must_fail git config set pull.rebase=false "hello world" 2>err &&
> -+ test_grep "invalid key: pull\\.rebase=false" err &&
> ++test_expect_success 'set with 2 args including "=" in invalid key does not suggest' '
> ++ test_must_fail git config set pull.rebase=false true 2>err &&
> + test_grep ! "did you mean" err
> +'
> +
> -+test_expect_success '"=" inside subsection is valid, no hint' '
> ++test_expect_success '"=" inside subsection is valid' '
> + test_when_finished "rm -f subsection.cfg" &&
> -+ git config set -f subsection.cfg foo.bar=baz.boo qux 2>err &&
> -+ test_grep ! "did you mean" err &&
> ++ git config set -f subsection.cfg foo.bar=baz.boo qux &&
> + echo qux >expect &&
> + git config get -f subsection.cfg foo.bar=baz.boo >actual &&
> + test_cmp expect actual
>
>
> builtin/config.c | 39 ++++++++++++++++++++++++++++++++++++++-
> t/t1300-config.sh | 43 +++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 81 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/config.c b/builtin/config.c
> index cf4ba0f7cc..6fe2d85814 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -1,6 +1,7 @@
> #define USE_THE_REPOSITORY_VARIABLE
> #include "builtin.h"
> #include "abspath.h"
> +#include "advice.h"
> #include "config.h"
> #include "color.h"
> #include "date.h"
> @@ -210,6 +211,33 @@ static void check_argc(int argc, int min, int max)
> exit(129);
> }
>
> +static int is_valid_key(const char *key)
> +{
> + const char *last_dot = strrchr(key, '.');
> +
> + return last_dot && isalpha(last_dot[1]);
> +}
> +
> +static NORETURN void die_missing_set_value(const char *arg)
> +{
> + const char *last_dot = strrchr(arg, '.');
> + const char *eq = last_dot ? strchr(last_dot + 1, '=') : NULL;
> + char *prefix = eq ? xstrndup(arg, eq - arg) : NULL;
> +
> + if (prefix && is_valid_key(prefix)) {
> + error(_("missing value to set to the variable '%s'"), arg);
> + advise(_("did you mean \"git config set %s %s\"?"),
> + prefix, eq + 1);
> + } else if (is_valid_key(arg)) {
> + error(_("missing value to set to the variable '%s'"), arg);
> + } else {
> + error(_("missing value to set to a variable with an invalid name '%s'"),
> + arg);
> + }
> + free(prefix);
> + exit(129);
> +}
> +
> static void show_config_origin(const struct config_display_options *opts,
> const struct key_value_info *kvi,
> struct strbuf *buf)
> @@ -1133,6 +1161,8 @@ static int cmd_config_set(int argc, const char **argv, const char *prefix,
>
> argc = parse_options(argc, argv, prefix, opts, builtin_config_set_usage,
> PARSE_OPT_STOP_AT_NON_OPTION);
> + if (argc == 1)
> + die_missing_set_value(argv[0]);
> check_argc(argc, 2, 2);
>
> if ((flags & CONFIG_FLAGS_FIXED_VALUE) && !value_pattern)
> @@ -1371,6 +1401,7 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
> };
> char *value = NULL, *comment = NULL;
> int ret = 0;
> + int actions_implicit;
> struct key_value_info default_kvi = KVI_INIT;
>
> argc = parse_options(argc, argv, prefix, opts,
> @@ -1385,7 +1416,8 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
> exit(129);
> }
>
> - if (actions == 0)
> + actions_implicit = (actions == 0);
> + if (actions_implicit)
> switch (argc) {
> case 1: actions = ACTION_GET; break;
> case 2: actions = ACTION_SET; break;
> @@ -1394,6 +1426,11 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
> error(_("no action specified"));
> exit(129);
> }
> + if (actions_implicit && argc == 1) {
> + const char *last_dot = strrchr(argv[0], '.');
> + if (last_dot && strchr(last_dot + 1, '='))
> + die_missing_set_value(argv[0]);
> + }
> if (display_opts.omit_values &&
> !(actions == ACTION_LIST || actions == ACTION_GET_REGEXP)) {
> error(_("--name-only is only applicable to --list or --get-regexp"));
> diff --git a/t/t1300-config.sh b/t/t1300-config.sh
> index 11fc976f3a..4a8a381bd8 100755
> --- a/t/t1300-config.sh
> +++ b/t/t1300-config.sh
> @@ -469,6 +469,49 @@ test_expect_success 'invalid key' '
> test_must_fail git config inval.2key blabla
> '
>
> +test_expect_success 'set with 1 arg of "key=value": valid key suggests split form' '
> + test_must_fail git config set pull.rebase=false 2>err &&
> + test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
> + test_grep "did you mean .git config set pull\\.rebase false." err
> +'
> +
> +test_expect_success 'set with 1 arg of "key=value": implicit form suggests split form' '
> + test_must_fail git config pull.rebase=false 2>err &&
> + test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
> + test_grep "did you mean .git config set pull\\.rebase false." err
> +'
> +
> +test_expect_success 'set with 1 arg of "key=value": invalid key does not suggest split form' '
> + test_must_fail git config set foo=bar 2>err &&
> + test_grep "missing value to set to a variable with an invalid name .foo=bar." err &&
> + test_grep ! "did you mean" err
> +'
> +
> +test_expect_success 'set with 1 arg: variable name starting with digit is invalid' '
> + test_must_fail git config set foo.1bar=baz 2>err &&
> + test_grep "missing value to set to a variable with an invalid name .foo\\.1bar=baz." err &&
> + test_grep ! "did you mean" err
> +'
> +
> +test_expect_success 'set with 1 arg of valid key reports missing value' '
> + test_must_fail git config set pull.rebase 2>err &&
> + test_grep "missing value to set to the variable .pull\\.rebase." err &&
> + test_grep ! "did you mean" err
> +'
> +
> +test_expect_success 'set with 2 args including "=" in invalid key does not suggest' '
> + test_must_fail git config set pull.rebase=false true 2>err &&
> + test_grep ! "did you mean" err
> +'
> +
> +test_expect_success '"=" inside subsection is valid' '
> + test_when_finished "rm -f subsection.cfg" &&
> + git config set -f subsection.cfg foo.bar=baz.boo qux &&
> + echo qux >expect &&
> + git config get -f subsection.cfg foo.bar=baz.boo >actual &&
> + test_cmp expect actual
> +'
> +
> test_expect_success 'correct key' '
> git config 123456.a123 987
> '
>
> base-commit: 56a4f3c3a221adf1df9b39da69b8a6890f803157
> --
> gitgitgadget
^ permalink raw reply
* [PATCH v4] config: improve diagnostic for "set" with missing value
From: Harald Nordgren via GitGitGadget @ 2026-05-26 19:21 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2302.v3.git.git.1779697995418.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
"git config set pull.rebase=false" currently fails with "wrong
number of arguments", and the implicit form "git config
pull.rebase=false" fails with "invalid key". Neither points at
the real problem: the value is missing.
Report that directly, and when the argument has the shape
"<valid-key>=<value>", also suggest the split form:
$ git config set pull.rebase=false
error: missing value to set to the variable 'pull.rebase=false'
hint: did you mean "git config set pull.rebase false"?
When the prefix before "=" is not a valid key, drop the hint:
$ git config set foo=bar
error: missing value to set to a variable with an invalid name 'foo=bar'
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
config: suggest the correct form when key contains "="
* Skip the hint when the inferred value contains whitespace, so git
config set pull.rebase=false "hello world" no longer suggests a
malformed command.
* Replace the inline actions == 0 check with a named actions_implicit
flag, simplfied the code.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2302%2FHaraldNordgren%2Fconfig-hint-equals-key-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2302/HaraldNordgren/config-hint-equals-key-v4
Pull-Request: https://github.com/git/git/pull/2302
Range-diff vs v3:
1: 6b9d66361d ! 1: 780b99409c config: suggest the correct form when key contains "=" in set context
@@ Metadata
Author: Harald Nordgren <haraldnordgren@gmail.com>
## Commit message ##
- config: suggest the correct form when key contains "=" in set context
+ config: improve diagnostic for "set" with missing value
- A user who types "git config pull.rebase=false" gets only "error:
- invalid key: pull.rebase=false" with no clue what went wrong.
+ "git config set pull.rebase=false" currently fails with "wrong
+ number of arguments", and the implicit form "git config
+ pull.rebase=false" fails with "invalid key". Neither points at
+ the real problem: the value is missing.
- Emit a "did you mean ..." hint suggesting the split form. Restrict it
- to plausible-set contexts ("git config set", bare "git config <key>",
- and their 2-arg forms); explicit "get"/"unset" keep the existing error.
+ Report that directly, and when the argument has the shape
+ "<valid-key>=<value>", also suggest the split form:
- "=" is legal inside a subsection, so only fire when "=" lands after
- the last ".". When the user supplied a separate value, use it in the
- suggestion instead of the suffix after "=":
+ $ git config set pull.rebase=false
+ error: missing value to set to the variable 'pull.rebase=false'
+ hint: did you mean "git config set pull.rebase false"?
- $ git config set pull.rebase=false true
- error: invalid key: pull.rebase=false
- hint: did you mean "git config set pull.rebase true"?
+ When the prefix before "=" is not a valid key, drop the hint:
+
+ $ git config set foo=bar
+ error: missing value to set to a variable with an invalid name 'foo=bar'
- Signed-off-by: Harald Nordgren <harald.nordgren@kostdoktorn.se>
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
## builtin/config.c ##
@@ builtin/config.c: static void check_argc(int argc, int min, int max)
exit(129);
}
-+static void advise_setting_with_equals(const char *key, const char *value)
++static int is_valid_key(const char *key)
+{
+ const char *last_dot = strrchr(key, '.');
-+ const char *eq;
+
-+ if (!last_dot)
-+ return;
-+ eq = strchr(last_dot + 1, '=');
-+ if (!eq)
-+ return;
-+ if (!value)
-+ value = eq + 1;
-+ if (!*value || strpbrk(value, " \t\n"))
-+ return;
-+ advise(_("did you mean \"git config set %.*s %s\"?"),
-+ (int)(eq - key), key, value);
++ return last_dot && isalpha(last_dot[1]);
++}
++
++static NORETURN void die_missing_set_value(const char *arg)
++{
++ const char *last_dot = strrchr(arg, '.');
++ const char *eq = last_dot ? strchr(last_dot + 1, '=') : NULL;
++ char *prefix = eq ? xstrndup(arg, eq - arg) : NULL;
++
++ if (prefix && is_valid_key(prefix)) {
++ error(_("missing value to set to the variable '%s'"), arg);
++ advise(_("did you mean \"git config set %s %s\"?"),
++ prefix, eq + 1);
++ } else if (is_valid_key(arg)) {
++ error(_("missing value to set to the variable '%s'"), arg);
++ } else {
++ error(_("missing value to set to a variable with an invalid name '%s'"),
++ arg);
++ }
++ free(prefix);
++ exit(129);
+}
+
static void show_config_origin(const struct config_display_options *opts,
@@ builtin/config.c: static int cmd_config_set(int argc, const char **argv, const c
argc = parse_options(argc, argv, prefix, opts, builtin_config_set_usage,
PARSE_OPT_STOP_AT_NON_OPTION);
-+ if (argc == 1 && strchr(argv[0], '=')) {
-+ error(_("wrong number of arguments, should be 2"));
-+ advise_setting_with_equals(argv[0], NULL);
-+ exit(129);
-+ }
++ if (argc == 1)
++ die_missing_set_value(argv[0]);
check_argc(argc, 2, 2);
if ((flags & CONFIG_FLAGS_FIXED_VALUE) && !value_pattern)
-@@ builtin/config.c: static int cmd_config_set(int argc, const char **argv, const char *prefix,
- error(_("cannot overwrite multiple values with a single value\n"
- " Use --value=<pattern>, --append or --all to change %s."), argv[0]);
- }
-+ if (ret == CONFIG_INVALID_KEY)
-+ advise_setting_with_equals(argv[0], argv[1]);
-
- location_options_release(&location_opts);
- free(comment);
@@ builtin/config.c: static int cmd_config_actions(int argc, const char **argv, const char *prefix)
};
char *value = NULL, *comment = NULL;
@@ builtin/config.c: static int cmd_config_actions(int argc, const char **argv, con
case 1: actions = ACTION_GET; break;
case 2: actions = ACTION_SET; break;
@@ builtin/config.c: static int cmd_config_actions(int argc, const char **argv, const char *prefix)
- if (ret == CONFIG_NOTHING_SET)
- error(_("cannot overwrite multiple values with a single value\n"
- " Use a regexp, --add or --replace-all to change %s."), argv[0]);
-+ else if (ret == CONFIG_INVALID_KEY)
-+ advise_setting_with_equals(argv[0], argv[1]);
- }
- else if (actions == ACTION_SET_ALL) {
- check_write(&location_opts.source);
-@@ builtin/config.c: static int cmd_config_actions(int argc, const char **argv, const char *prefix)
- check_argc(argc, 1, 2);
- ret = get_value(&location_opts, &display_opts, argv[0], argv[1],
- 0, flags);
-+ if (ret == CONFIG_INVALID_KEY && actions_implicit)
-+ advise_setting_with_equals(argv[0], NULL);
- }
- else if (actions == ACTION_GET_ALL) {
- check_argc(argc, 1, 2);
+ error(_("no action specified"));
+ exit(129);
+ }
++ if (actions_implicit && argc == 1) {
++ const char *last_dot = strrchr(argv[0], '.');
++ if (last_dot && strchr(last_dot + 1, '='))
++ die_missing_set_value(argv[0]);
++ }
+ if (display_opts.omit_values &&
+ !(actions == ACTION_LIST || actions == ACTION_GET_REGEXP)) {
+ error(_("--name-only is only applicable to --list or --get-regexp"));
## t/t1300-config.sh ##
@@ t/t1300-config.sh: test_expect_success 'invalid key' '
test_must_fail git config inval.2key blabla
'
-+test_expect_success 'misplaced "=" in key: bare 1-arg form hints' '
-+ test_must_fail git config pull.rebase=false 2>err &&
-+ test_grep "invalid key: pull\\.rebase=false" err &&
++test_expect_success 'set with 1 arg of "key=value": valid key suggests split form' '
++ test_must_fail git config set pull.rebase=false 2>err &&
++ test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
+ test_grep "did you mean .git config set pull\\.rebase false." err
+'
+
-+test_expect_success 'misplaced "=" in key: bare 2-arg form uses given value' '
-+ test_must_fail git config pull.rebase=false true 2>err &&
-+ test_grep "did you mean .git config set pull\\.rebase true." err
-+'
-+
-+test_expect_success 'misplaced "=" in key: set subcommand uses given value' '
-+ test_must_fail git config set pull.rebase=false true 2>err &&
-+ test_grep "did you mean .git config set pull\\.rebase true." err
-+'
-+
-+test_expect_success 'misplaced "=" in key: set with single arg hints' '
-+ test_must_fail git config set pull.rebase=false 2>err &&
-+ test_grep "wrong number of arguments" err &&
++test_expect_success 'set with 1 arg of "key=value": implicit form suggests split form' '
++ test_must_fail git config pull.rebase=false 2>err &&
++ test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
+ test_grep "did you mean .git config set pull\\.rebase false." err
+'
+
-+test_expect_success 'misplaced "=" in key: explicit --get does not hint' '
-+ test_must_fail git config --get pull.rebase=false 2>err &&
-+ test_grep "invalid key: pull\\.rebase=false" err &&
++test_expect_success 'set with 1 arg of "key=value": invalid key does not suggest split form' '
++ test_must_fail git config set foo=bar 2>err &&
++ test_grep "missing value to set to a variable with an invalid name .foo=bar." err &&
+ test_grep ! "did you mean" err
+'
+
-+test_expect_success 'misplaced "=" in key: get subcommand does not hint' '
-+ test_must_fail git config get pull.rebase=false 2>err &&
++test_expect_success 'set with 1 arg: variable name starting with digit is invalid' '
++ test_must_fail git config set foo.1bar=baz 2>err &&
++ test_grep "missing value to set to a variable with an invalid name .foo\\.1bar=baz." err &&
+ test_grep ! "did you mean" err
+'
+
-+test_expect_success 'misplaced "=" in key: unset subcommand does not hint' '
-+ test_must_fail git config unset pull.rebase=false 2>err &&
++test_expect_success 'set with 1 arg of valid key reports missing value' '
++ test_must_fail git config set pull.rebase 2>err &&
++ test_grep "missing value to set to the variable .pull\\.rebase." err &&
+ test_grep ! "did you mean" err
+'
+
-+test_expect_success 'misplaced "=" in key: value with whitespace skips hint' '
-+ test_must_fail git config set pull.rebase=false "hello world" 2>err &&
-+ test_grep "invalid key: pull\\.rebase=false" err &&
++test_expect_success 'set with 2 args including "=" in invalid key does not suggest' '
++ test_must_fail git config set pull.rebase=false true 2>err &&
+ test_grep ! "did you mean" err
+'
+
-+test_expect_success '"=" inside subsection is valid, no hint' '
++test_expect_success '"=" inside subsection is valid' '
+ test_when_finished "rm -f subsection.cfg" &&
-+ git config set -f subsection.cfg foo.bar=baz.boo qux 2>err &&
-+ test_grep ! "did you mean" err &&
++ git config set -f subsection.cfg foo.bar=baz.boo qux &&
+ echo qux >expect &&
+ git config get -f subsection.cfg foo.bar=baz.boo >actual &&
+ test_cmp expect actual
builtin/config.c | 39 ++++++++++++++++++++++++++++++++++++++-
t/t1300-config.sh | 43 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 81 insertions(+), 1 deletion(-)
diff --git a/builtin/config.c b/builtin/config.c
index cf4ba0f7cc..6fe2d85814 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -1,6 +1,7 @@
#define USE_THE_REPOSITORY_VARIABLE
#include "builtin.h"
#include "abspath.h"
+#include "advice.h"
#include "config.h"
#include "color.h"
#include "date.h"
@@ -210,6 +211,33 @@ static void check_argc(int argc, int min, int max)
exit(129);
}
+static int is_valid_key(const char *key)
+{
+ const char *last_dot = strrchr(key, '.');
+
+ return last_dot && isalpha(last_dot[1]);
+}
+
+static NORETURN void die_missing_set_value(const char *arg)
+{
+ const char *last_dot = strrchr(arg, '.');
+ const char *eq = last_dot ? strchr(last_dot + 1, '=') : NULL;
+ char *prefix = eq ? xstrndup(arg, eq - arg) : NULL;
+
+ if (prefix && is_valid_key(prefix)) {
+ error(_("missing value to set to the variable '%s'"), arg);
+ advise(_("did you mean \"git config set %s %s\"?"),
+ prefix, eq + 1);
+ } else if (is_valid_key(arg)) {
+ error(_("missing value to set to the variable '%s'"), arg);
+ } else {
+ error(_("missing value to set to a variable with an invalid name '%s'"),
+ arg);
+ }
+ free(prefix);
+ exit(129);
+}
+
static void show_config_origin(const struct config_display_options *opts,
const struct key_value_info *kvi,
struct strbuf *buf)
@@ -1133,6 +1161,8 @@ static int cmd_config_set(int argc, const char **argv, const char *prefix,
argc = parse_options(argc, argv, prefix, opts, builtin_config_set_usage,
PARSE_OPT_STOP_AT_NON_OPTION);
+ if (argc == 1)
+ die_missing_set_value(argv[0]);
check_argc(argc, 2, 2);
if ((flags & CONFIG_FLAGS_FIXED_VALUE) && !value_pattern)
@@ -1371,6 +1401,7 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
};
char *value = NULL, *comment = NULL;
int ret = 0;
+ int actions_implicit;
struct key_value_info default_kvi = KVI_INIT;
argc = parse_options(argc, argv, prefix, opts,
@@ -1385,7 +1416,8 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
exit(129);
}
- if (actions == 0)
+ actions_implicit = (actions == 0);
+ if (actions_implicit)
switch (argc) {
case 1: actions = ACTION_GET; break;
case 2: actions = ACTION_SET; break;
@@ -1394,6 +1426,11 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
error(_("no action specified"));
exit(129);
}
+ if (actions_implicit && argc == 1) {
+ const char *last_dot = strrchr(argv[0], '.');
+ if (last_dot && strchr(last_dot + 1, '='))
+ die_missing_set_value(argv[0]);
+ }
if (display_opts.omit_values &&
!(actions == ACTION_LIST || actions == ACTION_GET_REGEXP)) {
error(_("--name-only is only applicable to --list or --get-regexp"));
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 11fc976f3a..4a8a381bd8 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -469,6 +469,49 @@ test_expect_success 'invalid key' '
test_must_fail git config inval.2key blabla
'
+test_expect_success 'set with 1 arg of "key=value": valid key suggests split form' '
+ test_must_fail git config set pull.rebase=false 2>err &&
+ test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
+ test_grep "did you mean .git config set pull\\.rebase false." err
+'
+
+test_expect_success 'set with 1 arg of "key=value": implicit form suggests split form' '
+ test_must_fail git config pull.rebase=false 2>err &&
+ test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
+ test_grep "did you mean .git config set pull\\.rebase false." err
+'
+
+test_expect_success 'set with 1 arg of "key=value": invalid key does not suggest split form' '
+ test_must_fail git config set foo=bar 2>err &&
+ test_grep "missing value to set to a variable with an invalid name .foo=bar." err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success 'set with 1 arg: variable name starting with digit is invalid' '
+ test_must_fail git config set foo.1bar=baz 2>err &&
+ test_grep "missing value to set to a variable with an invalid name .foo\\.1bar=baz." err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success 'set with 1 arg of valid key reports missing value' '
+ test_must_fail git config set pull.rebase 2>err &&
+ test_grep "missing value to set to the variable .pull\\.rebase." err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success 'set with 2 args including "=" in invalid key does not suggest' '
+ test_must_fail git config set pull.rebase=false true 2>err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success '"=" inside subsection is valid' '
+ test_when_finished "rm -f subsection.cfg" &&
+ git config set -f subsection.cfg foo.bar=baz.boo qux &&
+ echo qux >expect &&
+ git config get -f subsection.cfg foo.bar=baz.boo >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'correct key' '
git config 123456.a123 987
'
base-commit: 56a4f3c3a221adf1df9b39da69b8a6890f803157
--
gitgitgadget
^ permalink raw reply related
* Re: git mv after the fact
From: Frieder Hannenheim @ 2026-05-26 16:50 UTC (permalink / raw)
To: Chris Torek; +Cc: git
In-Reply-To: <CAPx1Gvd9+z0th9whCbcA60_bWproPp+kwp3qDmhQOe4G=0=E6A@mail.gmail.com>
In my particular use case I changed a patch to be a git patch with a
commit message and different filename so the move was not discovered
automatically. But I'm not sure if I staged the files so maybe it would
have been discovered.
Frieder
On 26.05.26 18:40, Chris Torek wrote:
> On Tue, May 26, 2026 at 6:18 AM Frieder Hannenheim <mail@fhannenheim.net> wrote:
>> I'd like to propose a new flag for git mv, that updates the index
>> like git mv normally would but does not move the file. ...
> You may already know this, but technically no flag is needed:
> you can just "git add" the new name and "git rm" the old one,
> with the same effect.
>
> A flag for "git mv" would be convenient (and slightly more
> efficient, not in terms of storage but in terms of CPU time
> spent discovering that the contents under the new name
> already exist in the object database). But Git will discover
> the rename on its own in the usual way regardless of how
> you get to that point.
>
> Chris
^ permalink raw reply
* [PATCH 5/5] git-son: add tests
From: Evan Haque via GitGitGadget @ 2026-05-26 16:47 UTC (permalink / raw)
To: git; +Cc: Evan Haque, Evan Haque
In-Reply-To: <pull.2122.git.1779814052.gitgitgadget@gmail.com>
From: Evan Haque <evanhaque1@gmail.com>
Add t5151-son.sh with nine test cases covering:
- basic child repository creation
- parent remote configuration in the child
- .gitignore update in the parent
- initial commit presence in the child
- failure when the target directory already exists
- --branch without --inherit is rejected cleanly
- no leftover directory on validation failure
- --inherit fetches parent history
Register the test in t/meson.build so the meson build system
discovers and runs it.
Assisted-by: Claude Opus 4.6
Signed-off-by: Evan Haque <evanhaque1@gmail.com>
---
t/meson.build | 1 +
t/t5151-son.sh | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 64 insertions(+)
create mode 100755 t/t5151-son.sh
diff --git a/t/meson.build b/t/meson.build
index fd955f44ef..523062df66 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -591,6 +591,7 @@ integration_tests = [
't5004-archive-corner-cases.sh',
't5100-mailinfo.sh',
't5150-request-pull.sh',
+ 't5151-son.sh',
't5200-update-server-info.sh',
't5300-pack-object.sh',
't5301-sliding-window.sh',
diff --git a/t/t5151-son.sh b/t/t5151-son.sh
new file mode 100755
index 0000000000..826cbbfa66
--- /dev/null
+++ b/t/t5151-son.sh
@@ -0,0 +1,63 @@
+#!/bin/sh
+
+test_description='Test git son command.'
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
+. ./test-lib.sh
+
+test_expect_success 'setup parent repository' '
+ echo "parent content" >file.txt &&
+ git add file.txt &&
+ git commit -m "Initial parent commit"
+'
+
+test_expect_success 'son creates child repository' '
+ git son my-child &&
+ test -d my-child &&
+ test -d my-child/.git
+'
+
+test_expect_success 'son sets parent remote in child' '
+ (
+ cd my-child &&
+ git remote get-url parent
+ )
+'
+
+test_expect_success 'son adds child to parent .gitignore' '
+ grep "my-child/" .gitignore
+'
+
+test_expect_success 'son child has initial commit' '
+ (
+ cd my-child &&
+ test $(git log --oneline | wc -l) -eq 1
+ )
+'
+
+test_expect_success 'son fails if target already exists' '
+ test_must_fail git son my-child
+'
+
+test_expect_success 'son with --branch requires --inherit' '
+ test_must_fail git son --branch main branch-child
+'
+
+test_expect_success 'son with --branch leaves no directory on failure' '
+ ! test -e branch-child
+'
+
+test_expect_success 'son with --inherit fetches parent history' '
+ git init --bare "$TRASH_DIRECTORY/parent.git" &&
+ git push "$TRASH_DIRECTORY/parent.git" main &&
+ git remote add origin "file://$TRASH_DIRECTORY/parent.git" &&
+ git son --inherit inherited-child &&
+ (
+ cd inherited-child &&
+ git log --oneline parent/main
+ )
+'
+
+test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH 4/5] git-son: add documentation
From: Evan Haque via GitGitGadget @ 2026-05-26 16:47 UTC (permalink / raw)
To: git; +Cc: Evan Haque, Evan Haque
In-Reply-To: <pull.2122.git.1779814052.gitgitgadget@gmail.com>
From: Evan Haque <evanhaque1@gmail.com>
Add a man page (git-son.adoc) documenting the synopsis, options, and
usage examples for the new command. Register the page in
Documentation/meson.build so it is built by the meson doc target.
Assisted-by: Claude Opus 4.6
Signed-off-by: Evan Haque <evanhaque1@gmail.com>
---
Documentation/git-son.adoc | 64 ++++++++++++++++++++++++++++++++++++++
Documentation/meson.build | 1 +
2 files changed, 65 insertions(+)
create mode 100644 Documentation/git-son.adoc
diff --git a/Documentation/git-son.adoc b/Documentation/git-son.adoc
new file mode 100644
index 0000000000..17ec992bfd
--- /dev/null
+++ b/Documentation/git-son.adoc
@@ -0,0 +1,64 @@
+git-son(1)
+==========
+
+NAME
+----
+git-son - Create an independent child repository that knows its parent
+
+SYNOPSIS
+--------
+[verse]
+'git son' [--inherit] [--branch <branch>] <name>
+
+DESCRIPTION
+-----------
+
+Create a new independent Git repository inside the current working
+tree as a subdirectory named `<name>`. Unlike a submodule, the child
+repository is not tracked by the parent; instead, `<name>/` is added
+to the parent's `.gitignore`.
+
+The child repository is configured with a remote called `parent`
+pointing back to the parent repository's origin URL (or local path
+if no origin is set), allowing the child to fetch from the parent
+at any time.
+
+OPTIONS
+-------
+--inherit::
+ Fetch the parent's history into the child repository at
+ creation time. Without this flag, the child starts with a
+ single initial commit.
+
+--branch <branch>::
+ When used with `--inherit`, check out the given branch from
+ the parent instead of the default branch. This option
+ requires `--inherit`.
+
+<name>::
+ The name of the subdirectory (and child repository) to create.
+ Must not already exist.
+
+EXAMPLES
+--------
+
+Create a simple child repository:
+
+ git son my-tool
+
+Create a child that inherits the parent's history:
+
+ git son --inherit my-fork
+
+Create a child starting from a specific parent branch:
+
+ git son --inherit --branch feature my-experiment
+
+Later, from within the child, fetch updates from the parent:
+
+ cd my-tool
+ git fetch parent
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Documentation/meson.build b/Documentation/meson.build
index f4854f802d..1ae7e5f644 100644
--- a/Documentation/meson.build
+++ b/Documentation/meson.build
@@ -139,6 +139,7 @@ manpages = {
'git-show-ref.adoc' : 1,
'git-show.adoc' : 1,
'git-sh-setup.adoc' : 1,
+ 'git-son.adoc' : 1,
'git-sparse-checkout.adoc' : 1,
'git-stage.adoc' : 1,
'git-stash.adoc' : 1,
--
gitgitgadget
^ permalink raw reply related
* [PATCH 3/5] git-son: add to command list as mainporcelain
From: Evan Haque via GitGitGadget @ 2026-05-26 16:47 UTC (permalink / raw)
To: git; +Cc: Evan Haque, Evan Haque
In-Reply-To: <pull.2122.git.1779814052.gitgitgadget@gmail.com>
From: Evan Haque <evanhaque1@gmail.com>
Register git-son in command-list.txt as a mainporcelain command so
that it appears in "git help" output and is discoverable through the
standard help machinery.
Assisted-by: Claude Opus 4.6
Signed-off-by: Evan Haque <evanhaque1@gmail.com>
---
command-list.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/command-list.txt b/command-list.txt
index 21b802c420..880177e0fd 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -186,6 +186,7 @@ git-show mainporcelain info
git-show-branch ancillaryinterrogators complete
git-show-index plumbinginterrogators
git-show-ref plumbinginterrogators
+git-son mainporcelain
git-sparse-checkout mainporcelain
git-stage complete
git-stash mainporcelain
--
gitgitgadget
^ permalink raw reply related
* [PATCH 2/5] git-son: register in Makefile and meson build system
From: Evan Haque via GitGitGadget @ 2026-05-26 16:47 UTC (permalink / raw)
To: git; +Cc: Evan Haque, Evan Haque
In-Reply-To: <pull.2122.git.1779814052.gitgitgadget@gmail.com>
From: Evan Haque <evanhaque1@gmail.com>
Add git-son.sh to SCRIPT_SH in the Makefile and to the scripts_sh
array in meson.build so that the script is installed alongside the
other shell-based Git commands during "make install" and meson builds.
Also add /git-son to .gitignore so that the build artifact produced
from git-son.sh is not flagged as an untracked file.
Assisted-by: Claude Opus 4.6
Signed-off-by: Evan Haque <evanhaque1@gmail.com>
---
.gitignore | 1 +
Makefile | 1 +
meson.build | 1 +
3 files changed, 3 insertions(+)
diff --git a/.gitignore b/.gitignore
index 4da58c6754..5f329179c8 100644
--- a/.gitignore
+++ b/.gitignore
@@ -164,6 +164,7 @@
/git-show-branch
/git-show-index
/git-show-ref
+/git-son
/git-sparse-checkout
/git-stage
/git-stash
diff --git a/Makefile b/Makefile
index fb50c57e4f..4791f47af1 100644
--- a/Makefile
+++ b/Makefile
@@ -728,6 +728,7 @@ SCRIPT_SH += git-merge-resolve.sh
SCRIPT_SH += git-mergetool.sh
SCRIPT_SH += git-quiltimport.sh
SCRIPT_SH += git-request-pull.sh
+SCRIPT_SH += git-son.sh
SCRIPT_SH += git-submodule.sh
SCRIPT_SH += git-web--browse.sh
diff --git a/meson.build b/meson.build
index 052c81f288..538bd4025f 100644
--- a/meson.build
+++ b/meson.build
@@ -1973,6 +1973,7 @@ scripts_sh = [
'git-mergetool.sh',
'git-quiltimport.sh',
'git-request-pull.sh',
+ 'git-son.sh',
'git-sh-i18n.sh',
'git-sh-setup.sh',
'git-submodule.sh',
--
gitgitgadget
^ permalink raw reply related
* [PATCH 1/5] git-son: add command to create independent child repositories
From: Evan Haque via GitGitGadget @ 2026-05-26 16:47 UTC (permalink / raw)
To: git; +Cc: Evan Haque, Evan Haque
In-Reply-To: <pull.2122.git.1779814052.gitgitgadget@gmail.com>
From: Evan Haque <evanhaque1@gmail.com>
Introduce git-son, a new porcelain command that creates an independent
child repository inside the current working tree. Unlike submodules,
the child is not tracked by the parent; instead its directory is added
to the parent's .gitignore and a "parent" remote is configured in the
child pointing back to the parent's origin URL or local path.
This gives users a lightweight way to spin off a related repository
that knows where it came from without the coupling that submodules
impose.
The command supports two optional flags:
--inherit fetch the parent's history into the child at creation
--branch check out a specific parent branch (requires --inherit)
Assisted-by: Claude Opus 4.6
Signed-off-by: Evan Haque <evanhaque1@gmail.com>
---
git-son.sh | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 97 insertions(+)
create mode 100755 git-son.sh
diff --git a/git-son.sh b/git-son.sh
new file mode 100755
index 0000000000..a212c7b69f
--- /dev/null
+++ b/git-son.sh
@@ -0,0 +1,97 @@
+#!/bin/sh
+#
+# git-son: create an independent child repository that knows its parent
+#
+
+SUBDIRECTORY_OK='Yes'
+OPTIONS_SPEC='git son [options] <name>
+--
+inherit fetch parent history into the son
+branch= start the son from a specific parent branch
+'
+
+. git-sh-setup
+require_work_tree
+cd_to_toplevel
+
+inherit=
+branch=
+while test $# -gt 0
+do
+ case "$1" in
+ --inherit)
+ inherit=1 ;;
+ --branch)
+ shift
+ branch="$1" ;;
+ --)
+ shift; break ;;
+ -*)
+ usage ;;
+ *)
+ break ;;
+ esac
+ shift
+done
+
+name="$1"
+test -n "$name" || usage
+
+if test -n "$branch" && test -z "$inherit"
+then
+ die "fatal: --branch requires --inherit"
+fi
+
+parent_dir="$(pwd)"
+parent_remote="$(git remote get-url origin 2>/dev/null)" || parent_remote=
+
+if test -e "$name"
+then
+ die "fatal: '$name' already exists"
+fi
+
+mkdir "$name" || die "fatal: could not create directory '$name'"
+
+if ! echo "$name/" >> "$parent_dir/.gitignore" 2>/dev/null
+then
+ rm -rf "$name"
+ die "fatal: could not update .gitignore"
+fi
+
+cd "$name" || die "fatal: could not enter directory '$name'"
+
+if ! git init
+then
+ rm -rf "$parent_dir/$name"
+ die "fatal: could not initialize repository in '$name'"
+fi
+
+if test -n "$parent_remote"
+then
+ git remote add parent "$parent_remote"
+else
+ git remote add parent "$parent_dir"
+fi
+
+if test -n "$inherit"
+then
+ git fetch parent || die "fatal: could not fetch from parent"
+ if test -n "$branch"
+ then
+ git checkout -b "$branch" "parent/$branch" ||
+ die "fatal: could not checkout branch '$branch'"
+ else
+ git checkout -b main parent/HEAD 2>/dev/null ||
+ git checkout -b main "parent/$(git remote show parent | sed -n 's/.*HEAD branch: //p')" 2>/dev/null ||
+ echo "warning: could not determine parent HEAD, starting empty"
+ fi
+else
+ echo "# $name" > README.md
+ git add README.md
+ git commit -q -m "Initial commit"
+fi
+
+echo ""
+echo "Created son repository '$name'"
+echo " parent: ${parent_remote:-$parent_dir}"
+echo " inherit: ${inherit:-no}"
--
gitgitgadget
^ permalink raw reply related
* [PATCH 0/5] git son: add command to create independent child repositories
From: Evan Haque via GitGitGadget @ 2026-05-26 16:47 UTC (permalink / raw)
To: git; +Cc: Evan Haque
Motivation
==========
When spinning off a new project that is related to an existing repository,
there is no built-in way to create a child repository that maintains a link
back to its parent without the tight coupling of submodules. Submodules pin
the child to a specific commit and require the parent to track the child in
its index, which is too heavyweight when the child is meant to be fully
independent.
The typical workflow today is manual: git init, git remote add, update
.gitignore — three steps that are easy to forget or get wrong. git son
automates this and establishes a lightweight convention for the parent-child
relationship: a remote named parent in the child, and nothing in the parent
except an ignore rule.
Summary
=======
This series introduces git son, a new porcelain command that creates an
independent child repository inside the current working tree. Unlike a
submodule, the child is not tracked by the parent; instead, its subdirectory
is added to the parent's .gitignore. The child is configured at creation
time with a remote named parent pointing back to the parent repository's
origin URL (or local path if no origin exists), making the lineage explicit
and recoverable via standard Git commands.
Changes
=======
* git-son.sh: new shell script implementing the command, supporting
--inherit to fetch parent history at creation time and --branch <branch>
(requires --inherit) to check out a specific parent branch
* git-son registered in command-list.txt as mainporcelain
* git-son.sh added to SCRIPT_SH in Makefile and to scripts_sh in
meson.build
* Documentation/git-son.adoc: new man page covering synopsis, option
descriptions, and worked examples
* Documentation/meson.build: git-son.adoc added to the manpage build list
* t/t5151-son.sh: new test script covering basic creation, parent remote
configuration, .gitignore update, idempotency failure, flag validation,
and --inherit with a bare remote
Details
=======
The key design property is independence: the child is a fully self-contained
repository with no entry in the parent's index. The parent remote is the
only artifact linking the two, which means git fetch parent and git log
parent/<branch> work as expected from within the child without any special
tooling.
The --branch flag is intentionally restricted to --inherit mode. Without
fetching, there is no remote-tracking branch to check out from, so accepting
--branch alone would be misleading; the command dies with a clear diagnostic
in that case.
When no origin URL is available in the parent, the parent remote is set to
the parent's absolute local path. This covers the common case of
repositories that have never been pushed to a remote.
Testing
=======
t/t5151-son.sh covers the following scenarios:
* Basic child repository creation (directory exists, .git present)
* parent remote is correctly recorded in the child
* Child directory is appended to the parent's .gitignore
* Child starts with exactly one initial commit when --inherit is not used
* Command fails without leaving a directory when the target already exists
* --branch without --inherit is rejected before any filesystem changes
* --inherit fetches parent history and the remote-tracking branch is
reachable in the child
Evan Haque (5):
git-son: add command to create independent child repositories
git-son: register in Makefile and meson build system
git-son: add to command list as mainporcelain
git-son: add documentation
git-son: add tests
.gitignore | 1 +
Documentation/git-son.adoc | 64 +++++++++++++++++++++++++
Documentation/meson.build | 1 +
Makefile | 1 +
command-list.txt | 1 +
git-son.sh | 97 ++++++++++++++++++++++++++++++++++++++
meson.build | 1 +
t/meson.build | 1 +
t/t5151-son.sh | 63 +++++++++++++++++++++++++
9 files changed, 230 insertions(+)
create mode 100644 Documentation/git-son.adoc
create mode 100755 git-son.sh
create mode 100755 t/t5151-son.sh
base-commit: aec3f587505a472db67e9462d0702e7d463a449d
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2122%2FEvandabest%2Fgit-son-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2122/Evandabest/git-son-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2122
--
gitgitgadget
^ permalink raw reply
* Re: git mv after the fact
From: Chris Torek @ 2026-05-26 16:40 UTC (permalink / raw)
To: Frieder Hannenheim; +Cc: git
In-Reply-To: <02663c67-01ad-4dd1-aae6-9e9706f3d040@fhannenheim.net>
On Tue, May 26, 2026 at 6:18 AM Frieder Hannenheim <mail@fhannenheim.net> wrote:
> I'd like to propose a new flag for git mv, that updates the index
> like git mv normally would but does not move the file. ...
You may already know this, but technically no flag is needed:
you can just "git add" the new name and "git rm" the old one,
with the same effect.
A flag for "git mv" would be convenient (and slightly more
efficient, not in terms of storage but in terms of CPU time
spent discovering that the contents under the new name
already exist in the object database). But Git will discover
the rename on its own in the usual way regardless of how
you get to that point.
Chris
^ 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