git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alejandro Colomar <alx.manpages@gmail.com>
To: Jeff King <peff@peff.net>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH 4/5] format-patch: do not respect diff.noprefix
Date: Thu, 9 Mar 2023 11:53:00 +0100	[thread overview]
Message-ID: <48706417-c184-fdd8-e3f4-edfd582b4358@gmail.com> (raw)
In-Reply-To: <ZAl4pZV08a6Bgoip@coredump.intra.peff.net>


[-- Attachment #1.1: Type: text/plain, Size: 3235 bytes --]



On 3/9/23 07:11, Jeff King wrote:
> 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>

Acked-by: Alejandro Colomar <alx@kernel.org>

> ---
>  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

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2023-03-09 10:53 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     ` [PATCH 4/5] format-patch: do not respect diff.noprefix Jeff King
2023-03-09 10:53       ` Alejandro Colomar [this message]
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

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

  git send-email \
    --in-reply-to=48706417-c184-fdd8-e3f4-edfd582b4358@gmail.com \
    --to=alx.manpages@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /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.
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 NNTP newsgroup(s).