Re: [PATCH 07/14] revert: Introduce struct to keep command-line options

Ramkumar Ramachandra wrote:

In later
steps in this series, we would like to introduce an API function that
calls into this machinery directly and have a way to tell it what to
do.  Hence, introduce a structure to group these variables, so that
the API can take them as a single replay_options parameter.

The variable "me" is left as a file-scope static variable because it
is not an independent option.  "me" is simply a string that needs to
be inferred from the "action" option, and is kept global to save each
function the trouble of determining it independently.

Hm, would it make sense for there to be a "private" section at the
end of the replay_opts struct for variables like this?

Unfortunately, this patch introduces a minor regression.  Parsing
strategy-option violates a C89 rule: Initializers cannot refer to
variables whose address is not known at compile time.  Currently, this
rule is violated by some other parts of Git as well, and it is
possible to get GCC to report these instances using the "-std=c89
-pedantic" option.

I would be interested in fixing that (as a patch on top, maybe).
What do you suggest:

 A. Apply patch 8 and make cmd_revert, cmd_cherry_pick, and parse_args
    manipulate a static "struct replay_opts" while pick_commits et al
    pass around a pointer to it

 B. Make parse_args work like this:

	copy from argument to private static struct replay_opts
	call parse_options()
	copy private static struct replay_opts to argument

 C. Use new option types:

	OPT_BOOL_MEMBER('n', "no-commit",
		offsetof(struct replay_opts, no_commit),
		"don't automatically commit"),

    and teach parse_options to take an additional parameter like it
    takes "prefix" now, to be used as a base address for options that
    write to an offset instead of a pointer

I'm leaning towards A but not sure if that would be wasted work in
light of your plans for these APIs in the long run (i.e., is
parse_args() going to be exposed and want to act on a caller-supplied
struct)?