| Message ID | e74eab6d21f655698ef8b6e1286b44ea070a7af7.1573241590.git.liu.denton@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | learn the "summary" pretty format | expand |
On Fri, Nov 8, 2019 at 3:08 PM Denton Liu <liu.denton@gmail.com> wrote: > Signed-off-by: Denton Liu <liu.denton@gmail.com> > --- > diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh > @@ -154,20 +154,23 @@ do > -test_expect_success 'NUL termination with --reflog --pretty=oneline' ' > - >expect && > - revs="$(git rev-list --reflog)" && > - [...] > - test_cmp expect actual > -' > +for p in oneline summary > +do > + test_expect_success "NUL termination with --reflog --pretty=$p" ' > + >expect && > + revs="$(git rev-list --reflog)" && > + [...] > + test_cmp expect actual > + ' > +done This patch would be less noisy (by eliminating the indentation change) if you wrapped this test in a for-loop back in 7/10 where it was introduced, with the intention of adding more items to the 'for' list. So, in 7/10, you'd have this: for p in online do test_expect_success "NUL termination with --reflog --pretty=$p" ' ... ' done and this patch, 9/10, would just make the minor change: -for p in oneline +for p in oneline summary Having a for-loop with only a single item is a minor-ugly which pays off with less noise in subsequent patch(es), thus easing review burden.
Am 08.11.19 um 21:08 schrieb Denton Liu: > The standard format for referencing other commits within some projects > (such as git.git) is the summary format. This is described in > Documentation/SubmittingPatches as > > If you want to reference a previous commit in the history of a stable > branch, use the format "abbreviated hash (subject, date)", > with the subject enclosed in a pair of double-quotes, like this: > > .... > Commit f86a374 ("pack-bitmap.c: fix a memleak", 2015-03-30) > noticed that ... > .... > > Since this format is so commonly used, standardize it as a pretty > format. > > This format is implemented as a separate flow that skips most of > pretty_print_commit() and instead calls format_commit_summary(). The > reason why this is done is because the other pretty formats expect > output to be generated in a specific order. Specifically, the header, > including the date, is always printed before the commit message, > including the subject. Reversing the order would be possible but would > involve butchering pretty_print_commit() so it is implemented as a > separate flow. > > Signed-off-by: Denton Liu <liu.denton@gmail.com> > --- > Documentation/pretty-formats.txt | 9 ++++ > Documentation/pretty-options.txt | 2 +- > Documentation/rev-list-options.txt | 2 +- > builtin/log.c | 30 +++++++++-- > log-tree.c | 11 ++-- > pretty.c | 31 ++++++++++- > pretty.h | 1 + > t/t4205-log-pretty-formats.sh | 83 +++++++++++++++++++++++++----- > 8 files changed, 144 insertions(+), 25 deletions(-) Hmm, that's quite a lot of code to add to the formatting code with its repeated special-case checks. Why not implement it as a built-in user format, like making it an alias for something like this? git log --format='%C(auto)%h ("%s", %as)' We don't have %as, yet, but making --date=short available as a placeholder would be a good idea anyway (patch below). -- >8 -- Subject: [PATCH] pretty: provide short date format Add the placeholders %as and %cs to format author date and committer date, respectively, without the time part, like --date=short does, i.e. like YYYY-MM-DD. Signed-off-by: René Scharfe <l.s.r@web.de> --- Documentation/pretty-formats.txt | 2 ++ pretty.c | 3 +++ t/t4205-log-pretty-formats.sh | 6 ++++++ 3 files changed, 11 insertions(+) diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt index b87e2e83e6..f80eaab439 100644 --- a/Documentation/pretty-formats.txt +++ b/Documentation/pretty-formats.txt @@ -169,6 +169,7 @@ The placeholders are: '%at':: author date, UNIX timestamp '%ai':: author date, ISO 8601-like format '%aI':: author date, strict ISO 8601 format +'%as':: author date, short format (`YYYY-MM-DD`) '%cn':: committer name '%cN':: committer name (respecting .mailmap, see linkgit:git-shortlog[1] or linkgit:git-blame[1]) @@ -181,6 +182,7 @@ The placeholders are: '%ct':: committer date, UNIX timestamp '%ci':: committer date, ISO 8601-like format '%cI':: committer date, strict ISO 8601 format +'%cs':: committer date, short format (`YYYY-MM-DD`) '%d':: ref names, like the --decorate option of linkgit:git-log[1] '%D':: ref names without the " (", ")" wrapping. '%S':: ref name given on the command line by which the commit was reached diff --git a/pretty.c b/pretty.c index b32f036953..76920c91dd 100644 --- a/pretty.c +++ b/pretty.c @@ -731,6 +731,9 @@ static size_t format_person_part(struct strbuf *sb, char part, case 'I': /* date, ISO 8601 strict */ strbuf_addstr(sb, show_ident_date(&s, DATE_MODE(ISO8601_STRICT))); return placeholder_len; + case 's': + strbuf_addstr(sb, show_ident_date(&s, DATE_MODE(SHORT))); + return placeholder_len; } skip: diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh index f42a69faa2..4980ed4c6f 100755 --- a/t/t4205-log-pretty-formats.sh +++ b/t/t4205-log-pretty-formats.sh @@ -503,6 +503,12 @@ test_expect_success 'ISO and ISO-strict date formats display the same values' ' test_cmp expected actual ' +test_expect_success 'short date' ' + git log --format=%ad%n%cd --date=short >expected && + git log --format=%as%n%cs >actual && + test_cmp expected actual +' + # get new digests (with no abbreviations) test_expect_success 'set up log decoration tests' ' head1=$(git rev-parse --verify HEAD~0) && -- 2.24.0
Am 08.11.19 um 21:08 schrieb Denton Liu: > The standard format for referencing other commits within some projects > (such as git.git) is the summary format. This is described in > Documentation/SubmittingPatches as > > If you want to reference a previous commit in the history of a stable > branch, use the format "abbreviated hash (subject, date)", > with the subject enclosed in a pair of double-quotes, like this: > > .... > Commit f86a374 ("pack-bitmap.c: fix a memleak", 2015-03-30) > noticed that ... > .... > > Since this format is so commonly used, standardize it as a pretty > format. If the title contains a double quote then we get results like these: 26b455f21e ("hashmap_put takes "struct hashmap_entry *"", 2019-10-06) 01991cee86 ("config/alias.txt: change " and ' to `", 2019-06-05) I noticed the lack of quoting in the code and was worried about the output being confusing or unclean, but after staring at the examples above for a while I find that it's not that bad. It's reversible by removing the outer quotes. So perhaps we can keep it like this. René
René Scharfe <l.s.r@web.de> writes: > Hmm, that's quite a lot of code to add to the formatting code with its > repeated special-case checks. Why not implement it as a built-in user > format, like making it an alias for something like this? > > git log --format='%C(auto)%h ("%s", %as)' > > We don't have %as, yet, but making --date=short available as a > placeholder would be a good idea anyway (patch below). Yes! Implementing the 'summary' internally as merely an alias to a canned user format does sound like the right approach. Further, instead of inventing %as, I think you could just use %ad there, and when 'summary' uses that canned user format, flip the default date format to short. That way, "git show -s --pretty=summary --date=iso" would be available to those who like the format. Thanks.
Hi all, On Sun, Nov 10, 2019 at 03:25:41PM +0900, Junio C Hamano wrote: > René Scharfe <l.s.r@web.de> writes: > > > Hmm, that's quite a lot of code to add to the formatting code with its > > repeated special-case checks. Why not implement it as a built-in user > > format, like making it an alias for something like this? > > > > git log --format='%C(auto)%h ("%s", %as)' > > > > We don't have %as, yet, but making --date=short available as a > > placeholder would be a good idea anyway (patch below). > > Yes! Implementing the 'summary' internally as merely an alias to a > canned user format does sound like the right approach. We don't have any canned user formats currently, do we? I'm trying to figure out where to insert the code to do this. Is the idea to just insert a row in the builtin_formats[] table in setup_commit_formats() with the `user_format` field populated? > > Further, instead of inventing %as, I think you could just use %ad > there, and when 'summary' uses that canned user format, flip the > default date format to short. > > That way, "git show -s --pretty=summary --date=iso" would be > available to those who like the format. If we do implement it as a canned format, then we lose support for displaying `--no-abbrev`, `--walk-reflogs`, `--decorate`. Would we be okay with this? Thanks, Denton > > Thanks.
On Mon, Nov 11, 2019 at 03:47:10PM -0800, Denton Liu wrote: > On Sun, Nov 10, 2019 at 03:25:41PM +0900, Junio C Hamano wrote: > > René Scharfe <l.s.r@web.de> writes: > > > > > Hmm, that's quite a lot of code to add to the formatting code with its > > > repeated special-case checks. Why not implement it as a built-in user > > > format, like making it an alias for something like this? > > > > > > git log --format='%C(auto)%h ("%s", %as)' > > > > > > We don't have %as, yet, but making --date=short available as a > > > placeholder would be a good idea anyway (patch below). > > > > Yes! Implementing the 'summary' internally as merely an alias to a > > canned user format does sound like the right approach. > > We don't have any canned user formats currently, do we? I'm trying to > figure out where to insert the code to do this. > > Is the idea to just insert a row in the builtin_formats[] table in > setup_commit_formats() with the `user_format` field populated? Yeah, it's as simple as adding: { "reference", CMIT_FMT_USERFORMAT, 1, 0, 0, "%C(auto)%h (%s, %as)" } Works like a charm, I've been using it for a few years now: https://github.com/szeder/git/commit/3604d0c28e7e2da5415986468994ef71a972e4ed but never seriously considered for submission, because I didn't want to argue about removing the double-quotes around the subject, and couldn't be bothered to check the corner cases (e.g. what if a user sets a pretty format alias with the same name in the configuration?).
On Fri, Nov 08, 2019 at 12:08:34PM -0800, Denton Liu wrote: > The standard format for referencing other commits within some projects > (such as git.git) is the summary format. This is described in > Documentation/SubmittingPatches as > > If you want to reference a previous commit in the history of a stable > branch, use the format "abbreviated hash (subject, date)", > with the subject enclosed in a pair of double-quotes, like this: > > .... > Commit f86a374 ("pack-bitmap.c: fix a memleak", 2015-03-30) > noticed that ... > .... > > Since this format is so commonly used, standardize it as a pretty > format. SubmittingPatches is simply wrong: our de-facto standard format for referencing other commits does not enclose the subject in a pair of double-quotes: $ git log v2.24.0 |grep -E '[0-9a-f]{7} \("' |wc -l 785 $ git log v2.24.0 |grep -E '[0-9a-f]{7} \([^"]' |wc -l 2276 Those double-quotes don't add any value to the references, but they result in weird looking references for 1083 of our commits whose subject lines happen to end with double-quotes, e.g.: f23a465132 ("hashmap_get{,_from_hash} return "struct hashmap_entry *"", 2019-10-06) and without those unnecessary pair of double-quotes we would have ~3000 more commits whose summary would fit on a single line.
SZEDER Gábor <szeder.dev@gmail.com> writes: > Yeah, it's as simple as adding: > > { "reference", CMIT_FMT_USERFORMAT, 1, 0, 0, "%C(auto)%h (%s, %as)" } > > Works like a charm, I've been using it for a few years now: > > https://github.com/szeder/git/commit/3604d0c28e7e2da5415986468994ef71a972e4ed The word "reference" does sound more to the point for the feature than "summary", and it is nice to see that the required change is essentially a single liner ;-) > but never seriously considered for submission, because I didn't want > to argue about removing the double-quotes around the subject, and I personally do not care whether the title is naked or inside a dq-pair, as that part is purely for human consumption. Being for human consumption, I guess we can argue that the shorter the better, so we may want to standardise the recommendation we give in our tutorials and such. > couldn't be bothered to check the corner cases (e.g. what if a user > sets a pretty format alias with the same name in the configuration?). It would be already possible to collide with built-in names, like "short", with or without your addition, wouldn't it? Then that is not an issue you would need to be worried about, right? Thanks.
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt index 71eef684d0..f9c1296d7c 100644 --- a/Documentation/pretty-formats.txt +++ b/Documentation/pretty-formats.txt @@ -63,6 +63,15 @@ This is designed to be as compact as possible. <full commit message> +* 'summary' + + <abbrev hash> ("<title line>", <short author date>) ++ +This format is useful for referring to other commits when writing a new +commit message. Although by default, '<abbrev hash>' is used, this can +be overridden with '--no-abbrev'. In addition, '<short author date>' can +be overridden by with '--date='. + * 'email' From <hash> <date> diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt index e44fc8f738..0a5b206193 100644 --- a/Documentation/pretty-options.txt +++ b/Documentation/pretty-options.txt @@ -3,7 +3,7 @@ Pretty-print the contents of the commit logs in a given format, where '<format>' can be one of 'oneline', 'short', 'medium', - 'full', 'fuller', 'email', 'raw', 'format:<string>' + 'full', 'fuller', 'summary', 'email', 'raw', 'format:<string>' and 'tformat:<string>'. When '<format>' is none of the above, and has '%placeholder' in it, it acts as if '--pretty=tformat:<format>' were given. diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt index 90ff9e2bea..76df494ed6 100644 --- a/Documentation/rev-list-options.txt +++ b/Documentation/rev-list-options.txt @@ -289,7 +289,7 @@ depending on a few rules: 4. Otherwise, show the index format. -- + -Under `--pretty=oneline`, the commit message is +Under `--pretty=oneline` and `--pretty=summary`, the commit message is prefixed with this information on the same line. This option cannot be combined with `--reverse`. See also linkgit:git-reflog[1]. diff --git a/builtin/log.c b/builtin/log.c index e4df16be79..ad96a746a3 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -223,15 +223,35 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix, read_mailmap(rev->mailmap, NULL); } - if (rev->pretty_given && rev->commit_format == CMIT_FMT_RAW) { + if (rev->pretty_given) { + switch (rev->commit_format) { + /* * "log --pretty=raw" is special; ignore UI oriented * configuration variables such as decoration. */ - if (!decoration_given) - decoration_style = 0; - if (!rev->abbrev_commit_explicit) - rev->abbrev_commit = 0; + case CMIT_FMT_RAW: + if (!decoration_given) + decoration_style = 0; + if (!rev->abbrev_commit_explicit) + rev->abbrev_commit = 0; + break; + + /* + * "log --pretty=summary" is special; ignore UI oriented + * configuration variables such as decoration but keep + * abbreviations. + */ + case CMIT_FMT_SUMMARY: + if (!decoration_given) + decoration_style = 0; + if (!rev->abbrev_commit_explicit) + rev->abbrev_commit = 1; + break; + + default: + break; + } } if (decoration_style) { diff --git a/log-tree.c b/log-tree.c index 4a7d668af6..433adaf0f1 100644 --- a/log-tree.c +++ b/log-tree.c @@ -627,7 +627,8 @@ void show_log(struct rev_info *opt) ctx.print_email_subject = 1; } else if (opt->commit_format != CMIT_FMT_USERFORMAT) { fputs(diff_get_color_opt(&opt->diffopt, DIFF_COMMIT), opt->diffopt.file); - if (opt->commit_format != CMIT_FMT_ONELINE) + if (opt->commit_format != CMIT_FMT_ONELINE && + opt->commit_format != CMIT_FMT_SUMMARY) fputs("commit ", opt->diffopt.file); if (!opt->graph) @@ -644,7 +645,8 @@ void show_log(struct rev_info *opt) find_unique_abbrev(&parent->object.oid, abbrev_commit)); fputs(diff_get_color_opt(&opt->diffopt, DIFF_RESET), opt->diffopt.file); show_decorations(opt, commit); - if (opt->commit_format == CMIT_FMT_ONELINE) { + if (opt->commit_format == CMIT_FMT_ONELINE || + opt->commit_format == CMIT_FMT_SUMMARY) { putc(' ', opt->diffopt.file); } else { putc('\n', opt->diffopt.file); @@ -658,12 +660,15 @@ void show_log(struct rev_info *opt) * graph info here. */ show_reflog_message(opt->reflog_info, - opt->commit_format == CMIT_FMT_ONELINE, + (opt->commit_format == CMIT_FMT_ONELINE || + opt->commit_format == CMIT_FMT_SUMMARY), &opt->date_mode, opt->date_mode_explicit); if (opt->commit_format == CMIT_FMT_ONELINE) { putc('\n', opt->diffopt.file); return; + } else if (opt->commit_format == CMIT_FMT_SUMMARY) { + putc(' ', opt->diffopt.file); } } } diff --git a/pretty.c b/pretty.c index a6e5fc115a..8efb2486a5 100644 --- a/pretty.c +++ b/pretty.c @@ -97,7 +97,8 @@ static void setup_commit_formats(void) { "mboxrd", CMIT_FMT_MBOXRD, 0, 0 }, { "fuller", CMIT_FMT_FULLER, 0, 8 }, { "full", CMIT_FMT_FULL, 0, 8 }, - { "oneline", CMIT_FMT_ONELINE, 1, 0 } + { "oneline", CMIT_FMT_ONELINE, 1, 0 }, + { "summary", CMIT_FMT_SUMMARY, 1, 0 }, /* * Please update $__git_log_pretty_formats in * git-completion.bash when you add new formats. @@ -1672,6 +1673,26 @@ void repo_format_commit_message(struct repository *r, do_repo_format_commit_message, (void *)format); } +static void do_repo_format_commit_summary(struct strbuf *sb, + struct format_commit_context *context, + void *additional_context) +{ + struct ident_split ident; + + parse_commit_header(context); + parse_commit_message(context); + + strbuf_addstr(sb, "(\""); + format_subject(sb, context->message + context->subject_off, " "); + if (!split_ident_line(&ident, + context->message + context->author.off, + context->author.len)) { + strbuf_addstr(sb, "\", "); + strbuf_addstr(sb, show_ident_date(&ident, &context->pretty_ctx->date_mode)); + } + strbuf_addstr(sb, ")"); +} + static void pp_header(struct pretty_print_context *pp, const char *encoding, const struct commit *commit, @@ -1923,6 +1944,14 @@ void pretty_print_commit(struct pretty_print_context *pp, format_commit_message(commit, user_format, sb, pp); return; } + if (pp->fmt == CMIT_FMT_SUMMARY) { + if (!pp->date_mode_explicit) + pp->date_mode = *DATE_MODE(SHORT); + + repo_format_commit_generic(the_repository, commit, sb, pp, + do_repo_format_commit_summary, NULL); + return; + } encoding = get_log_output_encoding(); msg = reencoded = logmsg_reencode(commit, NULL, encoding); diff --git a/pretty.h b/pretty.h index 4ad1fc31ff..6d5b18a4f1 100644 --- a/pretty.h +++ b/pretty.h @@ -16,6 +16,7 @@ enum cmit_fmt { CMIT_FMT_FULL, CMIT_FMT_FULLER, CMIT_FMT_ONELINE, + CMIT_FMT_SUMMARY, CMIT_FMT_EMAIL, CMIT_FMT_MBOXRD, CMIT_FMT_USERFORMAT, diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh index d35650cae7..fcfd66faf9 100755 --- a/t/t4205-log-pretty-formats.sh +++ b/t/t4205-log-pretty-formats.sh @@ -154,20 +154,23 @@ do ' done -test_expect_success 'NUL termination with --reflog --pretty=oneline' ' - >expect && - revs="$(git rev-list --reflog)" && - for r in $revs - do - # trim trailing newline - output="$(git show -s --pretty=oneline "$r")" || return 1 - printf "%s" "$output" >>expect - emit_nul >>expect - done && - git log -z --pretty=oneline --reflog >actual && - # no trailing NUL - test_cmp expect actual -' +for p in oneline summary +do + test_expect_success "NUL termination with --reflog --pretty=$p" ' + >expect && + revs="$(git rev-list --reflog)" && + for r in $revs + do + # trim trailing newline + output="$(git show -s --pretty='$p' "$r")" || return 1 + printf "%s" "$output" >>expect + emit_nul >>expect + done && + git log -z --pretty='$p' --reflog >actual && + # no trailing NUL + test_cmp expect actual + ' +done test_expect_success 'setup more commits' ' test_commit "message one" one one message-one && @@ -823,4 +826,56 @@ test_expect_success '%S in git log --format works with other placeholders (part test_cmp expect actual ' +test_expect_success 'log --pretty=summary' ' + git log --date=short --pretty="tformat:%h (\"%s\", %ad)" >expect && + git log --pretty=summary >actual && + test_cmp expect actual +' + +test_expect_success 'log --pretty=summary with log.date is overridden by short date' ' + git log --date=short --pretty="tformat:%h (\"%s\", %ad)" >expect && + test_config log.date rfc && + git log --pretty=summary >actual && + test_cmp expect actual +' + +test_expect_success 'log --pretty=summary with explicit date overrides short date' ' + git log --date=rfc --pretty="tformat:%h (\"%s\", %ad)" >expect && + git log --date=rfc --pretty=summary >actual && + test_cmp expect actual +' + +test_expect_success 'log --pretty=summary with log.abbrevCommit is overidden' ' + git log --date=short --pretty="tformat:%h (\"%s\", %ad)" >expect && + test_config log.abbrevCommit false && + git log --pretty=summary >actual && + test_cmp expect actual +' + +test_expect_success 'log --pretty=summary with explicit --no-abbrev overrides abbreviated' ' + git log --date=short --pretty="tformat:%H (\"%s\", %ad)" >expect && + git log --no-abbrev --pretty=summary >actual && + test_cmp expect actual +' + +test_expect_success 'log --pretty=summary with log.decorate is overridden' ' + git log --date=short --pretty="tformat:%h (\"%s\", %ad)" >expect && + test_config log.decorate short && + git log --pretty=summary >actual && + test_cmp expect actual +' + +test_expect_success 'log --pretty=summary with explicit decorate overrides no decoration' ' + git log --date=short --pretty="tformat:%h%d (\"%s\", %ad)" >expect && + git log --decorate=short --pretty=summary >actual && + test_cmp expect actual +' + +test_expect_success 'log --pretty=summary with --walk-reflogs' ' + test_config log.date short && + git log --walk-reflogs --pretty="tformat:%h %gd: %gs (\"%s\", %ad)" >expect && + git log --walk-reflogs --pretty=summary >actual && + test_cmp expect actual +' + test_done
The standard format for referencing other commits within some projects (such as git.git) is the summary format. This is described in Documentation/SubmittingPatches as If you want to reference a previous commit in the history of a stable branch, use the format "abbreviated hash (subject, date)", with the subject enclosed in a pair of double-quotes, like this: .... Commit f86a374 ("pack-bitmap.c: fix a memleak", 2015-03-30) noticed that ... .... Since this format is so commonly used, standardize it as a pretty format. This format is implemented as a separate flow that skips most of pretty_print_commit() and instead calls format_commit_summary(). The reason why this is done is because the other pretty formats expect output to be generated in a specific order. Specifically, the header, including the date, is always printed before the commit message, including the subject. Reversing the order would be possible but would involve butchering pretty_print_commit() so it is implemented as a separate flow. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- Documentation/pretty-formats.txt | 9 ++++ Documentation/pretty-options.txt | 2 +- Documentation/rev-list-options.txt | 2 +- builtin/log.c | 30 +++++++++-- log-tree.c | 11 ++-- pretty.c | 31 ++++++++++- pretty.h | 1 + t/t4205-log-pretty-formats.sh | 83 +++++++++++++++++++++++++----- 8 files changed, 144 insertions(+), 25 deletions(-)