diff mbox series

[7/8] fetch: introduce new `--output-format` option

Message ID 3b2cad066a3b3446fc335d6944a62bf79b0779bb.1681906949.git.ps@pks.im (mailing list archive)
State Superseded
Headers show
Series fetch: introduce machine-parseable output | expand

Commit Message

Patrick Steinhardt April 19, 2023, 12:31 p.m. UTC
It is only possible to configure the output format that git-fetch(1)
uses by setting it via a config key. While this interface may be fine as
long as we only have the current "full" and "compact" output formats,
where it is unlikely that the user will have to change them regularly.
But we're about to introduce a new machine-parseable interface where the
current mechanism feels a little bit indirect and rigid.

Introduce a new `--output-format` option that allows the user to change
the desired output format more directly.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Documentation/fetch-options.txt |  5 ++++
 builtin/fetch.c                 | 17 +++++++++++
 t/t5574-fetch-output.sh         | 50 +++++++++++++++++++++++++--------
 3 files changed, 60 insertions(+), 12 deletions(-)

Comments

Glen Choo April 26, 2023, 7:40 p.m. UTC | #1
Patrick Steinhardt <ps@pks.im> writes:

> @@ -2101,6 +2116,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
>  			    PARSE_OPT_OPTARG, option_fetch_parse_recurse_submodules),
>  		OPT_BOOL(0, "dry-run", &dry_run,
>  			 N_("dry run")),
> +		OPT_CALLBACK(0, "output-format", &display_format, N_("format"), N_("output format"),
> +			     opt_parse_output_format),
>  		OPT_BOOL(0, "write-fetch-head", &write_fetch_head,
>  			 N_("write fetched references to the FETCH_HEAD file")),
>  		OPT_BOOL('k', "keep", &keep, N_("keep downloaded pack")),

This change is good enough for fetching from a single remote, but if we
want to support "--all", we'd also need to propagate the CLI flag to the
child "fetch" processes. (The config option wouldn't have this bug
because the child processes would parse config and get the correct
value.)
Patrick Steinhardt April 27, 2023, 10:58 a.m. UTC | #2
On Wed, Apr 26, 2023 at 12:40:13PM -0700, Glen Choo wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > @@ -2101,6 +2116,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
> >  			    PARSE_OPT_OPTARG, option_fetch_parse_recurse_submodules),
> >  		OPT_BOOL(0, "dry-run", &dry_run,
> >  			 N_("dry run")),
> > +		OPT_CALLBACK(0, "output-format", &display_format, N_("format"), N_("output format"),
> > +			     opt_parse_output_format),
> >  		OPT_BOOL(0, "write-fetch-head", &write_fetch_head,
> >  			 N_("write fetched references to the FETCH_HEAD file")),
> >  		OPT_BOOL('k', "keep", &keep, N_("keep downloaded pack")),
> 
> This change is good enough for fetching from a single remote, but if we
> want to support "--all", we'd also need to propagate the CLI flag to the
> child "fetch" processes. (The config option wouldn't have this bug
> because the child processes would parse config and get the correct
> value.)

Oh, right, good catch. Will fix, thanks!

Patrick
diff mbox series

Patch

diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index 622bd84768..654f96f79d 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -78,6 +78,11 @@  linkgit:git-config[1].
 --dry-run::
 	Show what would be done, without making any changes.
 
+--output-format::
+	Control how ref update status is printed. Valid values are
+	`full` and `compact`. Default value is `full`. See section
+	OUTPUT in linkgit:git-fetch[1] for detail.
+
 ifndef::git-pull[]
 --[no-]write-fetch-head::
 	Write the list of remote refs fetched in the `FETCH_HEAD`
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 81581b0033..22ba75a958 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -2050,6 +2050,21 @@  static int fetch_one(struct remote *remote, int argc, const char **argv,
 	return exit_code;
 }
 
+static int opt_parse_output_format(const struct option *opt, const char *arg, int unset)
+{
+	enum display_format *format = opt->value;
+	if (unset || !arg)
+		return 1;
+	else if (!strcmp(arg, "full"))
+		*format = DISPLAY_FORMAT_FULL;
+	else if (!strcmp(arg, "compact"))
+		*format = DISPLAY_FORMAT_COMPACT;
+	else
+		return error(_("unsupported output format '%s'"), arg);
+
+	return 0;
+}
+
 int cmd_fetch(int argc, const char **argv, const char *prefix)
 {
 	const char *bundle_uri;
@@ -2101,6 +2116,8 @@  int cmd_fetch(int argc, const char **argv, const char *prefix)
 			    PARSE_OPT_OPTARG, option_fetch_parse_recurse_submodules),
 		OPT_BOOL(0, "dry-run", &dry_run,
 			 N_("dry run")),
+		OPT_CALLBACK(0, "output-format", &display_format, N_("format"), N_("output format"),
+			     opt_parse_output_format),
 		OPT_BOOL(0, "write-fetch-head", &write_fetch_head,
 			 N_("write fetched references to the FETCH_HEAD file")),
 		OPT_BOOL('k', "keep", &keep, N_("keep downloaded pack")),
diff --git a/t/t5574-fetch-output.sh b/t/t5574-fetch-output.sh
index 55f0f05b6a..5144d5ed21 100755
--- a/t/t5574-fetch-output.sh
+++ b/t/t5574-fetch-output.sh
@@ -24,14 +24,37 @@  test_expect_success 'fetch with invalid output format configuration' '
 	test_cmp expect actual
 '
 
+test_expect_success 'fetch with invalid output format via command line' '
+	test_must_fail git fetch --output-format >actual 2>&1 &&
+	cat >expect <<-EOF &&
+	error: option \`output-format${SQ} requires a value
+	EOF
+	test_cmp expect actual &&
+
+	test_must_fail git fetch --output-format= origin >actual 2>&1 &&
+	cat >expect <<-EOF &&
+	error: unsupported output format ${SQ}${SQ}
+	EOF
+	test_cmp expect actual &&
+
+	test_must_fail git fetch --output-format=garbage origin >actual 2>&1 &&
+	cat >expect <<-EOF &&
+	error: unsupported output format ${SQ}garbage${SQ}
+	EOF
+	test_cmp expect actual
+'
+
 test_expect_success 'fetch aligned output' '
-	git clone . full-output &&
+	test_when_finished "rm -rf full-cfg full-cli" &&
+	git clone . full-cfg &&
+	git clone . full-cli &&
 	test_commit looooooooooooong-tag &&
-	(
-		cd full-output &&
-		git -c fetch.output=full fetch origin >actual 2>&1 &&
-		grep -e "->" actual | cut -c 22- >../actual
-	) &&
+
+	git -C full-cfg -c fetch.output=full fetch origin >actual-cfg 2>&1 &&
+	git -C full-cli fetch --output-format=full origin >actual-cli 2>&1 &&
+	test_cmp actual-cfg actual-cli &&
+
+	grep -e "->" actual-cfg | cut -c 22- >actual &&
 	cat >expect <<-\EOF &&
 	main                 -> origin/main
 	looooooooooooong-tag -> looooooooooooong-tag
@@ -40,13 +63,16 @@  test_expect_success 'fetch aligned output' '
 '
 
 test_expect_success 'fetch compact output' '
-	git clone . compact &&
+	test_when_finished "rm -rf compact-cfg compact-cli" &&
+	git clone . compact-cli &&
+	git clone . compact-cfg &&
 	test_commit extraaa &&
-	(
-		cd compact &&
-		git -c fetch.output=compact fetch origin >actual 2>&1 &&
-		grep -e "->" actual | cut -c 22- >../actual
-	) &&
+
+	git -C compact-cfg -c fetch.output=compact fetch origin >actual-cfg 2>&1 &&
+	git -C compact-cli fetch --output-format=compact origin >actual-cli 2>&1 &&
+	test_cmp actual-cfg actual-cli &&
+
+	grep -e "->" actual-cfg | cut -c 22- >actual &&
 	cat >expect <<-\EOF &&
 	main       -> origin/*
 	extraaa    -> *