Re: [PATCH v5 1/2] bisect--helper: use OPT_CMDMODE instead of OPT_BOOL
- From
Pranit Bauva <pranit.bauva@gmail.com>
- Date
- May 9, 2016, 15:33 UTC
- Message-ID
- <CAFZEwPNgEHOYhdqaX6W=_j7OUoBbX-=fHBLndGYO20OZjRUx2w@mail.gmail.com>
- In-Reply-To
- <alpine.DEB.2.20.1605091657290.4092@virtualbox>
Hey Johannes,
On Mon, May 9, 2016 at 8:29 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
Show 41 quoted lines
> Hi Pranit,
>
> On Sun, 8 May 2016, Pranit Bauva wrote:
>
>> On Sun, May 8, 2016 at 12:34 PM, Johannes Schindelin
>> <Johannes.Schindelin@gmx.de> wrote:
>> >
>> > On Fri, 6 May 2016, Pranit Bauva wrote:
>> >
>> >> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
>> >> index 3324229..d8de651 100644
>> >> --- a/builtin/bisect--helper.c
>> >> +++ b/builtin/bisect--helper.c
>> >> @@ -8,13 +8,17 @@ static const char * const git_bisect_helper_usage[] = {
>> >> NULL
>> >> };
>> >>
>> >> +enum subcommand {
>> >> + NEXT_ALL = 1
>> >> +};
>> >
>> > I still do not think that this enum needs to have file scope. Function
>> > scope is enough.
>>
>> In the very initial patch I made it in function scope. To which you
>> pointed out[1] that in all other examples but for one have file scope
>> so then I thought maybe that exception was a wrong example and I
>> should stick to the convention of putting it in file scope.
>
> Oh, sorry, I meant to imply that it is good as it is by saying "so this
> code is fine"...
>
> I was just surprised because I thought I remembered that some old C
> standard does not allow enums to be function scoped. But I was wrong.
>
>> But now I also realize that builtin/replace.c uses "cmdmode" instead of
>> "subcommand" so I am still wondering what would be the most appropriate?
>
> I think the replace.c code is really a good example. Function-scoped,
> using the "cmdmode" name that obviously corresponds to the OPT_CMDMODE
> name.Sure. WIll do!
Regards, Pranit Bauva