Junio C Hamano <gitster@xxxxxxxxx> writes: > For example, it ought to be sufficient, for the purpose of improved > readability, to instead doing this > >> static enum parse_opt_result usage_with_options_internal(struct parse_opt_ctx_t *, >> const char * const *, >> const struct option *, >> - int, int); >> + enum usage_style, >> + enum usage_output); > > just do > > int full_usage, > int usage_to_stderr); > > here. Dropping the parameter names in the function prototype is > allowed, and we encourage to do so in our codebase but _only_ when > the meaning of each parameter is obvious from their type. The first > 3 parameters we see above are of distinct types and except for the > second one being the usage string given to the users, they should be > obvious. But the last two unnamed integers are not obvious and they > should have been spelled out---otherwise a developer who is adding > a new callsite cannot work from the prototype alone and has to go to > the implementation to figure out what to pass. > > Adding two enums for this is a bit overkill, but is OK here locally. And with that lessor impact change, you could still add a smaller change to help callers (you do not need any change to the callee, which uses biased parameter names for these two), if you wanted to (though as I said this is internal implementation detail of the parse_options API, so you do not have to). For example, you could do #define USAGE_FULL 1 #define USAGE_TO_STDERR 1 without anything else and do >> @@ -1088,7 +1099,7 @@ enum parse_opt_result parse_options_step(struct parse_opt_ctx_t *ctx, >> } >> >> if (internal_help && !strcmp(arg + 2, "help-all")) >> - return usage_with_options_internal(ctx, usagestr, options, 1, 0); >> + return usage_with_options_internal(ctx, usagestr, options, style_full, to_out); return usage_with_options_internal(ctx, usagestr, options, USAGE_FULL, 0); which may make it easier to follow. The point is that you be more verbose only when you do a non-standard thing. And without enum, of course you do not need any change like below. > One way to reduce this churn is to do > > int err = (to_where == to_stderr); > int full = (help_style == style_full); > > at the very beginning of the function. Then you do not have to > change the body of the function harder to read at all.
