From: "Martin Ågren" <martin.agren@gmail.com>
To: Jacob Stopak <jacob@initialcommit.io>
Cc: git@vger.kernel.org
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: <CAN0heSr8HFR1y+aZUjFaeY3y-9yn+nfyDrkxQh82punLnSonGg@mail.gmail.com> (raw)
In-Reply-To: <20220922061824.16988-1-jacob@initialcommit.io>
Hi Jacob,
On Thu, 22 Sept 2022 at 09:48, Jacob Stopak <jacob@initialcommit.io> 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
next prev parent 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:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAN0heSr8HFR1y+aZUjFaeY3y-9yn+nfyDrkxQh82punLnSonGg@mail.gmail.com \
--to=martin.agren@gmail.com \
--cc=git@vger.kernel.org \
--cc=jacob@initialcommit.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* 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
https://80x24.org/mirrors/git.git
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).