git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] format-patch: add support for mailmap file

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 14, 2024, 21:53 UTC
Message-ID
<xmqq4j7mydof.fsf@gitster.g>
In-Reply-To
<20240813-jk-support-mailmap-git-format-patch-v1-1-1aea690ea5dd@gmail.com>
Jacob Keller <jacob.e.keller@intel.com> writes:
Show 9 quoted lines
> diff --git a/builtin/log.c b/builtin/log.c
> index 4d4b60caa76a..94560add6fbc 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -975,6 +975,7 @@ struct format_config {
>  	struct log_config log;
>  	enum thread_level thread;
>  	int do_signoff;
> +	int use_mailmap;

As we share the "--[no-]mailmap" option from the command line with "git log", shouldn't we be able to reuse log.use_mailmap_config as well without adding yet another member to the struct? "git log" defaults use_mailmap_config to true, but this command would want to default it to false to avoid disrupting existing users, or something, perhaps?

Show 9 quoted lines
> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
> index 8708b3159309..f3de349990bf 100644
> --- a/Documentation/git-format-patch.txt
> +++ b/Documentation/git-format-patch.txt
> @@ -30,6 +30,7 @@ SYNOPSIS
>  		   [--range-diff=<previous> [--creation-factor=<percent>]]
>  		   [--filename-max-length=<n>]
>  		   [--progress]
> +		   [(--mailmap|--no-mailmap|--use-mailmap|--no-use-mailmap)]

We seem to say "[--[no-]cover-letter]" to abbreviate, and because "--[no-]use-mailmap" is merely a synonym, shouldn't it be sufficient to say

		   [--[no-]mailmap]

without any other frills? I find the use of the (al|terna|tive) here especially annoying, as it is not like it is an error if you give "--mailmap" and then say "--no-mailmap" later on the same command line---it's just the usual "last one wins".

I haven't decided what my response to Peff's concern on the fallout to "rebase --apply". On one hand, those who conciously choose to rebase by creating patches and applying them would find it puzzling if it did not honor format.mailmap setting. But I would not be strongly opposed if we hardcoded to pass "--no-mailmap" to the internal invocation of "format-patch", just like we hardcode "-k" and other options and justified it with "the use of format-patch is a mere implementation detail".

Thanks.
Previous: Junio C Hamano
Message 7 of 7 in “format-patch: add support for mailmap file”
  1. format-patch: add support for mailmap fileJacob Keller, Aug 13, 2024
  2. Josh SteadmonAug 13, 2024
  3. Jacob KellerAug 14, 2024
  4. Jeff KingAug 14, 2024
  5. Jacob KellerAug 14, 2024
  6. Junio C HamanoAug 14, 2024
  7. Junio C HamanoAug 14, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.