From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Sun, 15 Feb 2009 21:29:02 GMT
Subject: Re: [PATCH v2] config: Use parseopt.
Message-ID: <alpine.DEB.1.00.0902152228530.10279@pacific.mpi-cbg.de>
In-Reply-To: <7vzlgnieei.fsf@gitster.siamese.dyndns.org>

Hi,

On Sun, 15 Feb 2009, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > On Sun, 15 Feb 2009, Junio C Hamano wrote:
> >
> >> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >> 
> >> >> > Or maybe even better: set a variable "ret" and at the end of 
> >> >> > cmd_config(), "return !!ret;"?
> >> >> 
> >> >> Huh? So git commands don't return negative error values?
> >> >
> >> > AFAICT an exit status is supposed to be between 0 and 127.
> >> 
> >> Are you two talking about the return value from cmd_config()?
> >> 
> >> git.c::run_builtin() already knows what to do with status codes from the
> >> builtins to protect you from (rare) shells that do not cope with a
> >> negative return that come from the common pattern of doing:
> >> 
> >> 	return error("it is wrong in this way")
> >> 
> >> So "negative" is not really a problem.
> >
> > Ooops.  I missed that.
> 
> Note that I refrained from using stronger words like "regression" on
> purpose, because I do not think any caller tells various error codes that
> come out of git_config_set_multivar() apart and act differently in
> practice.
> 
> But it does appear that the said function wants to say why the call failed
> with its return code, and using !!ret to lose information does not feel
> right.

I fully agree.

Ciao,
Dscho