Re: [PATCH 5/8] revert: Catch incompatible command-line options early
flat view
From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:51:13
Ramkumar Ramachandra wrote:
Earlier, incompatible command-line options used to be caught in pick_commits after parse_args has parsed the options and populated the options structure; a lot of unncessary work has already been done, and significant amount of cleanup is required to die at this stage. Instead, hand over this responsibility to parse_args so that the program can die early.
Looking at the patch, this seems like a bugfix (error messages currently say "cherry-pick: " when they should sometimes say "revert: ") and cleanup (dealing with options incompatible with "--ff" in a loop instead of one by one) in addition to the "check and die early" improvement you explain above.
quoted hunk ↗ jump to hunk
--- a/builtin/revert.c +++ b/builtin/revert.c@@ -80,10 +80,29 @@ static int option_parse_x(const struct option *opt, return 0; } +static void die_opt_incompatible(const char *me, const char *base_opt, ...) +{ + const char *this_opt; + int this_opt_set; + va_list ap; + + va_start(ap, base_opt); + while (1) { + if (!(this_opt = va_arg(ap, const char *))) + break; + if ((this_opt_set = va_arg(ap, int))) + die(_("%s: %s cannot be used with %s"), + me, this_opt, base_opt); + } + va_end(ap); +}
Wait a second --- this doesn't always die! Why is it called
die_opt_incompatible rather than verify_opt_compatible_or_die or
something?
I think I would have written the loop something like
va_start(ap, opt1);
while ((opt2 = va_arg(ap, const char *))) {
int set = va_arg(ap, int);
if (set)
die(opt1 cannot be used with opt2);
}
va_end(ap);
Thanks. The refactoring into a loop is nice.