Re: [PATCH 03/11] revert: Introduce a struct to parse command-line options into

Hi Jonathan,

Jonathan Nieder writes:

Ramkumar Ramachandra wrote:

quoted

--- a/builtin/revert.c
+++ b/builtin/revert.c

@@ -35,17 +35,27 @@ static const char * const cherry_pick_usage[] = {
 	NULL
 };
 
-static int edit, no_replay, no_commit, mainline, signoff, allow_ff;
-static enum { REVERT, CHERRY_PICK } action;
-static int commit_argc;
-static const char **commit_argv;
-static int allow_rerere_auto;
 
+static struct {
+	enum { REVERT, CHERRY_PICK } action;

That would require giving this struct a name, so it can be passed
around.  Not a bad idea anyway imho since then a person reading
top-to-bottom is not left in suspense:

 struct cherry_pick_opts {
	enum { REVERT, CHERRY_PICK } action;

	unsigned edit:1;
	unsigned no_replay:1;
	...
 };

Thanks.  A couple of nits:
1. You can't take the address of a bitfield, so 'edit' and 'replay'
   can't be passed to the command line argument parsing framework
   directly.
2. GCC throws a "warning: useless storage class specifier in empty
   declaration" for this kind of declaration.

-- Ram