Patrick Steinhardt <ps@xxxxxx> writes:
> We're about to introduce a output format though that is intended to be
> parseable by machines, for example inside of a script. In that case it
> becomes a bit awkward of an interface if you have to call git-fetch(1)
> with the `fetch.output` config key set. We're thus going to introduce a
> new `--output-format` switch for git-fetch(1) so that the output format
> can be configured more directly.
Good. I was wondering about that code in the context of the
previous patch, especially the error message had a hard-coded
assumption that it comes from a configuration variable.
And the changes to display_state_init() to lift the responsibility
of finding and validating .format out of the function, and the
changes to intermediate functions to pass the .format through the
callchain, are all expected and there was nothing questionable.
> @@ -2157,6 +2149,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
> {
> int i;
> const char *bundle_uri;
> + enum display_format display_format = DISPLAY_FORMAT_UNKNOWN;
> struct string_list list = STRING_LIST_INIT_DUP;
> struct remote *remote = NULL;
> int result = 0;
> @@ -2183,6 +2176,19 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
> argc = parse_options(argc, argv, prefix,
> builtin_fetch_options, builtin_fetch_usage, 0);
>
> + if (display_format == DISPLAY_FORMAT_UNKNOWN) {
> + const char *format = "full";
> +
> + git_config_get_string_tmp("fetch.output", &format);
> + if (!strcasecmp(format, "full"))
> + display_format = DISPLAY_FORMAT_FULL;
> + else if (!strcasecmp(format, "compact"))
> + display_format = DISPLAY_FORMAT_COMPACT;
> + else
> + die(_("invalid value for '%s': '%s'"),
> + "fetch.output", format);
> + }
OK, but isn't the usual way to do this to have configuration parser
before parse_options() and then let parse_options() override
whatever display_format set by it?
That way, we do not have to worry about DISPLAY_FORMAT_UNKNOWN at
all. Just initialize the variable to whatever default format at the
beginning of this function, read "fetch.output" to override it if
the configuration exists, and then let parse_options() to handle
"--output-format" or "--porcelain" or whatever to further override
it.
Thanks.
