Re: [PATCH v6 08/10] submodule: add a helper to check if it is safe to write to .gitmodules

[Antonio Ospite <ao2@ao2.it>]

On Sun, 07 Oct 2018 08:44:20 +0900
Junio C Hamano <gitster@pobox.com> wrote:

> Stefan Beller <sbeller@google.com> writes:
> 
> >>  static int module_config(int argc, const char **argv, const char *prefix)
> >>  {
> >> +       enum {
> >> +               CHECK_WRITEABLE = 1
> >> +       } command = 0;
> >
> > Can we have the default named? Then we would only use states
> > from within the enum?
> 
> Why?  Do we use a half-intelligent "switch () { case ...: ... }"
> checker that would otherwise complain if we handled "case 0" in such
> a switch statement, or something like that?
> 
> Are we going to gain a lot more enum members, by the way?  At this
> point, this looks more like a
> 
> 	unsigned check_writable = 0; /* default is not to check */
> 
> to me.

Hi,

the CHECK_WRITEABLE operation is alternative to the get/set ones, not
an addition, so I can see the rationale behind Stefan's suggestion:
either have named enums members for all command "modes" or for none of
them; however other users of enum+OPT_CMDMODE seems to think like the
enum is for commands passed as *options* and the unnamed default is for
actions derived from *arguments*. I don't have a strong opinion on this
matter, tho, so just tell me what you prefer and I'll do it for v7.

Using an enum was to have a more explicit syntax in case other commands
were going to be added in the future (I imagine "--stage" or
"--list-all" as possible additions), and does not affect the generated
code, so I though it was worth it.

Anyways, these are really details, let's concentrate on patches 9 and
10 which deserve much more attention. :)

Thanks you,
   Antonio
-- 
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?