Re: [PATCH v2 09/10] pretty: implement 'summary' format

From: "René Scharfe" <l.s.r@web.de>
To: Denton Liu <liu.denton@gmail.com>,
	Git Mailing List <git@vger.kernel.org>
Cc: Eric Sunshine <sunshine@sunshineco.com>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH v2 09/10] pretty: implement 'summary' format
Date: Sat, 9 Nov 2019 07:38:13 +0100	[thread overview]
Message-ID: <08afdbcd-5972-05f9-ec8c-b12bd29d9030@web.de> (raw)
In-Reply-To: <e74eab6d21f655698ef8b6e1286b44ea070a7af7.1573241590.git.liu.denton@gmail.com>

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
