Tanay Abhra <tanayabh@xxxxxxxxx> writes: > I was aping the old git_config() system, it also does exactly what you described > above. for example, builtin/gc.c line 91, > > if (!strcmp(var, "gc.pruneexpire")) { > if (value && strcmp(value, "now")) { > unsigned long now = approxidate("now"); > if (approxidate(value) >= now) > return error(_("Invalid %s: '%s'"), var, value); > } > > would print, > error: Invalid gc.pruneexpire: 'value' > fatal: bad config variable 'gc.pruneexpire' at file line 15 in .git/config It's good to do at least as well as the old system, but I agree with Junio that it's suboptimal. Having a single API call to replace error("'%s' must be between 1 and 3"); git_config_die(key); with stg like: git_config_die(key, "'%s' must be between 1 and 3"); in Junio's example would allow git_config_die to format the error message the way it likes (i.e. it can be the same as before when you introduce it, and improve afterwards). I've never been disturbed by the quality of Git's error messages wrt config files (it's not a compiler!), so having good quality messages is not a high priority IMHO, but having a good API that as a side effect can produce good error messages is important. If changing the error messages requires rewritting all callers later, then we've missed the point doing the refactoring now. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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