On 07/20/2016 11:08 AM, Johannes Schindelin wrote:
On Tue, 19 Jul 2016, Jeff Hostetler wrote:
diff --git a/builtin/commit.c b/builtin/commit.c
+       } else if (arg) {
+               int n = strtol(arg, NULL, 10);
+               if (n == 1)
+                       *value = STATUS_FORMAT_PORCELAIN;
+               else
+                       die("unsupported porcelain version");
+       } else {
+               *value = STATUS_FORMAT_PORCELAIN;

This could be folded into the previous conditional:

        }
        else {
                int n = arg ? strtol(arg, NULL, 10) : 1;
                ...


I did it this way because I didn't want to make any assumptions
here on the numeric value of the enum values.  Or rather, I didn't
want to add specific assignments to the enum type.

This also helps make it easier to see my later commit:
        else if (n == 2) *value = STATUS_FORMAT_PORCELAIN_V2;

Also, I didn't want to alter the order of assignments to the global
status_format variable.  That is, if I capture the value of <n> in
a porcelain_version variable and assign that to status_format
afterwards, there is opportunity for mistakes if they type:
        git status --short --porcelain=1 --long
where the status_format variable is assigned 3 times.

Having said this, ...

@@ -1336,9 +1347,9 @@ int cmd_status(int argc, const char **argv, const char 
*prefix)
                            N_("show status concisely"), STATUS_FORMAT_SHORT),
                OPT_BOOL('b', "branch", &s.show_branch,
                         N_("show branch information")),
-               OPT_SET_INT(0, "porcelain", &status_format,
-                           N_("machine-readable output"),
-                           STATUS_FORMAT_PORCELAIN),
+               { OPTION_CALLBACK, 0, "porcelain", &status_format,
+                 N_("version"), N_("machine-readable output"),
+                 PARSE_OPT_OPTARG, opt_parse_porcelain },

How about using a COUNTUP here instead? We could then set the status
format afterwards, like this:

        if (porcelain == 0)
                status_format = STATUS_FORMAT_UNSPECIFIED;
        else {
                status_format = STATUS_FORMAT_PORCELAIN;
                if (porcelain > 1)
                        warning("No porcelain v%d; falling back to v1",
                                porcelain);
        }


Maybe I misread the COUNTUP docs, but it looked like it would
allow "--porcelain --porcelain", but not "--porcelain=2".

Jeff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to