Re: [PATCH v2] config: Use parseopt.

On Sun, Feb 15, 2009 at 11:29 PM, Johannes Schindelin
[off-list ref] wrote:

Hi,

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

quoted

Johannes Schindelin [off-list ref] writes:

quoted

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

quoted

Johannes Schindelin [off-list ref] writes:

quoted

quoted

quoted

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