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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 2014-07-11 at 17:06 -0400, Jeff King wrote:
> 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
> 

ok that makes more sense. I can't really avoid printing the warning at
all since config parsing happens before the option parsing.

I like this wording. I will respin again :D

Thanks,
Jake

> 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


��.n��������+%������w��{.n��������n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]