Re: [GSoC PATCH v2] object-name: turn INTERPRET_BRANCH_* constants into enum values

[Junio C Hamano <gitster@pobox.com>]

Karthik Nayak <karthik.188@gmail.com> writes:

>> diff --git a/object-name.h b/object-name.h
>> index cda4934cd5..167a9154ea 100644
>> --- a/object-name.h
>> +++ b/object-name.h
>> @@ -101,9 +101,12 @@ int set_disambiguate_hint_config(const char *var, const char *value);
>>   * If the input was ok but there are not N branch switches in the
>>   * reflog, it returns 0.
>>   */
>> -#define INTERPRET_BRANCH_LOCAL (1<<0)
>> -#define INTERPRET_BRANCH_REMOTE (1<<1)
>> -#define INTERPRET_BRANCH_HEAD (1<<2)
>> +enum interpret_branch_kind {
>> +	INTERPRET_BRANCH_LOCAL = (1 << 0),
>> +	INTERPRET_BRANCH_REMOTE = (1 << 1),
>> +	INTERPRET_BRANCH_HEAD = (1 << 2),
>> +};
>
> Generally when we use preprocessor constants with bit setting like
> `1 << 0`, we want to use them as flags which aren't mutually exclusive,
> allowing us to do 'INTERPRET_BRANCH_LOCAL | INTERPRET_BRANCH_HEAD' and
> so on.
>
> Is this the case here? If not, maybe we want to mention that explicitly
> and simply use '1, 2....N'?

Taking a brief look at the way these constants are used, e.g.,

        static int branch_interpret_allowed(const char *refname, unsigned allowed)
        {
                if (!allowed)
                        return 1;

                if ((allowed & INTERPRET_BRANCH_LOCAL) &&
                    starts_with(refname, "refs/heads/"))
                        return 1;
                if ((allowed & INTERPRET_BRANCH_REMOTE) &&
                    starts_with(refname, "refs/remotes/"))
                        return 1;

                return 0;
        }

it should be obvious that these are not mutually exclusive choices,
rather they are independent flags that you can flip ON to express
dwimming a short name to what types of branches are allowed.

Besides, the original assignes one-bit-per-value to these constants;
it is not a place for this "CPP macro turned into enum to help those
who inspect a running program with gdb" patch to change assignments
of values.  That would be a separate topic.