Re: [PATCH 3/5] diff: add --default-prefix option

From

Jeff King <peff@peff.net>

Date

Mar 13, 2023, 16:43 UTC

Message-ID

<ZA9SmZaUyrgbH2fb@coredump.intra.peff.net>

In-Reply-To

<xmqqcz5g7d2i.fsf@gitster.g>

On Fri, Mar 10, 2023 at 09:04:21AM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > While it is not _quite_ the same thing to say "use prefixes a/ and b/"
> > versus "countermand any config and use the default", it is close enough
> > that I am tempted to say this patch should be scrapped. I mostly just
> > wanted to have a way to counter format.noprefix, if we are going to
> > endorse it as a concept (whether by adding it, or saying "no, respecting
> > diff.noprefix is not a bug").
> >
> > (If we do scrap it, I'd probably fold the extra tests into the previous
> > commit, but using --src-prefix, etc).
> 
> I would very much like to keep this one; if we can find a shorter
> name that would be even sweeter.

By the way, we might also want something like this:

diff --git a/builtin/rebase.c b/builtin/rebase.c
index dd31d5ab91e..5b7b908b66b 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -661,7 +661,7 @@ static int run_am(struct rebase_options *opts)
 	format_patch.git_cmd = 1;
 	strvec_pushl(&format_patch.args, "format-patch", "-k", "--stdout",
 		     "--full-index", "--cherry-pick", "--right-only",
-		     "--src-prefix=a/", "--dst-prefix=b/", "--no-renames",
+		     "--default-prefix", "--no-renames",
 		     "--no-cover-letter", "--pretty=mboxrd", "--topo-order",
 		     "--no-base", NULL);
 	if (opts->git_format_patch_opt.len)

which uses --src-prefix to (you may have guessed it!) counteract
diff.noprefix in the user's config. (It would still be necessary even
with my series because the user might have set format.patch).

> I am wondering if we can keep the current behaviour instead and send
> a message: "if you do not want your everyday diff not to have
> prefixes, fine, go set diff.noprefix, but if you do not like that
> format-patch also gives a no-prefix patches with that configuration,
> or at times you may want your 'git show' to show the standard
> prefix, you can countermand your diff.noprefix configuration".

Sure, but how do we send that message? I guess if we leave diff.noprefix
as it is and add a new format.patch (which preempts diff.noprefix only
for format-patch), then people will still accidentally send patches
without prefixes, but at least there is an "out" for the maintainer
receiving them to say "don't do that; please set format.patch".

I was hoping to avoid having the accident happen in the first place, but
if we're not willing to change how diff.noprefix works now, then I think
that's our runner-up.

It would be pretty easy to rework the series (drop patch 4, and tweak
patch 5 to be a yes/no/unset tri-state, plus a new test to check the
fallback behavior).

-Peff