[2/7] shortlog: accept `--date`-related options

Commit Message

Taylor Blau Oct. 11, 2022, 12:34 a.m. UTC

From: Jeff King <[e-mail]>

Prepare for the future patch which will introduce arbitrary pretty
formats via the `--group` argument.

To allow additional customizability (for example, to support something
like `git shortlog -s --group='%aD' --date='format:%Y-%m' ...` (which
groups commits by the datestring 'YYYY-mm' according to author date), we
must store off the `--date` parsed from calling `parse_revision_opt()`.

Signed-off-by: Jeff King <[e-mail]>
Signed-off-by: Taylor Blau <[e-mail]>
---
 Documentation/git-shortlog.txt | 2 ++
 builtin/shortlog.c             | 3 ++-
 shortlog.h                     | 2 ++
 3 files changed, 6 insertions(+), 1 deletion(-)

Comments

Jeff King Oct. 11, 2022, 12:48 a.m. UTC | #1

On Mon, Oct 10, 2022 at 08:34:05PM -0400, Taylor Blau wrote:

> From: Jeff King <[e-mail]>
> 
> Prepare for the future patch which will introduce arbitrary pretty
> formats via the `--group` argument.
> 
> To allow additional customizability (for example, to support something
> like `git shortlog -s --group='%aD' --date='format:%Y-%m' ...` (which
> groups commits by the datestring 'YYYY-mm' according to author date), we
> must store off the `--date` parsed from calling `parse_revision_opt()`.

Sorry, I haven't had a chance yet to look carefully at the rest of the
shortlog discussion, but I wanted to note here: this patch also affects
custom output formats. So:

  - we probably want to note that in the commit message as a limitation
    of this, versus something like %(authordate:format=...). I think
    that's fine, as the same limitation already applies to multiple
    dates in a single format string.

  - it's arguably a bug-fix, and can be tested in isolation like:

       git shortlog --format='%ad %s' --date=short

    which currently ignores the --date option entirely.

-Peff

Taylor Blau Oct. 11, 2022, 1:14 a.m. UTC | #2

On Mon, Oct 10, 2022 at 08:48:49PM -0400, Jeff King wrote:
>   - it's arguably a bug-fix, and can be tested in isolation like:
>
>        git shortlog --format='%ad %s' --date=short
>
>     which currently ignores the --date option entirely.

Good call-out, and I agree with this reasoning. I updated the patch
accordingly, thanks!

Thanks,
Taylor

Patch

diff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt
index f64e77047b..4982ceee21 100644
--- a/Documentation/git-shortlog.txt
+++ b/Documentation/git-shortlog.txt
@@ -108,6 +108,8 @@  options or the revision range, when confusion arises.
 :git-shortlog: 1
 include::rev-list-options.txt[]
 
+include::date-options.txt[]
+
 MAPPING AUTHORS
 ---------------
 
diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 7a1e1fe7c0..53c379a51d 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -211,7 +211,7 @@  void shortlog_add_commit(struct shortlog *log, struct commit *commit)
 	ctx.fmt = CMIT_FMT_USERFORMAT;
 	ctx.abbrev = log->abbrev;
 	ctx.print_email_subject = 1;
-	ctx.date_mode.type = DATE_NORMAL;
+	ctx.date_mode = log->date_mode;
 	ctx.output_encoding = get_log_output_encoding();
 
 	if (!log->summary) {
@@ -407,6 +407,7 @@  int cmd_shortlog(int argc, const char **argv, const char *prefix)
 	log.user_format = rev.commit_format == CMIT_FMT_USERFORMAT;
 	log.abbrev = rev.abbrev;
 	log.file = rev.diffopt.file;
+	log.date_mode = rev.date_mode;
 
 	if (!log.groups)
 		log.groups = SHORTLOG_GROUP_AUTHOR;
diff --git a/shortlog.h b/shortlog.h
index 3f7e9aabca..dc388dd459 100644
--- a/shortlog.h
+++ b/shortlog.h
@@ -2,6 +2,7 @@ 
 #define SHORTLOG_H
 
 #include "string-list.h"
+#include "date.h"
 
 struct commit;
 
@@ -15,6 +16,7 @@  struct shortlog {
 	int in2;
 	int user_format;
 	int abbrev;
+	struct date_mode date_mode;
 
 	enum {
 		SHORTLOG_GROUP_AUTHOR = (1 << 0),