On Tue, Jul 19, 2016 at 06:10:53PM -0400, Jeff Hostetler wrote:
> +static int opt_parse_porcelain(const struct option *opt, const char *arg, int unset)
> +{
> + enum wt_status_format *value = (enum wt_status_format *)opt->value;
> + if (unset) {
> + *value = STATUS_FORMAT_UNSPECIFIED;
Nice attention to detail here and below in handling "unset" and "!arg"
cases. I think should be STATUS_FORMAT_NONE, though, which is what the
old code used to do (since "0" is the usual special value for --no-*
options). It only matters if you do:
git status --no-porcelain
Right now that will switch to the long format, regardless of your
config. With your path it defaults to any configured value. It's
probably a case that nobody hits ever, but in the absence of a good
reason to do otherwise, I'd stick with the current behavior.
> + } else if (arg) {
> + int n = strtol(arg, NULL, 10);
> + if (n == 1)
> + *value = STATUS_FORMAT_PORCELAIN;
> + else
> + die("unsupported porcelain version");
This silently allows:
git status --porcelain="1 for the money"
and later:
git status --porcelain="2 for the show"
Probably not a big deal in practice, but since the list of formats is
constrained, we don't really care about parsing arbitrary numbers.
So:
if (!strcmp(arg, "1"))
*value = STATUS_FORMAT_PORCELAIN;
is actually simpler, and more robust.
I also wondered if:
git status --porcelain=v1
is more self-documenting about the meaning of "1". It's purely
aesthetics, but it somehow looks better to me. Matching that is also
much easier with pure strcmps.
> @@ -1381,6 +1392,8 @@ int cmd_status(int argc, const char **argv, const char *prefix)
>
> s.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;
> s.ignore_submodule_arg = ignore_submodule_arg;
> + s.status_format = status_format;
> +
I wonder what happens if you pass a "wt_status" with a format of "SHORT"
to the long-formatting code.
I think it is ignored completely, as you are just now introducing the
s.status_format field. But I wonder if there is room for further cleanup
in pushing the big switch statements from run_status() and cmd_status()
into wt-status.c.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html