From: Keller, Jacob E <jacob.e.keller@intel.com>
Date: Fri, 11 Jul 2014 17:20:18 GMT
Subject: Re: [PATCH 2/2 v4] tag: support configuring --sort via .gitconfig
Message-ID: <1405099217.22963.11.camel@jekeller-desk1.amr.corp.intel.com>
In-Reply-To: <xmqqion4543l.fsf@gitster.dls.corp.google.com>

On Fri, 2014-07-11 at 08:04 -0700, Junio C Hamano wrote:
> Jacob Keller <jacob.e.keller@intel.com> writes:
> 

> > +
> > +	if (strcmp(arg, "refname"))
> > +		die(_("unsupported sort specification %s"), arg);
> 
> Hmm.  I _thought_ we try to catch unsupported option value coming
> from the command line and die but at the same time we try *not* to
> die but warn and whatever is sensible when the value comes from the
> configuration, so that .git/config or $HOME/.gitconfig can be shared
> by those who use different versions of Git.
> 
> Do we already have many precedences where we see configuration value
> that our version of Git do not yet understand?
> 
> Not a very strong objection; just something that worries me.

Based on a cursory glance at how the config is parsed, we actually
die_on_error for most options. Does it makes sense to warn or not? I am
not really sure here.. At any rate I updated this to be a bit cleaner
and use error( instead of die, so that it will work within how config is
supposed to.. Plus, this also makes it so that --sort shows the usage if
it's invalid.

Please comment more on the new set I am sending so we can really
determine what the correct course of action is here.

Regards,
Jake