Christian Couder <chriscool@xxxxxxxxxxxxx> writes: > This is needed because we want to reuse the parse_args() function > so that we can parse options saved in a TODO file. This probably is not _needed_ but something you thought would be a good thing to do while at it. You forgot to mention something more important, though. You added two extra arguments to revert_or_cherry_pick, neither of which I agree with. * it is a regression to call the first extra argument "int revert"; (action == REVERT) was more readable. * "int edit" is ill thought out; it is about giving the default of "edit" to revert_or_cherry_pick() depending on what action it is going to take. In this particular case, the logic for the default of "edit" is trivial and localized (it is 0 unless we are interactive revert), so I would drop the argument and have default logic immediately after "memset(&info, 0, sizeof(info))". If there were many such args-info elements whose default have to be different depending on the action, the caller should be passing an instance of "struct args_info" with the default, and have the parser to update the default supplied. I don't think it is warranted in this case. By the way, "infos" is an eyesore at least to me. Any data you work on is information, naming a variable "struct args_info info", unless it primarily works on that "args_info" and not any other kinds of info (like "struct rev_info revs"), is like calling a variable "var", adding _no_ useful information. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html