Frank Lichtenheld <frank@xxxxxxxxxxxxxx> writes: > On Mon, Jun 25, 2007 at 04:06:34PM +0200, Johannes Sixt wrote: >> Frank Lichtenheld wrote: >> > >> > Signed-off-by: Frank Lichtenheld <frank@xxxxxxxxxxxxxx> >> >> Please excuse if I'm missing the big picture, but why do we need this >> change? > > - Of course the user or script calling git-config can do the > normalization and error checking, if they want to. But I would > prefer to have it available in git-config. > - I would prefer that these options wouldn't be silently ignored, > because that can be confusing (at least it is documented now, but > still). So we should either using them or error out. I prefer the former. > > Something that I forgot to mention in the previous mail: > One real problem with the patch is that it expands the k,m,g suffixes > for integer values. It probably shouldn't do that. How about doing something like this, then? git_config_int() knows that a missing value is a nonsense and barfs on such an input, so (value ? value : "") is redundant here. Besides, you check value == NULL much earlier in this function. diff --git a/builtin-config.c b/builtin-config.c index 9973f94..33b60ec 100644 --- a/builtin-config.c +++ b/builtin-config.c @@ -148,10 +148,11 @@ char* normalize_value(const char* key, const char* value) if (type == T_RAW) normalized = xstrdup(value); else { - normalized = xmalloc(64); - if (type == T_INT) - sprintf(normalized, "%d", - git_config_int(key, value?value:"")); + normalized = xmalloc(64 + strlen(value)); + if (type == T_INT) { + int v = git_config_int(key, value); + sprintf(normalized, "%d # %s", v, value); + } else if (type == T_BOOL) sprintf(normalized, "%s", git_config_bool(key, value) ? "true" : "false"); - 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