[PATCH 0/4] format-rev: add --abbrev, --color, and --date

[PATCH 0/4] format-rev: add --abbrev, --color, and --date

[kristofferhaugsbakk]

From: Kristoffer Haugsbakk <code@khaugsbakk.name>

Topic name: kh/format-rev-more-options

Topic summary: Add three more options for controlling the formatting. Also
do some minor refactoring and text fixes as preparatory steps.

[1/4] format-rev: use lower case for opts description
[2/4] format-rev: factor option variables into a struct
[3/4] doc: rev-list-options.adoc: factor out --date alts
[4/4] format-rev: learn --abbrev, --color, and --date

 Documentation/git-format-rev.adoc             | 44 +++++++++-
 .../rev-list-option-date-alternatives.adoc    | 55 ++++++++++++
 Documentation/rev-list-options.adoc           | 56 +-----------
 builtin/name-rev.c                            | 88 ++++++++++++-------
 t/t6120-describe.sh                           | 44 ++++++++++
 5 files changed, 195 insertions(+), 92 deletions(-)
 create mode 100644 Documentation/rev-list-option-date-alternatives.adoc


base-commit: 010afd3166ddc64c9863b1506f12cbcdda0d4ea1
-- 
2.54.0.22.g9e26862b904

[PATCH 1/4] format-rev: use lower case for opts description

[kristofferhaugsbakk]

From: Kristoffer Haugsbakk <code@khaugsbakk.name>

The option descriptions use a mix of initial capital and lower case
letters. Lower case is the correct style.

Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---
 builtin/name-rev.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 60cbbfb4b7d..254c88199fd 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -833,12 +833,12 @@ int cmd_format_rev(int argc,
 		OPT_STRING_LIST(0, "notes", &notes, N_("notes"),
 				N_("display notes for pretty format")),
 		OPT_CALLBACK_F('z', "null", &nul_data, N_("z"),
-			       N_("Use NUL for input and output termination"),
+			       N_("use NUL for input and output termination"),
 			       PARSE_OPT_NOARG | PARSE_OPT_NONEG, format_nul_cb),
 		OPT_BOOL(0, "null-input", &nul_data.nul_input,
-			 N_("Use NUL for input termination")),
+			 N_("use NUL for input termination")),
 		OPT_BOOL(0, "null-output", &nul_data.nul_output,
-			 N_("Use NUL for output termination")),
+			 N_("use NUL for output termination")),
 		OPT_END(),
 	};
 
-- 
2.54.0.22.g9e26862b904

[PATCH 2/4] format-rev: factor option variables into a struct

[kristofferhaugsbakk]

From: Kristoffer Haugsbakk <code@khaugsbakk.name>

We will in two commits add three more options to this command.
Let’s prepare for that by moving option variables into a struct
so that we get less local variables.

This allows us to inline `format_nul_data` into this new
structure. Let’s also rename `stdin_mode_arg` to `stdin_mode`.
(We couldn’t use `stdin_mode` before because of the enumeration
with the same name.)

Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---
 builtin/name-rev.c | 44 +++++++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 21 deletions(-)

diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 254c88199fd..7d824aa1c5d 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -772,16 +772,19 @@ int cmd_name_rev(int argc,
 	return 0;
 }
 
-struct format_nul_data {
+struct format_rev_data {
+	const char *format;
+	const char *stdin_mode;
 	bool nul_input;
 	bool nul_output;
+	struct string_list notes;
 };
 
 static int format_nul_cb(const struct option *option,
 			 const char *arg,
 			 int unset)
 {
-	struct format_nul_data *data = option->value;
+	struct format_rev_data *data = option->value;
 	data->nul_input = 1;
 	data->nul_output = 1;
 	BUG_ON_OPT_NEG(unset);
@@ -813,31 +816,30 @@ int cmd_format_rev(int argc,
 		   const char *prefix,
 		   struct repository *repo UNUSED)
 {
-	const char *format = NULL;
+	struct format_rev_data data = {
+		NULL, NULL, 0, 0, STRING_LIST_INIT_NODUP
+	};
 	enum stdin_mode stdin_mode;
-	const char *stdin_mode_arg = NULL;
-	struct format_nul_data nul_data = { 0, 0 };
 	char output_terminator;
 	strbuf_getline_fn getline_fn;
 	struct display_notes_opt format_notes_opt;
 	struct rev_info format_rev = REV_INFO_INIT;
 	struct pretty_format format_pp = { 0 };
-	struct string_list notes = STRING_LIST_INIT_NODUP;
 	struct strbuf scratch_buf = STRBUF_INIT;
 	struct command cmd;
 	struct option opts[] = {
-		OPT_STRING(0, "format", &format, N_("format"),
+		OPT_STRING(0, "format", &data.format, N_("format"),
 			   N_("pretty format to use")),
-		OPT_STRING(0, "stdin-mode", &stdin_mode_arg, N_("stdin-mode"),
+		OPT_STRING(0, "stdin-mode", &data.stdin_mode, N_("stdin-mode"),
 			   N_("how revs are processed")),
-		OPT_STRING_LIST(0, "notes", &notes, N_("notes"),
+		OPT_STRING_LIST(0, "notes", &data.notes, N_("notes"),
 				N_("display notes for pretty format")),
-		OPT_CALLBACK_F('z', "null", &nul_data, N_("z"),
+		OPT_CALLBACK_F('z', "null", &data, N_("z"),
 			       N_("use NUL for input and output termination"),
 			       PARSE_OPT_NOARG | PARSE_OPT_NONEG, format_nul_cb),
-		OPT_BOOL(0, "null-input", &nul_data.nul_input,
+		OPT_BOOL(0, "null-input", &data.nul_input,
 			 N_("use NUL for input termination")),
-		OPT_BOOL(0, "null-output", &nul_data.nul_output,
+		OPT_BOOL(0, "null-output", &data.nul_output,
 			 N_("use NUL for output termination")),
 		OPT_END(),
 	};
@@ -849,18 +851,18 @@ int cmd_format_rev(int argc,
 		usage_with_options(format_rev_usage, opts);
 	}
 
-	if (!format)
+	if (!data.format)
 		die(_("'%s' is required"), "--format");
-	if (!stdin_mode_arg)
+	if (!data.stdin_mode)
 		die(_("'%s' is required"), "--stdin-mode");
 
-	getline_fn = nul_data.nul_input ? strbuf_getline_nul : strbuf_getline_lf;
-	output_terminator = nul_data.nul_output ? '\0' : '\n';
+	getline_fn = data.nul_input ? strbuf_getline_nul : strbuf_getline_lf;
+	output_terminator = data.nul_output ? '\0' : '\n';
 
 	init_display_notes(&format_notes_opt);
-	stdin_mode = parse_stdin_mode(stdin_mode_arg);
+	stdin_mode = parse_stdin_mode(data.stdin_mode);
 
-	get_commit_format(format, &format_rev);
+	get_commit_format(data.format, &format_rev);
 	format_pp.ctx.rev = &format_rev;
 	format_pp.ctx.fmt = format_rev.commit_format;
 	format_pp.ctx.abbrev = format_rev.abbrev;
@@ -868,13 +870,13 @@ int cmd_format_rev(int argc,
 	format_pp.ctx.date_mode = format_rev.date_mode;
 	format_pp.ctx.color = GIT_COLOR_AUTO;
 
-	userformat_find_requirements(format,
+	userformat_find_requirements(data.format,
 				     &format_pp.want);
 	if (format_pp.want.notes) {
 		int ignore_show_notes = 0;
 		struct string_list_item *n;
 
-		for_each_string_list_item(n, &notes)
+		for_each_string_list_item(n, &data.notes)
 			enable_ref_display_notes(&format_notes_opt,
 						 &ignore_show_notes,
 						 n->string);
@@ -934,7 +936,7 @@ int cmd_format_rev(int argc,
 	}
 
 	strbuf_release(&scratch_buf);
-	string_list_clear(&notes, 0);
+	string_list_clear(&data.notes, 0);
 	release_display_notes(&format_notes_opt);
 	return 0;
 }
-- 
2.54.0.22.g9e26862b904

[PATCH 3/4] doc: rev-list-options.adoc: factor out --date alts

[kristofferhaugsbakk]

From: Kristoffer Haugsbakk <code@khaugsbakk.name>

We will introduce `--date` to git-format-rev(1) in the next commit and
will need to add it to the documentation. Let’s factor out the option
alternatives so that it can be included in git-format-rev(1).

The initial paragraph of this option mentions things like git-log(1).
We could make it fit in git-format-rev(1) while not changing it for
git-rev-list(1) and related commands with some conditionals like
`ifndef`, but writing a dedicated paragraph is simple enough.

Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---
 .../rev-list-option-date-alternatives.adoc    | 55 ++++++++++++++++++
 Documentation/rev-list-options.adoc           | 56 +------------------
 2 files changed, 56 insertions(+), 55 deletions(-)
 create mode 100644 Documentation/rev-list-option-date-alternatives.adoc

diff --git a/Documentation/rev-list-option-date-alternatives.adoc b/Documentation/rev-list-option-date-alternatives.adoc
new file mode 100644
index 00000000000..141570b1059
--- /dev/null
+++ b/Documentation/rev-list-option-date-alternatives.adoc
@@ -0,0 +1,55 @@
+--
+`--date=relative` shows dates relative to the current time,
+e.g. ``2 hours ago''. The `-local` option has no effect for
+`--date=relative`.
+
+`--date=local` is an alias for `--date=default-local`.
+
+`--date=iso` (or `--date=iso8601`) shows timestamps in a ISO 8601-like format.
+The differences to the strict ISO 8601 format are:
+
+	- a space instead of the `T` date/time delimiter
+	- a space between time and time zone
+	- no colon between hours and minutes of the time zone
+
+`--date=iso-strict` (or `--date=iso8601-strict`) shows timestamps in strict
+ISO 8601 format.
+
+`--date=rfc` (or `--date=rfc2822`) shows timestamps in RFC 2822
+format, often found in email messages.
+
+`--date=short` shows only the date, but not the time, in `YYYY-MM-DD` format.
+
+`--date=raw` shows the date as seconds since the epoch (1970-01-01
+00:00:00 UTC), followed by a space, and then the timezone as an offset
+from UTC (a `+` or `-` with four digits; the first two are hours, and
+the second two are minutes). I.e., as if the timestamp were formatted
+with `strftime("%s %z")`).
+Note that the `-local` option does not affect the seconds-since-epoch
+value (which is always measured in UTC), but does switch the accompanying
+timezone value.
+
+`--date=human` shows the timezone if the timezone does not match the
+current time-zone, and doesn't print the whole date if that matches
+(ie skip printing year for dates that are "this year", but also skip
+the whole date itself if it's in the last few days and we can just say
+what weekday it was).  For older dates the hour and minute is also
+omitted.
+
+`--date=unix` shows the date as a Unix epoch timestamp (seconds since
+1970).  As with `--raw`, this is always in UTC and therefore `-local`
+has no effect.
+
+`--date=format:<format>` feeds the _<format>_ to your system `strftime`,
+except for `%s`, `%z`, and `%Z`, which are handled internally.
+Use `--date=format:%c` to show the date in your system locale's
+preferred format.  See the `strftime`(3) manual for a complete list of
+format placeholders. When using `-local`, the correct syntax is
+`--date=format-local:<format>`.
+
+`--date=default` is the default format, and is based on ctime(3)
+output.  It shows a single line with three-letter day of the week,
+three-letter month, day-of-month, hour-minute-seconds in "HH:MM:SS"
+format, followed by 4-digit year, plus timezone information, unless
+the local time zone is used, e.g. `Thu Jan 1 00:00:00 1970 +0000`.
+--
diff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc
index fd831f0ec64..6e6093f4747 100644
--- a/Documentation/rev-list-options.adoc
+++ b/Documentation/rev-list-options.adoc
@@ -1132,61 +1132,7 @@ include::pretty-options.adoc[]
 	author's). If `-local` is appended to the format (e.g.,
 	`iso-local`), the user's local time zone is used instead.
 +
---
-`--date=relative` shows dates relative to the current time,
-e.g. ``2 hours ago''. The `-local` option has no effect for
-`--date=relative`.
-
-`--date=local` is an alias for `--date=default-local`.
-
-`--date=iso` (or `--date=iso8601`) shows timestamps in a ISO 8601-like format.
-The differences to the strict ISO 8601 format are:
-
-	- a space instead of the `T` date/time delimiter
-	- a space between time and time zone
-	- no colon between hours and minutes of the time zone
-
-`--date=iso-strict` (or `--date=iso8601-strict`) shows timestamps in strict
-ISO 8601 format.
-
-`--date=rfc` (or `--date=rfc2822`) shows timestamps in RFC 2822
-format, often found in email messages.
-
-`--date=short` shows only the date, but not the time, in `YYYY-MM-DD` format.
-
-`--date=raw` shows the date as seconds since the epoch (1970-01-01
-00:00:00 UTC), followed by a space, and then the timezone as an offset
-from UTC (a `+` or `-` with four digits; the first two are hours, and
-the second two are minutes). I.e., as if the timestamp were formatted
-with `strftime("%s %z")`).
-Note that the `-local` option does not affect the seconds-since-epoch
-value (which is always measured in UTC), but does switch the accompanying
-timezone value.
-
-`--date=human` shows the timezone if the timezone does not match the
-current time-zone, and doesn't print the whole date if that matches
-(ie skip printing year for dates that are "this year", but also skip
-the whole date itself if it's in the last few days and we can just say
-what weekday it was).  For older dates the hour and minute is also
-omitted.
-
-`--date=unix` shows the date as a Unix epoch timestamp (seconds since
-1970).  As with `--raw`, this is always in UTC and therefore `-local`
-has no effect.
-
-`--date=format:<format>` feeds the _<format>_ to your system `strftime`,
-except for `%s`, `%z`, and `%Z`, which are handled internally.
-Use `--date=format:%c` to show the date in your system locale's
-preferred format.  See the `strftime`(3) manual for a complete list of
-format placeholders. When using `-local`, the correct syntax is
-`--date=format-local:<format>`.
-
-`--date=default` is the default format, and is based on ctime(3)
-output.  It shows a single line with three-letter day of the week,
-three-letter month, day-of-month, hour-minute-seconds in "HH:MM:SS"
-format, followed by 4-digit year, plus timezone information, unless
-the local time zone is used, e.g. `Thu Jan 1 00:00:00 1970 +0000`.
---
+include::rev-list-option-date-alternatives.adoc[]
 
 ifdef::git-rev-list[]
 `--header`::
-- 
2.54.0.22.g9e26862b904

[PATCH 4/4] format-rev: learn --abbrev, --color, and --date

[kristofferhaugsbakk]

From: Kristoffer Haugsbakk <code@khaugsbakk.name>

Add three more options for controlling the formatting.

This does not complete all the pretty formatting knobs for this command
relative to e.g. git-log(1), but it does add the most important ones, in
my opinion. We can see which are missing by taking a look at
`Documentation/pretty-options.adoc`:

• `--encoding=<encoding>`
• `--show-signature`
• `--expand-tabs=<n>`

***

We could add these options to the command synopsis, but let’s instead
simplify the synopsis to just mention the mandatory options and stuff
the other ones into `[<options>]`. I don’t think a long command synopsis
line is useful. And this way the two mandatory options stand out more.

Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---

Notes (series):
    > We can see which are missing by taking a look at
    
    Or am I missing some?

 Documentation/git-format-rev.adoc | 44 ++++++++++++++++++++++++++++---
 builtin/name-rev.c                | 42 ++++++++++++++++++++---------
 t/t6120-describe.sh               | 44 +++++++++++++++++++++++++++++++
 3 files changed, 115 insertions(+), 15 deletions(-)

diff --git a/Documentation/git-format-rev.adoc b/Documentation/git-format-rev.adoc
index 505a52feccd..1a06ccbf9b8 100644
--- a/Documentation/git-format-rev.adoc
+++ b/Documentation/git-format-rev.adoc
@@ -9,7 +9,7 @@ git-format-rev - EXPERIMENTAL: Pretty format revisions on demand
 SYNOPSIS
 --------
 [synopsis]
-(EXPERIMENTAL!) git format-rev --stdin-mode=<mode> --format=<pretty> [--[no-]notes=<ref>] [-z] [--[no-]null-output] [--[no-]null-input]
+(EXPERIMENTAL!) git format-rev [<options>] --stdin-mode=<mode> --format=<pretty>
 
 DESCRIPTION
 -----------
@@ -33,8 +33,8 @@ OPTIONS
 The argument `rev` is also accepted.
 
 `text`;; Formats all commit object names found in freeform text. These
-	must be full object names, i.e. abbreviated hexadecimal object
-	names will not be interpreted.
+	must be full object names, i.e. abbreviated hexadecimal (_hex_)
+	object names will not be interpreted.
 +
 Anything that is parsed as an object name but that is not found to be a
 commit object name is left alone (echoed).
@@ -76,6 +76,44 @@ This is useful if the output could contain newlines, for example if the
 +
 This is useful if the input revision expressions could contain newlines.
 
+`--color[=<when>]`::
+`--no-color`::
+	Respect color formatting. The default color behavior is
+	`auto`. Bare `--color` is the same as `--color=always`.
++
+Giving `--no-color` is the same as `--color=never`.
++
+_<when>_ must be one of:
++
+--
+`always`;;
+	Always use color, even if the output is something like a file.
+`never`;;
+	Never use color.
+`auto`;;
+	Use color when the output is a terminal but not when the output
+	is something like a file.
+--
+
+`--abbrev[=<n>]`::
+`--no-abbrev`::
+	Abbreviate the commit hex output. Without _<n>_ it will find the
+	minimum length which can describe the commit uniquely, with some
+	extra slack. Giving _<n>_ specifies the minimum length; a longer
+	length will be used if needed.
++
+Giving `--no-abbrev` will turn off abbreviation, showing the full commit
+hex output.
++
+Note that some pretty formats use `--abbrev`. This behavior can be
+controlled with these two options.
+
+`--date=<format>`::
+	Date format for pretty formats. Note that date atoms like `%aI`
+	are not affected. This option cannot be negated.
++
+include::rev-list-option-date-alternatives.adoc[]
+
 [[io]]
 INPUT AND OUTPUT FORMAT
 -----------------------
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 7d824aa1c5d..0c9014ca594 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -21,6 +21,7 @@
 #include "revision.h"
 #include "notes.h"
 #include "write-or-die.h"
+#include "date.h"
 
 /*
  * One day.  See the 'name a rev shortly after epoch' test in t6120 when
@@ -778,6 +779,8 @@ struct format_rev_data {
 	bool nul_input;
 	bool nul_output;
 	struct string_list notes;
+	struct rev_info rev;
+	int color;
 };
 
 static int format_nul_cb(const struct option *option,
@@ -792,6 +795,17 @@ static int format_nul_cb(const struct option *option,
 	return 0;
 }
 
+static int date_cb(const struct option *option,
+		   const char *arg,
+		   int unset)
+{
+	struct rev_info *data = option->value;
+	parse_date_format(arg, &data->date_mode);
+	data->date_mode_explicit = 1;
+	BUG_ON_OPT_NEG(unset);
+	return 0;
+}
+
 static enum stdin_mode parse_stdin_mode(const char *stdin_mode)
 {
 	if (!strcmp(stdin_mode, "text"))
@@ -805,9 +819,8 @@ static enum stdin_mode parse_stdin_mode(const char *stdin_mode)
 }
 
 static char const *const format_rev_usage[] = {
-	N_("(EXPERIMENTAL!) git format-rev --stdin-mode=<mode> "
-	   "--format=<pretty> [--[no-]notes=<ref>] "
-	   "[-z] [--[no-]null-output] [--[no-]null-input]"),
+	N_("(EXPERIMENTAL!) git format-rev [<options>] "
+	   "--stdin-mode=<mode> --format=<pretty>"),
 	NULL
 };
 
@@ -817,13 +830,13 @@ int cmd_format_rev(int argc,
 		   struct repository *repo UNUSED)
 {
 	struct format_rev_data data = {
-		NULL, NULL, 0, 0, STRING_LIST_INIT_NODUP
+		NULL, NULL, 0, 0, STRING_LIST_INIT_NODUP,
+		REV_INFO_INIT, GIT_COLOR_AUTO
 	};
 	enum stdin_mode stdin_mode;
 	char output_terminator;
 	strbuf_getline_fn getline_fn;
 	struct display_notes_opt format_notes_opt;
-	struct rev_info format_rev = REV_INFO_INIT;
 	struct pretty_format format_pp = { 0 };
 	struct strbuf scratch_buf = STRBUF_INIT;
 	struct command cmd;
@@ -834,6 +847,11 @@ int cmd_format_rev(int argc,
 			   N_("how revs are processed")),
 		OPT_STRING_LIST(0, "notes", &data.notes, N_("notes"),
 				N_("display notes for pretty format")),
+		OPT__ABBREV(&data.rev.abbrev),
+		OPT__COLOR(&data.color, N_("use colored output")),
+		OPT_CALLBACK_F(0, "date", &data.rev, N_("date"),
+			       N_("date format"),
+			       PARSE_OPT_NONEG, date_cb),
 		OPT_CALLBACK_F('z', "null", &data, N_("z"),
 			       N_("use NUL for input and output termination"),
 			       PARSE_OPT_NOARG | PARSE_OPT_NONEG, format_nul_cb),
@@ -862,13 +880,13 @@ int cmd_format_rev(int argc,
 	init_display_notes(&format_notes_opt);
 	stdin_mode = parse_stdin_mode(data.stdin_mode);
 
-	get_commit_format(data.format, &format_rev);
-	format_pp.ctx.rev = &format_rev;
-	format_pp.ctx.fmt = format_rev.commit_format;
-	format_pp.ctx.abbrev = format_rev.abbrev;
-	format_pp.ctx.date_mode_explicit = format_rev.date_mode_explicit;
-	format_pp.ctx.date_mode = format_rev.date_mode;
-	format_pp.ctx.color = GIT_COLOR_AUTO;
+	get_commit_format(data.format, &data.rev);
+	format_pp.ctx.rev = &data.rev;
+	format_pp.ctx.fmt = data.rev.commit_format;
+	format_pp.ctx.abbrev = data.rev.abbrev;
+	format_pp.ctx.date_mode_explicit = data.rev.date_mode_explicit;
+	format_pp.ctx.date_mode = data.rev.date_mode;
+	format_pp.ctx.color = data.color;
 
 	userformat_find_requirements(data.format,
 				     &format_pp.want);
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 7a7c46658a3..2621edb5937 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -1017,4 +1017,48 @@ do
 	'
 done <stdin-modes
 
+format_rev_cmp_log () {
+	opts="$1"
+	format=reference
+	cat >input <<-\EOF &&
+	third
+	second
+	first
+	EOF
+	git -C repo-format log --stdin --no-walk \
+		"$opts" --format="$format" >expect <input &&
+	git -C repo-format format-rev "$opts" \
+		--stdin-mode=revs --format="$format" >actual <input &&
+	test_cmp expect actual
+}
+
+format_rev_err_cmp_log () {
+	opts="$1"
+	format=reference
+	# No input since we ought to fail while parsing options
+	test_must_fail git -C repo-format log --stdin --no-walk \
+		"$opts" --format="$format" 2>expect &&
+	test_must_fail git -C repo-format format-rev "$opts" \
+		--stdin-mode=revs --format="$format" 2>actual &&
+	test_cmp expect actual
+}
+
+test_expect_success 'format-rev --color' '
+	format_rev_cmp_log --color=always &&
+	format_rev_cmp_log --color &&
+	format_rev_cmp_log --no-color &&
+	format_rev_err_cmp_log --color=not-valid
+'
+
+test_expect_success 'format-rev --abbrev' '
+	format_rev_cmp_log --abbrev=31 &&
+	format_rev_cmp_log --no-abbrev
+'
+
+test_expect_success 'format-rev --date' '
+	format_rev_cmp_log --date=relative &&
+	format_rev_cmp_log --date=iso-strict &&
+	format_rev_err_cmp_log --date=not-valid
+'
+
 test_done
-- 
2.54.0.22.g9e26862b904

Re: [PATCH 2/4] format-rev: factor option variables into a struct

[Junio C Hamano]

kristofferhaugsbakk@fastmail.com writes:

> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
>
> We will in two commits add three more options to this command.
> Let’s prepare for that by moving option variables into a struct
> so that we get less local variables.
>
> This allows us to inline `format_nul_data` into this new
> structure. Let’s also rename `stdin_mode_arg` to `stdin_mode`.
> (We couldn’t use `stdin_mode` before because of the enumeration
> with the same name.)
>
> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
> ---
>  builtin/name-rev.c | 44 +++++++++++++++++++++++---------------------
>  1 file changed, 23 insertions(+), 21 deletions(-)
>
> diff --git a/builtin/name-rev.c b/builtin/name-rev.c
> index 254c88199fd..7d824aa1c5d 100644
> --- a/builtin/name-rev.c
> +++ b/builtin/name-rev.c
> @@ -772,16 +772,19 @@ int cmd_name_rev(int argc,
>  	return 0;
>  }
>  
> -struct format_nul_data {
> +struct format_rev_data {
> +	const char *format;
> +	const char *stdin_mode;
>  	bool nul_input;
>  	bool nul_output;
> +	struct string_list notes;
>  };
>  
>  static int format_nul_cb(const struct option *option,
>  			 const char *arg,
>  			 int unset)
>  {
> -	struct format_nul_data *data = option->value;
> +	struct format_rev_data *data = option->value;
>  	data->nul_input = 1;
>  	data->nul_output = 1;
>  	BUG_ON_OPT_NEG(unset);
> @@ -813,31 +816,30 @@ int cmd_format_rev(int argc,
>  		   const char *prefix,
>  		   struct repository *repo UNUSED)
>  {
> -	const char *format = NULL;
> +	struct format_rev_data data = {
> +		NULL, NULL, 0, 0, STRING_LIST_INIT_NODUP
> +	};

It will make it easier to maintain if you used designated
initializer here, i.e.,

	struct format_rev_data data = {
		.notes = STRING_LIST_INIT_NODUP,
	};

The other members not explicitly mentioned by the initializer will
be zero-initialized.

Other parts of the patch look good.

Thanks.