git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Alejandro Colomar <alx.manpages@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: [PATCH 4/5] format-patch: do not respect diff.noprefix
Date: Thu, 9 Mar 2023 01:11:49 -0500	[thread overview]
Message-ID: <ZAl4pZV08a6Bgoip@coredump.intra.peff.net> (raw)
In-Reply-To: <ZAl3bHB9zxjLITgf@coredump.intra.peff.net>

The output of format-patch respects diff.noprefix, but this usually ends
up being a hassle for people receiving the patch, as they have to
manually specify "-p0" in order to apply it.

I don't think there was any specific intention for it to behave this
way. The noprefix option is handled by git_diff_ui_config(), and
format-patch exists in a gray area between plumbing and porcelain.
People do look at the output, and we'd expect it to colorize things,
respect their choice of algorithm, and so on. But this particular option
creates problems for the receiver (in theory so does diff.mnemonicprefix,
but since we are always formatting commits, the mnemonic prefixes will
always be "a/" and "b/").

So let's disable it. The slight downsides are:

  - people who have set diff.noprefix presumably like to see their
    patches without prefixes. If they use format-patch to review their
    series, they'll see prefixes. On the other hand, it is probably a
    good idea for them to look at what will actually get sent out.

    We could try to play games here with "is stdout a tty", as we do for
    color. But that's not a completely reliable signal, and it's
    probably not worth the trouble. If you want to see the patch with
    the usual bells and whistles, then you are better off using "git
    log" or "git show".

  - if a project really does have a workflow that likes prefix-less
    patches, and the receiver is prepared to use "-p0", then the sender
    now has to manually say "--no-prefix" for each format-patch
    invocation. That doesn't seem _too_ terrible given that the receiver
    has to manually say "-p0" for each git-am invocation.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/log.c           | 9 +++++++++
 t/t4014-format-patch.sh | 5 +++++
 2 files changed, 14 insertions(+)

diff --git a/builtin/log.c b/builtin/log.c
index a70fba198f9..eaf511aab86 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1085,6 +1085,15 @@ static int git_format_config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 
+	/*
+	 * ignore some porcelain config which would otherwise be parsed by
+	 * git_diff_ui_config(), via git_log_config(); we can't just avoid
+	 * diff_ui_config completely, because we do care about some ui options
+	 * like color.
+	 */
+	if (!strcmp(var, "diff.noprefix"))
+		return 0;
+
 	return git_log_config(var, value, cb);
 }
 
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index f3313b8c58f..f5a41fd47ed 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -2386,4 +2386,9 @@ test_expect_success 'interdiff: solo-patch' '
 	test_cmp expect actual
 '
 
+test_expect_success 'format-patch does not respect diff.noprefix' '
+	git -c diff.noprefix format-patch -1 --stdout >actual &&
+	grep "^--- a/blorp" actual
+'
+
 test_done
-- 
2.40.0.rc2.537.g928a61c97db


  parent reply	other threads:[~2023-03-09  6:12 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-08 20:15 Better suggestions when git-am(1) fails Alejandro Colomar
2023-03-09  3:17 ` Jeff King
2023-03-09  6:06   ` Jeff King
2023-03-09  6:07     ` [PATCH 1/5] diff: factor out src/dst prefix setup Jeff King
2023-03-09 10:50       ` Alejandro Colomar
2023-03-09  6:07     ` [PATCH 2/5] t4013: add tests for diff prefix options Jeff King
2023-03-09  6:09     ` [PATCH 3/5] diff: add --default-prefix option Jeff King
2023-03-09 10:51       ` Alejandro Colomar
2023-03-09 16:31       ` Junio C Hamano
2023-03-10  9:44         ` Jeff King
2023-03-10 17:04           ` Junio C Hamano
2023-03-13 16:43             ` Jeff King
2023-03-13 17:17               ` Junio C Hamano
2023-03-13 17:31               ` Junio C Hamano
2023-03-13 19:54                 ` Jeff King
2023-03-09  6:11     ` Jeff King [this message]
2023-03-09 10:53       ` [PATCH 4/5] format-patch: do not respect diff.noprefix Alejandro Colomar
2023-03-09 16:41       ` Junio C Hamano
2023-03-10  9:49         ` Jeff King
2023-03-09  6:12     ` [PATCH 5/5] format-patch: add format.noprefix option Jeff King
2023-03-09 17:00       ` Junio C Hamano
2023-03-10  9:51         ` Jeff King
2023-03-09 10:58     ` Better suggestions when git-am(1) fails Alejandro Colomar
2023-03-09 21:53     ` Junio C Hamano
2023-03-10  9:54       ` Jeff King
2023-03-09 16:22   ` Junio C Hamano
2023-03-10  9:39     ` Jeff King
2023-03-10 16:28       ` Junio C Hamano
2023-03-13 16:37         ` Jeff King
2023-03-13 17:10           ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZAl4pZV08a6Bgoip@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=alx.manpages@gmail.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).