Re: [PATCH v2] config: Use parseopt.
- From
Felipe Contreras <felipe.contreras@gmail.com>
- Date
- Feb 17, 2009, 00:50 UTC
- Message-ID
- <94a0d4530902161650ld10602dxdb73b78d5d9a6223@mail.gmail.com>
- In-Reply-To
- <alpine.DEB.1.00.0902152228530.10279@pacific.mpi-cbg.de>
On Sun, Feb 15, 2009 at 11:29 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
Show 39 quoted lines
> 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.Ok, dropping the patch.
-- Felipe Contreras