| Message ID | cover.1666391136.git.me@ttaylorr.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | shortlog: introduce `--group=<format>` | expand |
On Fri, Oct 21, 2022 at 06:25:37PM -0400, Taylor Blau wrote: > Here is a (likely final) reroll of my series to implement arbitrary > pretty formats as shortlog `--group`'s, based on a suggestion from Jacob > Stopak. > > The changes are cosmetic, based on the latest round of review from Peff. > There aren't any substantive changes from the last round. I looked over the range-diff and gave another careful read of patch 4. It mostly looks good, though I did find an oddity in patch 4. It's a really subtle breakage that goes away by the end, so I'm not entirely sure it's worth dealing with. So I'd be happy enough with this version, or if you want to do a final re-roll that addresses that, I'm happy to review it. -Peff
Here is a (likely final) reroll of my series to implement arbitrary pretty formats as shortlog `--group`'s, based on a suggestion from Jacob Stopak. The changes are cosmetic, based on the latest round of review from Peff. There aren't any substantive changes from the last round. Thanks in advance for your review. Jeff King (1): shortlog: accept `--date`-related options Taylor Blau (6): shortlog: make trailer insertion a noop when appropriate shortlog: extract `--group` fragment for translation shortlog: support arbitrary commit format `--group`s shortlog: extract `shortlog_finish_setup()` shortlog: implement `--group=author` in terms of `--group=<format>` shortlog: implement `--group=committer` in terms of `--group=<format>` Documentation/git-shortlog.txt | 8 ++++ builtin/log.c | 1 + builtin/shortlog.c | 87 +++++++++++++++++++++++----------- shortlog.h | 5 ++ t/t4201-shortlog.sh | 39 +++++++++++++++ 5 files changed, 113 insertions(+), 27 deletions(-) Range-diff against v2: 1: 58baccbaa8 ! 1: add457f319 shortlog: accept `--date`-related options @@ Documentation/git-shortlog.txt: OPTIONS Each pretty-printed commit will be rewrapped before it is shown. +--date=<format>:: -+ With a `--group=format:<format>`, show dates formatted -+ according to the given date string. (See the `--date` options -+ in the "COMMIT FORMATTING" section of linkgit:git-log[1].) ++ Show dates formatted according to the given date string. (See ++ the `--date` option in the "Commit Formatting" section of ++ linkgit:git-log[1]). + --group=<type>:: Group commits based on `<type>`. If no `--group` option is 2: 7decccad7c = 2: 25cdc28215 shortlog: make trailer insertion a noop when appropriate 3: 3665488fc9 = 3: cf84f829aa shortlog: extract `--group` fragment for translation 4: 4a36c0ca4e ! 4: f75a952421 shortlog: support arbitrary commit format `--group`s @@ Commit message Signed-off-by: Taylor Blau <me@ttaylorr.com> ## Documentation/git-shortlog.txt ## +@@ Documentation/git-shortlog.txt: OPTIONS + --date=<format>:: + Show dates formatted according to the given date string. (See + the `--date` option in the "Commit Formatting" section of +- linkgit:git-log[1]). ++ linkgit:git-log[1]). Useful with `--group=format:<format>`. + + --group=<type>:: + Group commits based on `<type>`. If no `--group` option is @@ Documentation/git-shortlog.txt: OPTIONS example, if your project uses `Reviewed-by` trailers, you might want to see who has been reviewing with @@ builtin/shortlog.c: static void insert_records_from_trailers(struct shortlog *lo unuse_commit_buffer(commit, commit_buffer); } ++static int shortlog_needs_dedup(const struct shortlog *log) ++{ ++ return log->format.nr > 1 || log->trailers.nr; ++} ++ +static void insert_records_from_format(struct shortlog *log, + struct strset *dups, + struct commit *commit, @@ builtin/shortlog.c: static void insert_records_from_trailers(struct shortlog *lo + + format_commit_message(commit, item->string, &buf, ctx); + -+ if (!(log->format.nr > 1 || log->trailers.nr) || -+ strset_add(dups, buf.buf)) ++ if (!shortlog_needs_dedup(log) || strset_add(dups, buf.buf)) + insert_one_record(log, buf.buf, oneline); + } + @@ t/t4201-shortlog.sh: test_expect_success 'shortlog --group=trailer:signed-off-by + test_cmp expect actual +' + -+test_expect_success 'shortlog multiple --group=format' ' -+ git shortlog -s --date="format:%Y" --group="format:%cN (%cd)" \ -+ HEAD >actual && -+ cat >expect <<-\EOF && -+ 4 C O Mitter (2005) -+ 1 Sin Nombre (2005) -+ EOF -+ test_cmp expect actual -+' -+ +test_expect_success 'shortlog bogus --group' ' + test_must_fail git shortlog --group=bogus HEAD 2>err && + grep "unknown group type" err @@ t/t4201-shortlog.sh: test_expect_success 'shortlog can match multiple groups' ' ' +test_expect_success 'shortlog can match multiple format groups' ' ++ GIT_COMMITTER_NAME="$GIT_AUTHOR_NAME" \ ++ git commit --allow-empty -m "identical names" && ++ test_tick && + cat >expect <<-\EOF && -+ 2 User B <b@example.com> -+ 1 A U Thor <author@example.com> -+ 1 User A <a@example.com> ++ 2 A U Thor ++ 1 C O Mitter + EOF -+ git shortlog -ns \ -+ --group="%(trailers:valueonly,key=some-trailer)" \ -+ --group="%(trailers:valueonly,key=another-trailer)" \ -+ -2 HEAD >actual.raw && -+ grep -v "^$" actual.raw >actual && ++ git shortlog -ns --group="%cn" --group="%an" -2 HEAD >actual && + test_cmp expect actual +' + 5: f0044682be ! 5: 8dd69edcf8 shortlog: implement `--group=author` in terms of `--group=<format>` @@ Metadata Author: Taylor Blau <me@ttaylorr.com> ## Commit message ## - shortlog: implement `--group=author` in terms of `--group=<format>` + shortlog: extract `shortlog_finish_setup()` - Instead of handling SHORTLOG_GROUP_AUTHOR separately, reimplement it as - a special case of the new `--group=<format>` mode, where the author mode - is a shorthand for `--group='%aN <%aE>'. + Extract a function which finishes setting up the shortlog struct for + use. The caller in `make_cover_letter()` does not care about trailer + sorting, so it isn't strictly necessary to add a call there in this + patch. - Note that we still need to keep the SHORTLOG_GROUP_AUTHOR enum since it - has a different meaning in `read_from_stdin()`, where it is still used - for a different purpose. + But the next patch will add additional functionality to the new + `shortlog_finish_setup()` function, which the caller in + `make_cover_letter()` will care about. Signed-off-by: Taylor Blau <me@ttaylorr.com> @@ builtin/log.c: static void make_cover_letter(struct rev_info *rev, int use_separ ## builtin/shortlog.c ## -@@ builtin/shortlog.c: void shortlog_add_commit(struct shortlog *log, struct commit *commit) - } - oneline_str = oneline.len ? oneline.buf : "<none>"; - -- if (log->groups & SHORTLOG_GROUP_AUTHOR) { -- strbuf_reset(&ident); -- format_commit_message(commit, -- log->email ? "%aN <%aE>" : "%aN", -- &ident, &ctx); -- if (!HAS_MULTI_BITS(log->groups) || -- strset_add(&dups, ident.buf)) -- insert_one_record(log, ident.buf, oneline_str); -- } - if (log->groups & SHORTLOG_GROUP_COMMITTER) { - strbuf_reset(&ident); - format_commit_message(commit, @@ builtin/shortlog.c: void shortlog_init(struct shortlog *log) log->format.strdup_strings = 1; } +void shortlog_finish_setup(struct shortlog *log) +{ -+ if (log->groups & SHORTLOG_GROUP_AUTHOR) -+ string_list_append(&log->format, -+ log->email ? "%aN <%aE>" : "%aN"); -+ + string_list_sort(&log->trailers); +} + -: ---------- > 6: f4f233fb48 shortlog: implement `--group=author` in terms of `--group=<format>` 6: 6a9e204177 = 7: 6ce06a054e shortlog: implement `--group=committer` in terms of `--group=<format>`