git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Keller, Jacob E <jacob.e.keller@intel.com>
Date
Jul 11, 2014, 20:36 UTC
Message-ID
<1405110973.22963.43.camel@jekeller-desk1.amr.corp.intel.com>
In-Reply-To
<20140711182214.GF7856@sigill.intra.peff.net>
On Fri, 2014-07-11 at 14:22 -0400, Jeff King wrote:
Show 28 quoted lines
> On Fri, Jul 11, 2014 at 06:11:08PM +0000, Keller, Jacob E wrote:
> 
> > I personally prefer error out on options, even though it can make it a
> > bit more difficult, though as far as I know unknown fields simply warn
> > or are ignored. (ie: old versions of git just ignore unknown fields in
> > configuration).
> 
> Right, we _have_ to ignore unknown config options, because we
> specifically allow other programs built on git to store their config
> with us (and anyway, our callback style of parsing means that no single
> callback knows about all of the keys).
> 
> In the past we have staked out particular areas of the namespace,
> though. E.g., the diff code said "I own all of color.diff.*, and if you
> put in something I don't understand, I'll complain". That ended up being
> annoying, and now we ignore slots we don't understand there.
> 
> So old gits will always silently ignore tag.sort if they don't know
> about it, and we can't change that. The only thing we can change is:
> 
> > It's possible we should warn instead though, so that older gits work
> > with new sorts that they don't understand.
> 
> Right. I think other config variables in similar situations will barf.
> This is backwards-compatible as long as the new variables are a superset
> (i.e., we only add new understood values, never remove or change the
> meaning of existing values). It's just not forwards-compatible.
> 
So should I respin this so that config option doesn't error out?
Show 10 quoted lines
> > I am ok with warning but I don't know the best practice for how to warn
> > here instead of failing. Returning error causes a fatal "bad config"
> > message. Any thoughts?
> 
> The simplest thing is ignoring the return from parse_sort_string and
> just calling "return 0". That will still say "error:", but continue on.
> If you really want it to say "warning:", I think you'll have to pass a
> flag into parse_sort_string. I'm not sure if it's worth the effort.
> 
> -Peff

Ok this makes sense, I am fine leaving it as error. Should I respin to make it not die though?

Thanks, Jake

Previous: Jeff KingNext: Junio C Hamano
Message 7 of 15 in “tag: use skip_prefix instead of magic numbers”
  1. 1/3 tag: use skip_prefix instead of magic numbersJacob Keller, Jul 11, 2014
  2. 2/3 tag: fix --sort tests to use cat<<-\EOF formatJacob Keller, Jul 11, 2014
  3. 3/3 tag: support configuring --sort via .gitconfigJacob Keller, Jul 11, 2014
  4. Jeff KingJul 11, 2014
  5. Keller, Jacob EJul 11, 2014
  6. Jeff KingJul 11, 2014
  7. Keller, Jacob EJul 11, 2014
  8. Junio C HamanoJul 11, 2014
  9. Keller, Jacob EJul 11, 2014
  10. Jeff KingJul 12, 2014
  11. Junio C HamanoJul 13, 2014
  12. Keller, Jacob EJul 14, 2014
  13. Jeff KingJul 11, 2014
  14. Keller, Jacob EJul 11, 2014
  15. Junio C HamanoJul 11, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.