Re: [PATCHv3 0/9] Removing deprecated parsing macros

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 6, 2013, 17:20 UTC
Message-ID: <7viozihh26.fsf@alter.siamese.dyndns.org>
In-Reply-To: <5200F3D6.5040004@googlemail.com>

Stefan Beller <stefanbeller@googlemail.com> writes:

Show 13 quoted lines

> On 08/06/2013 08:39 AM, Junio C Hamano wrote:
>> Thanks.  Queued this at the tip of 'pu'.  There seem to be some
>> fallouts found in the test suite, though.
>> 
>
> Thanks. I am sorry for forgetting 'make test' before sending patches.
> And the test suite is correct.
>
> e35ea450 (branch, commit, name-rev: ease up boolean conditions)
> is faulty. In builtin/branch.c the variables 'delete' and 'rename' 
> are not used as a true boolean but I assumed so. 
> These are parsed in via OPT_BIT and depending if you pass -d or -D 
> for deletion you have value 1 or 2 in delete.

Likewise for 'rename' that becomes 2 when -M is given.

> Hence this change is incorrect:
> -	if (!!delete + !!rename + !!force_create + !!list + !!new_upstream + !!unset_upstream > 1)
> +	if (delete + rename + force_create + list + unset_upstream +
> +	    !!new_upstream > 1)

As a follow-up to this series, we may want to think about the following as well:

 - OPT__VERBOSE() is defined in terms of OPT_BOOLEAN().  I think we
   use it to represent increasing levels of verbosity, so we cannot
   turn this into OPT_BOOL().  Its implementation has to become
   OPT_COUNTUP().

 - OPT__QUIET() is defined in terms of OPT_BOOLEAN().  I offhand do
   not know if we have increasing levels of quietness.  The users
   need auditing before we can decide to turn this into either
   OPT_COUNTUP() or OPT_BOOL().

 - OPT__DRY_RUN() should probably become OPT_BOOL().

 - OPT_FORCE(); do we have levels of forcefulness?  If so
   OPT_COUNTUP(), otherwise OPT_BOOL().