# An annoying "Bug" that we would probably leave as-is 14 messages from 2026-03-06 to 2026-03-11. Participants: Junio C Hamano, Tian Yuchen, D. Ben Knoble, Oswald Buddenhagen, Jean-Noël AVILA. Thread: https://gitlist.dev/t/65158 ## Junio C Hamano, 2026-03-06 23:10 Subject: An annoying "Bug" that we would probably leave as-is Message-ID: URL: https://gitlist.dev/e/xmqqh5qswo45.fsf%40gitster.g ``` "git show -U" does not complain. In an ideal world, it should say "-U wants a number", just like "git show -Unan" does. Unfortunately t/t4013/ actually has tests that break if we start tightening the command line parser for this. This falls into the "if it hurts, do not do it" category that somebody might be taking advantage of out there that we might be better off leaving them broken. diff.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git c/diff.c w/diff.c index a1961526c0..ff79a41cfc 100644 --- c/diff.c +++ w/diff.c @@ -5581,15 +5581,14 @@ static int diff_opt_unified(const struct option *opt, const char *arg, int unset) { struct diff_options *options = opt->value; - char *s; + char *s = NULL; BUG_ON_OPT_NEG(unset); - if (arg) { + if (arg) options->context = strtol(arg, &s, 10); - if (*s) - return error(_("%s expects a numerical value"), "--unified"); - } + if (!s || *s) + return error(_("%s expects a numerical value"), "--unified"); enable_patch_output(&options->output_format); return 0; ``` ## Tian Yuchen, 2026-03-09 17:10 Subject: Re: An annoying "Bug" that we would probably leave as-is Message-ID: <8d0ca4db-3745-49fd-8373-2d3d71aeea78@gmail.com> URL: https://gitlist.dev/e/8d0ca4db-3745-49fd-8373-2d3d71aeea78%40gmail.com In-Reply-To: ``` Hi Junio, On 3/7/26 07:10, Junio C Hamano wrote: > "git show -U" does not complain. In an ideal world, it should say > "-U wants a number", just like "git show -Unan" does. > > Unfortunately t/t4013/ actually has tests that break if we start > tightening the command line parser for this. > > This falls into the "if it hurts, do not do it" category that > somebody might be taking advantage of out there that we might be > better off leaving them broken. Out of curiosity, I applied your patch and ran t4013 to see exactly what would break. It turns out it's 'not ok 157 - git diff -U initial..side'. The suite explicitly relies on 'git diff -U' (without a number) to implicitly default to 3 lines of context, behaving identically to '-u'. With your patch, it correctly but fatally throws 'error: --unified expects a numerical value' instead of generating the diff, causing the test to fail. I looked at diff-context-options.adoc, and it says: `-U`:: `--unified=`:: Generate diffs with __ lines of context. Defaults to `diff.context` or 3 if the config option is unset. I believe this description needs revision, as in actual practice (as you described), the number following '-U' is not strictly required. (I was taken aback, especially when you mentioned “somebody might be taking advantage of out there.” It made me realize how an inadvertent little bug or minor error can become a daily habit for many people, or even foundations of millions of scripts. Just a thought. ;) Regards, Yuchen ``` ## Tian Yuchen, 2026-03-09 17:27 Subject: [PATCH v1] diff: document -U without as using default context Message-ID: <20260309172719.125419-1-cat@malon.dev> URL: https://gitlist.dev/e/20260309172719.125419-1-cat%40malon.dev In-Reply-To: ``` The documentation for '-U' implies that the numeric value '' is mandatory. However, the command line parser has historically accepted '-U' without a number. Strictly requiring a number for '-U' would break existing tests (e.g., in 't4013') and likely disrupt user scripts relying on this undocumented behavior. Since we are retaining this fallback behavior for backward compatibility, update the documentation to explicitly state that '' can be omitted for the short option '-U'. Signed-off-by: Tian Yuchen --- Documentation/diff-context-options.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/diff-context-options.adoc b/Documentation/diff-context-options.adoc index e161260358..655496ec3a 100644 --- a/Documentation/diff-context-options.adoc +++ b/Documentation/diff-context-options.adoc @@ -1,4 +1,4 @@ -`-U`:: +`-U[]`:: `--unified=`:: Generate diffs with __ lines of context. Defaults to `diff.context` or 3 if the config option is unset. -- 2.43.0 ``` ## D. Ben Knoble, 2026-03-09 22:00 Subject: Re: [PATCH v1] diff: document -U without as using default context Message-ID: URL: https://gitlist.dev/e/CALnO6CAJfton4KwywVeRFbynWH9zu4nY4c6uXOnEdddKtZpeqA%40mail.gmail.com In-Reply-To: <20260309172719.125419-1-cat@malon.dev> ``` On Mon, Mar 9, 2026 at 1:28 PM Tian Yuchen wrote: > > The documentation for '-U' implies that the numeric value '' is > mandatory. However, the command line parser has historically accepted > '-U' without a number. > > Strictly requiring a number for '-U' would break existing tests > (e.g., in 't4013') and likely disrupt user scripts relying on this > undocumented behavior. > > Since we are retaining this fallback behavior for backward compatibility, > update the documentation to explicitly state that '' can be omitted > for the short option '-U'. > > Signed-off-by: Tian Yuchen > --- > Documentation/diff-context-options.adoc | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/diff-context-options.adoc b/Documentation/diff-context-options.adoc > index e161260358..655496ec3a 100644 > --- a/Documentation/diff-context-options.adoc > +++ b/Documentation/diff-context-options.adoc > @@ -1,4 +1,4 @@ > -`-U`:: > +`-U[]`:: > `--unified=`:: > Generate diffs with __ lines of context. Defaults to `diff.context` > or 3 if the config option is unset. > -- > 2.43.0 I was curious about the way we indicate this kind of optionality for single-letter options, so: git grep -e '-[[:alnum:]]\[' Documentation which finds many hits of this pattern. Cool. Adding -A1, we see that it is also common to document the long-form like --unified[=] which you may want to add to this patch. (I haven't really considered the rest of it very well, although it does seem worth updating the syntax to match what some of our tests exercise.) Which makes me notice: 3 is the default if the config option is unset _or_ if is not provided. Is there a better wording to indicate that? Maybe the simplest tweak is to clarify that is the thing which defaults to… (since a first read might leave the reader wondering "what defaults to diff.context or 3? ah, it's probably n…"). But a glance at other docs makes this pattern seem common while some do say "if is specified…", so idk. BTW I noticed these docs are duplicated between Documentation/diff-{,context-}options.adoc -- D. Ben Knoble ``` ## Junio C Hamano, 2026-03-09 22:17 Subject: Re: [PATCH v1] diff: document -U without as using default context Message-ID: URL: https://gitlist.dev/e/xmqqwlzk1wd8.fsf%40gitster.g In-Reply-To: <20260309172719.125419-1-cat@malon.dev> ``` Tian Yuchen writes: > The documentation for '-U' implies that the numeric value '' is > mandatory. However, the command line parser has historically accepted > '-U' without a number. > > Strictly requiring a number for '-U' would break existing tests > (e.g., in 't4013') and likely disrupt user scripts relying on this > undocumented behavior. > > Since we are retaining this fallback behavior for backward compatibility, > update the documentation to explicitly state that '' can be omitted > for the short option '-U'. > > Signed-off-by: Tian Yuchen > --- > Documentation/diff-context-options.adoc | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) I am moderately nagative. It is not like we are _encouraging_ users to omit from -U, but it is not errored out only due to a bug. Who would the new text help? Users would wonder why is not optional in --unified=, the other way to spell the same thing. If we want to be explicit, we should probably do this instead: `-U`:: `--unified=`:: Generate diffs with __ lines of context. Defaults to `diff.context` or 3 if the config option is unset (`-U` without '' is accepted as a silent synonym for `-p` due to a historical accident). which would tell readers what happens when '' is omitted and why we allow such an inconsistency. ``` ## Tian Yuchen, 2026-03-10 04:51 Subject: Re: [PATCH v1] diff: document -U without as using default context Message-ID: <71287f25-eb60-4a45-b4ac-9368053c9183@malon.dev> URL: https://gitlist.dev/e/71287f25-eb60-4a45-b4ac-9368053c9183%40malon.dev In-Reply-To: ``` Hi Junio, > I am moderately nagative. > > It is not like we are _encouraging_ users to omit from -U, > but it is not errored out only due to a bug. Who would the new text > help? Users would wonder why is not optional in --unified=, > the other way to spell the same thing. Indeed. What I was actually thinking earlier was: for bugs like this, which most likely come from misoperations, typos, or inconsistencies from *ancient* standards, should we acknowledge them as “yes it is a bug/historical issue” or “*special behavior* we defined”? I'm not as familiar with the Git codebase as you are, so I couldn't make a definitive call. I chose the second approach, but I now agree the first makes more sense. In other words, users shouldn't be *encouraged* to engage in such behavior, right? > If we want to be explicit, we should probably do this instead: > > `-U`:: > `--unified=`:: > Generate diffs with __ lines of context. Defaults to `diff.context` > or 3 if the config option is unset (`-U` without '' is accepted > as a silent synonym for `-p` due to a historical accident). > > which would tell readers what happens when '' is omitted and why > we allow such an inconsistency. That makes sense. I'll make the changes accordingly. Regards, Yuchen ``` ## Tian Yuchen, 2026-03-10 04:55 Subject: Re: [PATCH v1] diff: document -U without as using default context Message-ID: URL: https://gitlist.dev/e/fcc68ecb-09b1-4063-bcdf-5052eb42dfcf%40malon.dev In-Reply-To: ``` Hi Ben, On 3/10/26 06:00, D. Ben Knoble wrote: > Which makes me notice: 3 is the default if the config option is unset > _or_ if is not provided. Is there a better wording to indicate > that? Maybe the simplest tweak is to clarify that is the thing > which defaults to… (since a first read might leave the reader > wondering "what defaults to diff.context or 3? ah, it's probably n…"). > But a glance at other docs makes this pattern seem common while some > do say "if is specified…", so idk. Indeed, the current wording isn't clear enough. I'll make corrections wherever it's needed. I think Junio provides a good template that's great with just a few tweaks ;) Thanks for the review! Yuchen ``` ## Tian Yuchen, 2026-03-10 05:30 Subject: [PATCH v2] diff: document -U without as using default context Message-ID: <20260310053032.129327-1-cat@malon.dev> URL: https://gitlist.dev/e/20260310053032.129327-1-cat%40malon.dev In-Reply-To: <20260309172719.125419-1-cat@malon.dev> ``` The documentation for '-U' implies that the numeric value '' is mandatory. However, the command line parser has historically accepted '-U' without a number. Strictly requiring a number for '-U' would break existing tests (e.g., in 't4013') and likely disrupt user scripts relying on this undocumented behavior. Since we are retaining this fallback behavior for backward compatibility, update the documentation to explicitly state that '' can be omitted for the short option '-U'. Signed-off-by: Tian Yuchen --- Documentation/diff-context-options.adoc | 6 ++++-- Documentation/diff-options.adoc | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/Documentation/diff-context-options.adoc b/Documentation/diff-context-options.adoc index e161260358..42d29a0c00 100644 --- a/Documentation/diff-context-options.adoc +++ b/Documentation/diff-context-options.adoc @@ -1,7 +1,9 @@ `-U`:: `--unified=`:: - Generate diffs with __ lines of context. Defaults to `diff.context` - or 3 if the config option is unset. + Generate diffs with __ lines of context. The number of context + lines defaults to `diff.context` or 3 if the config option is unset. + (`-U` without `` is accepted as a silent synonym for `-p` due + to a historical accident). `--inter-hunk-context=`:: Show the context between diff hunks, up to the specified __ diff --git a/Documentation/diff-options.adoc b/Documentation/diff-options.adoc index 9cdad6f72a..40502639a2 100644 --- a/Documentation/diff-options.adoc +++ b/Documentation/diff-options.adoc @@ -127,8 +127,10 @@ endif::git-log[] `-U`:: `--unified=`:: - Generate diffs with __ lines of context instead of - the usual three. + Generate diffs with __ lines of context. The number of context + lines defaults to `diff.context` or 3 if the config option is unset. + (`-U` without `` is accepted as a silent synonym for `-p` due + to a historical accident). ifndef::git-format-patch[] Implies `--patch`. endif::git-format-patch[] -- 2.43.0 ``` ## Oswald Buddenhagen, 2026-03-10 09:15 Subject: Re: [PATCH v2] diff: document -U without as using default context Message-ID: URL: https://gitlist.dev/e/aa_hTifKYZ5KYREv%40ugly.lan In-Reply-To: <20260310053032.129327-1-cat@malon.dev> ``` On Tue, Mar 10, 2026 at 01:30:32PM +0800, Tian Yuchen wrote: >The documentation for '-U' implies that the numeric value '' is >mandatory. However, the command line parser has historically accepted >'-U' without a number. > >Strictly requiring a number for '-U' would break existing tests >(e.g., in 't4013') and likely disrupt user scripts relying on this >undocumented behavior. >Since we are retaining this fallback behavior for backward compatibility, >update the documentation to explicitly state that '' can be omitted >for the short option '-U'. > i'd replace that with: Hence we retain this fallback behavior for backward compatibility, but document it as such. >+ (`-U` without `` is accepted as a silent synonym for `-p` due >+ to a historical accident). > "silently accepted as a synonym" would be much more natural. ``` ## Tian Yuchen, 2026-03-10 09:43 Subject: Re: [PATCH v2] diff: document -U without as using default context Message-ID: <736d3c7a-49cf-4ed5-9686-5a653caa20a4@malon.dev> URL: https://gitlist.dev/e/736d3c7a-49cf-4ed5-9686-5a653caa20a4%40malon.dev In-Reply-To: ``` On 3/10/26 17:15, Oswald Buddenhagen wrote: > i'd replace that with: > > Hence we retain this fallback behavior for backward compatibility, but > document it as such. > >> +    (`-U` without `` is accepted as a silent synonym for `-p` due >> +    to a historical accident). >> > "silently accepted as a synonym" would be much more natural. Thank you for pointing out. This change does make the message read more smoothly. Will send v3 shortly. Yuchen ``` ## Tian Yuchen, 2026-03-10 09:50 Subject: [PATCH v3] diff: document -U without as using default context Message-ID: <20260310095017.131957-1-cat@malon.dev> URL: https://gitlist.dev/e/20260310095017.131957-1-cat%40malon.dev In-Reply-To: <20260309172719.125419-1-cat@malon.dev> ``` The documentation for '-U' implies that the numeric value '' is mandatory. However, the command line parser has historically accepted '-U' without a number. Strictly requiring a number for '-U' would break existing tests (e.g., in 't4013') and likely disrupt user scripts relying on this undocumented behavior. Hence we retain this fallback behavior for backward compatibility, but document it as such. Signed-off-by: Tian Yuchen --- Documentation/diff-context-options.adoc | 6 ++++-- Documentation/diff-options.adoc | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/Documentation/diff-context-options.adoc b/Documentation/diff-context-options.adoc index e161260358..306abfd9ab 100644 --- a/Documentation/diff-context-options.adoc +++ b/Documentation/diff-context-options.adoc @@ -1,7 +1,9 @@ `-U`:: `--unified=`:: - Generate diffs with __ lines of context. Defaults to `diff.context` - or 3 if the config option is unset. + Generate diffs with __ lines of context. The number of context + lines defaults to `diff.context` or 3 if the config option is unset. + (`-U` without `` is silently accepted as a synonym for `-p` due + to a historical accident). `--inter-hunk-context=`:: Show the context between diff hunks, up to the specified __ diff --git a/Documentation/diff-options.adoc b/Documentation/diff-options.adoc index 9cdad6f72a..e1f20bedf6 100644 --- a/Documentation/diff-options.adoc +++ b/Documentation/diff-options.adoc @@ -127,8 +127,10 @@ endif::git-log[] `-U`:: `--unified=`:: - Generate diffs with __ lines of context instead of - the usual three. + Generate diffs with __ lines of context. The number of context + lines defaults to `diff.context` or 3 if the config option is unset. + (`-U` without `` is silently accepted as a synonym for `-p` due + to a historical accident). ifndef::git-format-patch[] Implies `--patch`. endif::git-format-patch[] -- 2.43.0 ``` ## Junio C Hamano, 2026-03-10 13:14 Subject: Re: [PATCH v2] diff: document -U without as using default context Message-ID: URL: https://gitlist.dev/e/xmqqfr67yggf.fsf%40gitster.g In-Reply-To: ``` Oswald Buddenhagen writes: >>Since we are retaining this fallback behavior for backward compatibility, >>update the documentation to explicitly state that '' can be omitted >>for the short option '-U'. >> > i'd replace that with: > > Hence we retain this fallback behavior for backward compatibility, but > document it as such. > >>+ (`-U` without `` is accepted as a silent synonym for `-p` due >>+ to a historical accident). >> > "silently accepted as a synonym" would be much more natural. An excellent suggestion. Thanks. ``` ## Jean-Noël AVILA, 2026-03-10 17:31 Subject: Re: [PATCH v1] diff: document -U without as using default context Message-ID: <5973423.DvuYhMxLoT@piment-oiseau> URL: https://gitlist.dev/e/5973423.DvuYhMxLoT%40piment-oiseau In-Reply-To: <20260309172719.125419-1-cat@malon.dev> ``` On Monday, 9 March 2026 18:27:19 CET Tian Yuchen wrote: > The documentation for '-U' implies that the numeric value '' is > mandatory. However, the command line parser has historically accepted > '-U' without a number. > > Strictly requiring a number for '-U' would break existing tests > (e.g., in 't4013') and likely disrupt user scripts relying on this > undocumented behavior. > > Since we are retaining this fallback behavior for backward compatibility, > update the documentation to explicitly state that '' can be omitted > for the short option '-U'. In this case, isn't the long option also changed to optional number, such as: `--unified[=]` ? > > Signed-off-by: Tian Yuchen > --- > Documentation/diff-context-options.adoc | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/diff-context-options.adoc > b/Documentation/diff-context-options.adoc index e161260358..655496ec3a 100644 > --- a/Documentation/diff-context-options.adoc > +++ b/Documentation/diff-context-options.adoc > @@ -1,4 +1,4 @@ > -`-U`:: > +`-U[]`:: > `--unified=`:: > Generate diffs with __ lines of context. Defaults to `diff.context` > or 3 if the config option is unset. ``` ## Tian Yuchen, 2026-03-11 04:33 Subject: Re: [PATCH v1] diff: document -U without as using default context Message-ID: <9fc9fb8b-5f91-4670-a674-76774c31d228@malon.dev> URL: https://gitlist.dev/e/9fc9fb8b-5f91-4670-a674-76774c31d228%40malon.dev In-Reply-To: <5973423.DvuYhMxLoT@piment-oiseau> ``` On 3/11/26 01:31, Jean-Noël AVILA wrote: > In this case, isn't the long option also changed to optional number, such as: > > `--unified[=]` > > ? Indeed. Your statement aligns with what Junio said earlier. >> The documentation for '-U' implies that the numeric value '' is >> mandatory. However, the command line parser has historically accepted >> '-U' without a number. >> >> Strictly requiring a number for '-U' would break existing tests >> (e.g., in 't4013') and likely disrupt user scripts relying on this >> undocumented behavior. >> >> Since we are retaining this fallback behavior for backward compatibility, >> update the documentation to explicitly state that '' can be omitted >> for the short option '-U'. >> >> Signed-off-by: Tian Yuchen >> --- >> Documentation/diff-context-options.adoc | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) > > I am moderately nagative. > > It is not like we are _encouraging_ users to omit from -U, > but it is not errored out only due to a bug. Who would the new text > help? Users would wonder why is not optional in --unified=, > the other way to spell the same thing. > > If we want to be explicit, we should probably do this instead: > > `-U`:: > `--unified=`:: > Generate diffs with __ lines of context. Defaults to `diff.context` > or 3 if the config option is unset (`-U` without '' is accepted > as a silent synonym for `-p` due to a historical accident). > > which would tell readers what happens when '' is omitted and why > we allow such an inconsistency. I have already fixed this in the v3 patch. Thank you, Yuchen ```