Re: [PATCH 3/3] tag: support configuring --sort via .gitconfig

From: Jeff King <peff@peff.net>
Date: Jul 11, 2014, 21:06 UTC
Message-ID: <20140711210633.GA12546@sigill.intra.peff.net>
In-Reply-To: <1405111895-17451-3-git-send-email-jacob.e.keller@intel.com>

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

Show 8 quoted lines

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