On Fri, 2014-07-11 at 14:22 -0400, Jeff King wrote:
> 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?
> > 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.
Ok this makes sense, I am fine leaving it as error. Should I respin to
make it not die though?