Re: [PATCH v2] bisect: Honor log.date

From: Peter Krefting <peter@softwolves.pp.se>
Date: Mar 31, 2024, 17:10 UTC
Message-ID: <5ea0837f-2668-028d-4094-c9400e92fceb@softwolves.pp.se>
In-Reply-To: <xmqqh6gni1ur.fsf@gitster.g>

Junio C Hamano:

> I suspect that log.date is a small tip of an iceberg of the benefit
> we'll get from this switch.

Yeah. I was planning on elaborating a bit on that, but forgot completely by the time I came around to look at it. I will update the message with your suggestions for the next version.

Show 24 quoted lines

> Some differences in the proposed output and the current output I see
> are:
>
> - the output now goes to the pager
>
> - it now honors log.mailmap (which may default to true, so you
>   could disable it with log.mailmap=false).
>
> - it shows the ref decoration by default (when the output goes to
>   terminal).
>
> - the commit object names for the merge parents are abbreviated.
>
> - it no longer shows the change summary (creation, deletion,
>   rename, copy).
>
> - it no longer shows the diffstat when the final commit turns out
>   to be a merge commit.
>
> There may be other differences.
>
> I personally welcome the first four changes above, which I suspect
> you didn't intend to make (I suspect that you weren't even aware of
> making these changes).

I hadn't really noticed that the previous implementation *didn't* display this. For the most part, the final output of 'bisect' looks like what I expect 'show' to display, to me it was mostly missing the other things.

Show 5 quoted lines

> If there were no existing users of "git bisect" other than me, I
> would even suggest dropping "--no-abbrev-commit" from the set of
> hardcoded "git show" options, so that the commit object name itself,
> just like the commit object names for the merge parents, gets
> abbreviated.

The full commit hash is shown in the line above anyway, so that entire line is redundant. But since there is no standard format available that omits the commit hash I thought I'd leave it at the full hash to be the most like the previous behaviour as possible.

Show 6 quoted lines

> The final two changes, lack of diffstat for merges, may or may not
> be considered a regression, depending on the user you ask.  I was
> just surprised by them but personally was not too unhappy with the
> behaviour change, but reactions from other couple of thousands of
> Git users (we have at least that many users these days, no?) may be
> different from mine, ranging from "Meh" to "you broke my workflow".

Those two were not intentional. I'll have to do a few test runs to compare the outputs and try to the change as non-intrusive as possible. Thanks.

> If you add --stat, don't you want to add --summary as well?  Try to
> bisect down to a commit that adds or removes files to see the output
> difference to decide.

There are a lot of parameters to show that I have haven't used in my 14+ years of using Git, --summary is one of them. That's why I didn't add it.

-- 
\\// Peter - http://www.softwolves.pp.se/