Re: Git for Windows v2.23.0-rc0, was Re: [ANNOUNCE] Git v2.23.0-rc0

From: Ariadne Conill <ariadne@dereferenced.org>
Date: Aug 1, 2019, 00:37 UTC
Message-ID: <CAAOiGNyCrUSPiF9GjXmpPjosV6Ef8B72v54=-5UTqaTBg2hkFA@mail.gmail.com>
In-Reply-To: <20190801002125.GA176307@google.com>

Hello,

On Wed, Jul 31, 2019 at 7:21 PM Jonathan Nieder <jrnieder@gmail.com> wrote:

Show 38 quoted lines

>
> Jeff King wrote:
>
> > I think part of what my annoyance is responding to (and your willingness
> > to just squelch this for everybody) is that switching this particular
> > default isn't really that big a deal, that it requires annoying people
> > on every single "git log" invocation. Perhaps we would be better
> > squelching it entirely and just putting a mention in the release notes.
>
> Yes.
>
> Although as Dscho mentions, it's particularly irritating because it is
> not part of the paginated output.
>
> I wonder if the ideal might not be to trigger it more selectively, when
> the output actually changed due to a reflog entry.  I mean something
> like
>
>         commit 393a9dd0f9762c69f753a8fa0bc89c203c6b4e9e (HEAD, origin/foo, other/pu)
>         Merge: 18598e40e6 1eba6eb1c2
>         Author: A U Thor <author@example.com> (see "git help mailmap")
>         Date:   Tue Jul 30 15:05:41 2019 -0700
>
>             Merge branch 'jt/fetch-cdn-offload' into foo
>
> The message
>
>         warning: log.mailmap is not set; its implicit value will change in an
>         upcoming release. To squelch this message and preserve current
>         behaviour, set the log.mailmap configuration value to false.
>
>         To squelch this message and adopt the new behaviour now, set the
>         log.mailmap configuration value to true.
>
> is *particularly* unactionable in the current state where we're not
> rewriting authors.  I think we should bite the bullet and just flip
> the default to "true", with the config as an escape hatch to allow
> going back to the old behavior.

This is what I originally proposed, but it was suggested that we should go through the typical cycle for changing default behaviour in Git, e.g. issue a warning and then flip the default in a later release cycle (my plan was to do so in the next cycle, in fact).

Show 16 quoted lines

>
> Is it too late in the release cycle to do that?  If not, we can do
>
> -- >8 --
> Subject: log: use mailmap by default in interactive use
>
> In f0596ecc8de9 (log: add warning for unspecified log.mailmap setting,
> 2019-07-15), we prepared for a future where "git log" uses the mailmap
> by default, using the following conditions:
>
>  1. If log.mailmap or --[no-]use-mailmap is set explicitly explicitly,
>     respect that.  Otherwise:
>
>  2. If output is not going to a terminal or pager, we might be in a
>     script.  Match historical behavior by not using the mailmap.
>     Otherwise:

For what it's worth, personally, I believe that it is best to just always use the mailmap (even in scripts), because most likely if you care about the author line, you're probably caring about the *present* identity of the author who made the commit, because you wish to ask them a question. However, I decided that it would be best to be conservative with the behaviour, for now, just in case somebody was dependent on the historical author lines.

>  3. If the output format was specified using --pretty, we might be in
>     a script that produces output intended for copy/paste.  Match
>     historical behavior by not using the mailmap.  Otherwise:

This one is definitely intentional, as the user specified exactly what they wanted. So, we give them what they want -- if they want the raw author lines, they get that, if they want the mapped version, then they get that, because they explicitly asked for one or the other.

Show 12 quoted lines

>  4. This is an interactive use, where we will be able to change the
>     default behavior.  Print a warning about the upcoming change
>     but don't use the mailmap yet.
>
> In practice, the case (4) turns out to be irritating.  It prints
> before pager setup, so it just flashes on the screen.  It appears on
> every "git log" invocation, even when the mailmap would not result in
> the output changing.  The change is not meaningful to most people, and
> the new default of --use-mailmap is likely to be preferable for all of
> them.
>
> Let's bite the bullet and jump straight to --use-mailmap in case (4).

Yes, I agree, the warning is suboptimal when used with a pager, but we can only write to stderr, since writing to stdout may break scripts. So, given the request of having a transitional period, I felt this was the best we could do without breaking anything.

> While at it, add a new log.mailmap setting "auto" that can be used to
> explicitly request the new automatic behavior (so that e.g. if
> log.mailmap is set to "true" system-side, I can set it to "auto" in my
> per-user configuration).

This sounds like a great compromise, but honestly we should just flip the default. I can't think of any situation where the non-mapped author lines wouldn't be the more preferable default. If you want the raw ones explicitly, you can just ask for them, either with `--no-use-mailmap` or with `log.mailmap=false`.

> Reported-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>

At any rate... I don't have any major objections with going this route as it solves the problem for the typical use-case, so...

Acked-by: Ariadne Conill <ariadne@dereferenced.org>

Ariadne