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

From: Jeff King <peff@peff.net>
Date: Jul 9, 2014, 21:58 UTC
Message-ID: <20140709215816.GE25854@sigill.intra.peff.net>
In-Reply-To: <1404940860-4004-1-git-send-email-jacob.e.keller@intel.com>

On Wed, Jul 09, 2014 at 02:21:00PM -0700, Jacob Keller wrote:

> Add support for configuring default sort ordering for git tags. Command
> line option will override this configured value, using the exact same
> syntax.

This makes sense, and was something I was expecting to add once tag and branch both learned for-each-ref's sorting code. I don't think there will be any compatibility problems in adding it now, though; the ultimate interface will be the same (it will just know about more types of sorting).

Show 6 quoted lines

> +tag.sort::
> +	This variable is used to control the sort ordering of tags when
> +	displayed via linkgit:git-tag[5]. The current supported types are
> +	"refname" for lexicographic order (default) or "version:refname" or
> +	"v:refname" for tag names treated as versions. You may prepend a "-" to
> +	reverse the sort ordering.

Your link to git-tag[5] should be to git-tag[1], I think. The final number is the manpage section.

I was going to complain that this is duplicating the sort documentation from git-tag, but I see you actually moved it from the --sort option to here, and refer back from there to here.

I think I'd prefer it the other way around (and explain this as "behave as if --sort=<value> had been given"). As the sort options grow, it seems likely that we will grow a new section in the git-tag manpage (and eventually probably feed that content from an include that works for all of for-each-ref, branch, and tag).

> +static char *configured_tag_sort;

When dealing with a config option that also has a command-line version, we usually forgo this separate storage for the configured value. Instead, we just set "sort" directly from the config callback (which may require making it a global so the callback can access it), and then let the command-line parser overwrite it.

-Peff