On Thu, Apr 18, 2019 at 10:31:30AM -0400, Jeff Hostetler wrote: > Currently, neither function looks at any other k/v pairs, so > this is a bit of a moot point, but I'm wondering if this should > look like this: > > int add_config(...) > { > // give add-interactive.c a chance to look at k/v pair, but > // do not short-cut because we don't know yet whether we > // will be interactive or not yet. > (void)add_i_config(...); > > ...ignore_add_errors... > ...use_builtin_add_i... > > return git_default_config(...); > } Yeah, I agree this split seems a bit more natural. It is worth propagating errors from add_i_config(), though, like: if (add_i_config(var, value, data)) return -1; so that any key-specific errors (e.g., config_error_nonbool) stop the parsing in the usual way. -Peff