Hi Jacob,
On Thu, 22 Sept 2022 at 09:48, Jacob Stopak <jacob@xxxxxxxxxxxxxxxx> 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.
format_commit_message(commit,
.git/rebase-apply/patch:111: trailing whitespace, indent with spaces.
buffer.buf,
.git/rebase-apply/patch:112: indent with spaces.
&ident, &ctx);
warning: squelched 6 whitespace errors
warning: 11 lines add whitespace errors.
Martin
