Re: [PATCH 2/8] shortlog: refactor committer/author grouping

From: Jeff King <peff@peff.net>
Date: Sep 27, 2020, 08:03 UTC
Message-ID: <20200927080345.GB1286220@coredump.intra.peff.net>
In-Reply-To: <CAPig+cTfYmNAP_65RDa_fZOyuQEH65HuLs-UYSPT0yJ=s6BftA@mail.gmail.com>

On Fri, Sep 25, 2020 at 04:05:04PM -0400, Eric Sunshine wrote:

Show 15 quoted lines

> > +--group=<type>::
> > +       By default, `shortlog` collects and collates author identities;
> > +       using `--group` will collect other fields.
> > +       `<type>` is one of:
> > ++
> > + - `author`, commits are grouped by author (this is the default)
> > + - `committer`, commits are grouped by committer (the same as `-c`)
> 
> I had trouble interpreting "(this is the default)". It made me think
> that <type> is optional:
> 
>     --group[=<type>]
> 
> but that isn't the case at all. Instead, it means that if --group
> isn't specified, then grouping is done by `author` by default.

Right.

Show 5 quoted lines

> It also
> repeats what the general description of --group already says with
> regard to the default, thus it is redundant to say it again when
> describing the `author` type. Therefore, perhaps drop "(this is the
> default)" altogether?

OK. I was worried somebody wouldn't quite pick up on the default. We could spell it out as:

  - `author`, commits are grouped by author (this is the default if no
    --group is given)

but that's pretty lengthy. We could also reword the whole thing as:

  --group=<type>::
          Collect and collate values of type `<type>`; if no `--group`
	  option is given, the default is `author`:
   - `author`, commits are grouped by author
   - `committer`, commits are grouped by committer (the same as `-c`)

I'm not sure if that's clearer or not. The first sentence seems a bit opaque, but I'm not sure how to improve it.

For now (and what plan to send out in v2), I'll take your suggestion to just drop the phrase.

-Peff