From: Jeff King <peff@peff.net>
To: Jacob Keller <jacob.e.keller@intel.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 3/3] tag: support configuring --sort via .gitconfig
Date: Fri, 11 Jul 2014 17:06:33 -0400 [thread overview]
Message-ID: <20140711210633.GA12546@sigill.intra.peff.net> (raw)
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:
> + 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
next prev parent reply other threads:[~2014-07-11 21:06 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-11 20:51 [PATCH 1/3] tag: use skip_prefix instead of magic numbers Jacob Keller
2014-07-11 20:51 ` [PATCH 2/3] tag: fix --sort tests to use cat<<-\EOF format Jacob Keller
2014-07-11 20:51 ` [PATCH 3/3] tag: support configuring --sort via .gitconfig Jacob Keller
2014-07-11 20:54 ` Keller, Jacob E
2014-07-11 21:06 ` Jeff King [this message]
2014-07-11 21:08 ` Keller, Jacob E
2014-07-11 22:17 ` Junio C Hamano
2014-07-11 22:30 ` Keller, Jacob E
-- strict thread matches above, loose matches on Subject: below --
2014-07-11 22:55 [PATCH 1/3] tag: use skip_prefix instead of magic numbers Jacob Keller
2014-07-11 22:55 ` [PATCH 3/3] tag: support configuring --sort via .gitconfig Jacob Keller
2014-07-13 3:29 ` Eric Sunshine
2014-07-13 17:10 ` Junio C Hamano
2014-07-13 17:33 ` Jeff King
2014-07-13 18:36 ` Jeff King
2014-07-14 17:17 ` Junio C Hamano
2014-07-15 14:52 ` Keller, Jacob E
2014-07-15 16:03 ` Junio C Hamano
2014-07-15 17:27 ` Keller, Jacob E
2014-07-15 18:17 ` Junio C Hamano
2014-07-15 18:31 ` Keller, Jacob E
2014-07-15 19:12 ` Junio C Hamano
2014-07-15 20:29 ` Keller, Jacob E
2014-07-15 21:31 ` Keller, Jacob E
2014-07-15 19:10 ` Junio C Hamano
2014-07-15 23:38 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140711210633.GA12546@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=jacob.e.keller@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).