Re: [PATCH v3 3/5] builtin/branch: clean up action-picking logic in cmd_branch()

Jonathan Tan [off-list ref] writes:

quoted

Incidentally, fix an incorrect usage string that combined the 'list'
usage of git branch (-l) with the 'create' usage; this string has been
incorrect since its inception, a8dfd5eac4 (Make builtin-branch.c use
parse_options., 2007-10-07).

I think that we implement such incidental fixes only when we're touching
the relevant lines, but this change looks correct.

That's fair. This fix is such low-hanging fruit that I don't think it
deserves its patch, but if others agree, I'll separate it.

quoted

-	int delete = 0, rename = 0, copy = 0, force = 0, list = 0;
-	int show_current = 0;
-	int reflog = 0, edit_description = 0;
-	int quiet = 0, unset_upstream = 0;
+	/* possible actions */
+	int delete = 0, rename = 0, copy = 0, force = 0, list = 0,
+	    unset_upstream = 0, show_current = 0, edit_description = 0;
+	int noncreate_actions = 0;
+	/* possible options */
+	int reflog = 0, quiet = 0, icase = 0;

[snip]

quoted

-	if (!!delete + !!rename + !!copy + !!new_upstream + !!show_current +
-	    list + edit_description + unset_upstream > 1)
+	noncreate_actions = !!delete + !!rename + !!copy + !!new_upstream +
+			    !!show_current + !!list + !!edit_description +
+			    !!unset_upstream;
+	if (noncreate_actions > 1)
 		usage_with_options(builtin_branch_usage, options);

Overall this change looks good, although if you're going to rearrange
the variable declarations (e.g. the positions of show_current,
edit_description, and unset_upstream have moved), you might as well make
them consistent with the noncreate_actions statement, I guess. Also
maybe move new_upstream closer.

Yeah this is obviously inconsistent, thanks for the catch.

* force isn't an action
* new_upstream is an action
* for QoL, all of the actions should be listed in the same order at both
  sites