Re: [PATCH v2 1/6] midx: add MIDX_PROGRESS flag <snip>

From: William Baker <williamtbakeremail@gmail.com>
Date: Sep 30, 2019, 17:01 UTC
Message-ID: <2de6b236-7bd8-256b-7d8f-911d63a47498@gmail.com>
In-Reply-To: <xmqqlfu9krzv.fsf@gitster-ct.c.googlers.com>

On 9/27/19 8:40 PM, Junio C Hamano wrote:

Show 28 quoted lines

> SZEDER Gábor <szeder.dev@gmail.com> writes:
> 
>>> +#define MIDX_PROGRESS     (1 << 0)
>>
>> Please consider using an enum.
> 
> If they are used by assiging one of their values, definitely a good
> idea to use an enum.  Are debuggers clever enough that they can
> tell, when they see something like this:
> 
> 	enum gress {
> 		PROGRESS = 1,
> 		REGRESS = 2,
> 	};
> 
> 	void func(enum gress v);
> 
> 	...
> 
>         void caller(void)
> 	{
> 		func(PROGRESS | REGRESS);
> 		func(PROGRESS + REGRESS);
> 		func(PROGRESS * 3);
> 	}
> 
> how caller came about to give 3?
>

My debugger was not smart enough to figure out what flags were combined to give the value of 3 in this example.

I saw that the code base is currently a mix of #define and enums when it comes to flags (e.g. dir_struct.flags and rebase_options.flags are both enums) and so using one here would not be something new stylistically.

Although my debugger might not be the smartest, I haven't noticed any downsides to switching this to an enum.

Thanks, William