On Fri, Jul 11, 2014 at 01:51:35PM -0700, Jacob Keller wrote:

> +     if (!strcmp(var, "tag.sort")) {
> +             if (!value)
> +                     return config_error_nonbool(var);
> +             status = parse_sort_string(value, &tag_sort);
> +             if (status) {
> +                     warning("using default lexicographic sort order");
> +                     tag_sort = STRCMP_SORT;
> +             }

I think this is a good compromise of the issues we discussed earlier. As
you noted, it should probably be marked for translation. I'm also not
sure the message content is clear in all situations. If I have a broken
tag.sort, I get:

  $ git config tag.sort bogus
  $ git tag >/dev/null
  error: unsupported sort specification bogus
  warning: using default lexicographic sort order

Not too bad, though reminding me that the value "bogus" came from
tag.sort would be nice. But what if I override it:

  $ git tag --sort=v:refname >/dev/null
  error: unsupported sort specification bogus
  warning: using default lexicographic sort order

That's actively wrong, because we are using v:refname from the
command-line. Perhaps we could phrase it like:

  warning: ignoring invalid config option tag.sort

or similar, which helps both cases.

As an aside, I also think the error line could more clearly mark the
literal contents of the variable. E.g., one of:

  error: unsupported sort specification: bogus

or

  error: unsupported sort specification 'bogus'

-Peff
--
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