Re: [PATCH 02/14] pull: pass verbosity, --progress flags to fetch and merge

From: Paul Tan <pyokagan@gmail.com>
Date: May 22, 2015, 13:38 UTC
Message-ID: <CACRoPnRR60DK7tE7yenSX0wouGSQDMDpcphjCdCr-GGdkfPrYQ@mail.gmail.com>
In-Reply-To: <f90387a786c1e0f4287c9fee405f8e2f@www.dscho.org>

Hi Johannes,

On Thu, May 21, 2015 at 11:59 PM, Johannes Schindelin <johannes.schindelin@gmx.de> wrote:

Show 9 quoted lines

> Hi Paul,
>
> On 2015-05-21 11:48, Paul Tan wrote:
>> Unfortunately, the usage of strbuf means that we lose the ability to
>> know if an option was not provided at all (the value is NULL). This is
>> important as some of these options are used to override the default
>> configured behavior.
>
> Would this not be implied by the strbuf's len being 0?

You're right >.< I think I need more coffee as well ;-)

It would mean adding STRBUF_INIT's to all of the option variables, but I don't think it is a problem. strbufs it is, then.

Show 7 quoted lines

>>> You might also want to verify that arg is `NULL` when `unset != 0`. Something like this:
>>
>> Hmm, would there be a situation where arg is NULL when `unset != 0`?
>
> I forgot to say that my suggestion was purely meant as defensive coding. Yes, `arg` *should* be `NULL` when `unset != 0`. Should ;-)
>
> By the way, just to make sure: My comments and suggestions are no demands; you should feel very free to reject them when you feel that your code is better without the suggested changes. I am just trying to be helpful.

Thanks, your comments have been really helpful. I really do appreciate it :).

Regards, Paul