mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Martin Ågren" <>
To: Jacob Stopak <>
Subject: Re: [RFC PATCH] shortlog: add group-by options for year and month
Date: Thu, 22 Sep 2022 17:46:30 +0200	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

Hi Jacob,

On Thu, 22 Sept 2022 at 09:48, Jacob Stopak <> wrote:
> Oh and third, for documentation, I updated "git-shortlog.txt", but
> wasn't able to test "git help shortlog" locally and see the updates. Is
> there a way to make that work locally or did I miss a step somewhere?

I can think of two steps. You need to build the documentation (`make
doc`). Look for "make doc" in INSTALL for some variants and
dependencies. Then you need to convince `git help ...` to pick up your
built docs. I would actually skip using/testing `git help` and just go
straight for the rendered page using, e.g, something like

  cd Documentation
  make ./git-shortlog.{1,html}
  man ./git-shortlog.1
  a-browser ./git-shortlog.html

Your docs render fine for me. Thanks for `backticking` for monospace.

> +-m::
> +--month::
> +       This is an alias for `--group=month`.
> +
> +-y::
> +--year::
> +       This is an alias for `--group=year`.
> +

Commit 2338c450b ("shortlog: add grouping option", 2020-09-27)
introduced `--group` and redefined `-c`  and `--committer` to alias to
that new thing. You could simply add a `--group` variant without
actually adding `--year` and `--month`. One of the nice things about
`--group` is that we can potentially have many groupings without having
to carry correspondingly many `--option`s.

In particular, it might be wise to wait with implementing `-y` and `-m`
until we know that your new feature turns out to be so hugely successful
that people start craving `-m` as a short form for `--group=month`. ;-)

See commit 47beb37bc6 ("shortlog: match commit trailers with --group",
2020-09-27) for some prior art of not adding a new `--option` for a new
way of grouping.

>         struct strbuf ident = STRBUF_INIT;
>         struct strbuf oneline = STRBUF_INIT;
> +       struct strbuf buffer;
> +       strbuf_init(&buffer, 100);
> +
>         struct strset dups = STRSET_INIT;
>         struct pretty_print_context ctx = {0};

This trips up `-Werror=declaration-after-statement`. If you build with
`DEVELOPER=Yes`, you should see the same thing.

I played a little with this functionality and it's quite cute. I can
easily imagine going even more granular with this (`--group=week`?), but
that can wait for some other time. :-)

BTW, I got this when `git am`-ing your patch:

  Applying: shortlog: add group-by options for year and month
  .git/rebase-apply/patch:82: space before tab in indent.
                  strftime(sb->buf, strbuf_avail(sb), "%Y/%m", &commit_date);
  .git/rebase-apply/patch:85: space before tab in indent.
                  strftime(sb->buf, strbuf_avail(sb), "%Y", &commit_date);
  .git/rebase-apply/patch:110: trailing whitespace.
  .git/rebase-apply/patch:111: trailing whitespace, indent with spaces.
  .git/rebase-apply/patch:112: indent with spaces.
                                        &ident, &ctx);
  warning: squelched 6 whitespace errors
  warning: 11 lines add whitespace errors.


  reply	other threads:[~2022-09-22 15:47 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-22  6:18 [RFC PATCH] shortlog: add group-by options for year and month Jacob Stopak
2022-09-22 15:46 ` Martin Ågren [this message]
2022-09-22 23:25 ` [RFC PATCH v2] " Jacob Stopak
2022-09-23 16:17   ` Junio C Hamano
2022-09-23 21:19     ` Jacob Stopak
2022-09-23 21:58     ` Jeff King
2022-09-23 22:06       ` Junio C Hamano
2022-09-24  4:38       ` Jacob Stopak
2022-10-05 22:14         ` Jeff King
2022-10-05 21:43       ` Taylor Blau
2022-10-05 22:26         ` Jeff King
2022-10-07  0:48           ` Jacob Stopak
2022-10-07 21:59             ` Taylor Blau
2022-10-11  0:59             ` Jeff King
2022-10-07 22:24           ` Taylor Blau
2022-10-11  1:00             ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

  List information:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).