On Fri, Jul 11, 2014 at 10:24:07AM -0700, Jacob Keller wrote:

> Updated to include changes due to Junio's feedback. This has not resolved
> whether we should fail on a configuration error or simply warn. It appears 
> that
> we actually seem to error out more than warn, so I am unsure what the correct
> action is here.

Yeah, we're quite inconsistent there. In some cases we silently ignore
something unknown (e.g., a color.diff.* slot that we do not understand),
but in most cases if it is a config key we understand but a value we do
not, we complain and die.

It's probably user-unfriendly to be silent for those cases, though. The
user gets no feedback on why their config value is doing nothing.

I tend to think that warning is not much better than erroring out. It is
helpful if you are running a single-shot of an old version (which is
something that I do a lot when testing old versions), but would quickly
become irritating if you were actually using an old version of git

I dunno. Maybe it is worth making life easier for people in the former

> +static int parse_sort_string(const char *arg, int *sort)
> +{
> +     int type = 0, flags = 0;
> +
> +     if (skip_prefix(arg, "-", &arg))
> +             flags |= REVERSE_SORT;
> +
> +     if (skip_prefix(arg, "version:", &arg) || skip_prefix(arg, "v:", &arg))
> +             type = VERCMP_SORT;
> +     else
> +             type = STRCMP_SORT;
> +
> +     if (strcmp(arg, "refname"))
> +             return error(_("unsupported sort specification %s"), arg);
> +
> +     *sort = (type | flags);
> +
> +     return 0;
> +}

Regardless of how we handle the error, I think this version that
assembles the final bitfield at the end is easier to read than the

To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to