Re: [PATCH 0/7] shortlog: introduce `--group=<format>`

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 24, 2022, 21:59 UTC
Message-ID: <xmqqwn8orihk.fsf@gitster.g>
In-Reply-To: <cover.1666637725.git.me@ttaylorr.com>

Taylor Blau <me@ttaylorr.com> writes:

Show 14 quoted lines

> Here is one more very tiny reroll of my series to implement arbitrary
> pretty formats as shortlog `--group`'s, based on a suggestion from Jacob
> Stopak.
>
> There are only a couple of changes from last time: one to rebase onto
> the current tip of 'master', and another to address a bug in 4/7 (which
> was resolved by the end of the series, but now works consistently
> throughout the series).
>
> This was pointed out by Peff in [1], and he indicated there:
>
>> It's hard to care too much, since the end result of the series is
>> correct, and you'd end up just removing that part of the line in
>> the final patch. So I could go either way on re-rolling.

One "huh?" is that the final patch in this round hasn't changed from the last round, so the check for HAS_MULTI_BITS(log->groups) remains in the final result.

IOW, after replacing the previous series with this round,

    $ git diff @{1}
    diff --git c/builtin/shortlog.c w/builtin/shortlog.c
    index 27b057940d..27a87167e1 100644
    --- c/builtin/shortlog.c
    +++ w/builtin/shortlog.c
    @@ -207,7 +207,7 @@ static void insert_records_from_trailers(struct shortlog *log,

     static int shortlog_needs_dedup(const struct shortlog *log)
     {
    -	return log->format.nr > 1 || log->trailers.nr;
    +	return HAS_MULTI_BITS(log->groups) || log->format.nr > 1 || log->trailers.nr;
     }

     static void insert_records_from_format(struct shortlog *log,

is the difference in the end result.