Re: [PATCH 2/3] submodule: implement `module_name` as a builtin helper

[Eric Sunshine <sunshine@sunshineco.com>]

On Mon, Aug 31, 2015 at 3:19 PM, Stefan Beller <sbeller@google.com> wrote:
> This implements the helper `module_name` in C instead of shell,
> yielding a nice performance boost.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index beaab7d..c8f7e0c 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -101,6 +104,26 @@ static int module_list(int argc, const char **argv, const char *prefix)
> +static int module_name(int argc, const char **argv, const char *prefix)
> +{
> +       const char *name;
> +       const struct submodule *sub;
> +
> +       if (argc != 1)
> +               usage("git submodule--helper module_name <path>\n");
> +
> +       gitmodules_config();
> +       sub = submodule_from_path(null_sha1, argv[0]);
> +
> +       if (!sub)
> +               die("No submodule mapping found in .gitmodules for path '%s'", argv[0]);

In the original shell code, this error message went through
eval_gettext(), so don't you want:

    die(_("No ..."), ...);

?

> +       name = sub->name;
> +       printf("%s\n", name);

Why the useless assignment to 'name'? Instead:

    printf("%s\n", sub->name);

> +       return 0;
> +}
> +
>  int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
>  {
>         if (argc < 2)
> @@ -109,6 +132,9 @@ int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
>         if (!strcmp(argv[1], "module_list"))
>                 return module_list(argc - 1, argv + 1, prefix);
>
> +       if (!strcmp(argv[1], "module_name"))
> +               return module_name(argc - 2, argv + 2, prefix);
> +
>  usage:
> -       usage("git submodule--helper module_list\n");
> +       usage("git submodule--helper [module_list | module_name]\n");
>  }