Re: [RFC PATCH v2 04/12] submodule--helper: add a new 'config' subcommand

[Jeff King <peff@peff.net>]

On Thu, Aug 02, 2018 at 11:47:30AM -0700, Stefan Beller wrote:

> > +static int module_config(int argc, const char **argv, const char *prefix)
> > +{
> > +       if (argc < 2 || argc > 3)
> > +               die("submodule--helper config takes 1 or 2 arguments: name [value]");
> > +
> > +       /* Equivalent to ACTION_GET in builtin/config.c */
> > +       if (argc == 2)
> > +               return print_config_from_gitmodules(argv[1]);
> > +
> > +       /* Equivalent to ACTION_SET in builtin/config.c */
> > +       if (argc == 3)
> > +               return config_set_in_gitmodules_file_gently(argv[1], argv[2]);
> > +
> > +       return 0;
> 
> Technically we cannot reach this point here?
> Maybe it would be more defensive to
> 
>     BUG("How did we get here?");
> 
> or at least return something !=0 ?

When I find myself reaching for a BUG(), it is often a good time to see
if we can restructure the code so that the logic more naturally falls
out.  Here the issue is that the first if conditional repeats the "else"
for the other two. So I think we could just write:

  if (argc == 2)
	return ...
  if (argc == 3)
	return ...

  die("need 1 or 2 arguments");

-Peff