From: Johannes Schindelin Date: Mon, 09 May 2016 14:59:41 GMT Subject: Re: [PATCH v5 1/2] bisect--helper: use OPT_CMDMODE instead of OPT_BOOL Message-ID: In-Reply-To: Hi Pranit, On Sun, 8 May 2016, Pranit Bauva wrote: > On Sun, May 8, 2016 at 12:34 PM, Johannes Schindelin > 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. Ciao, Dscho