Re: [PATCHv19 09/11] git submodule update: have a dedicated helper for cloning

[Stefan Beller <sbeller@google.com>]

On Sat, Feb 27, 2016 at 12:40 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Fri, Feb 26, 2016 at 6:48 AM, Stefan Beller <sbeller@google.com> wrote:
>> +static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
>> +                                          struct child_process *child,
>> +                                          struct submodule_update_clone *suc,
>> +                                          struct strbuf *out)
>> +{
>> +       const struct submodule *sub = NULL;
>> +       struct strbuf displaypath_sb = STRBUF_INIT;
>> +       struct strbuf sb = STRBUF_INIT;
>> +       const char *displaypath = NULL;
>> +       char *url = NULL;
>> +       int needs_cloning = 0;
>> +
>> +       if (ce_stage(ce)) {
>> +               if (suc->recursive_prefix) {
>> +                       strbuf_addf(out, "Skipping unmerged submodule %s/%s\n",
>> +                                   suc->recursive_prefix, ce->name);
>
> I'm pretty sure this string is for human consumption (because it's
> _()'d elsehwere in this function), please _() this string.
>
>> +               } else {
>> +                       strbuf_addf(out, "Skipping unmerged submodule %s\n",
>> +                                   ce->name);
>
> and this one
>
>> +               }
>> +               goto cleanup;
>> +       }
>> +
>> +       sub = submodule_from_path(null_sha1, ce->name);
>> +
>> +       if (suc->recursive_prefix)
>> +               displaypath = relative_path(suc->recursive_prefix,
>> +                                           ce->name, &displaypath_sb);
>> +       else
>> +               displaypath = ce->name;
>> +
>> +       if (suc->update.type == SM_UPDATE_NONE
>> +           || (suc->update.type == SM_UPDATE_UNSPECIFIED
>> +               && sub->update_strategy.type == SM_UPDATE_NONE)) {
>> +               strbuf_addf(out, "Skipping submodule '%s'\n",
>> +                           displaypath);
>
> and this one
>
>> +               goto cleanup;
>> +       }
>> +
>> +       /*
>> +        * Looking up the url in .git/config.
>> +        * We must not fall back to .gitmodules as we only want
>> +        * to process configured submodules.
>> +        */
>> +       strbuf_reset(&sb);
>> +       strbuf_addf(&sb, "submodule.%s.url", sub->name);
>> +       git_config_get_string(sb.buf, &url);
>> +       if (!url) {
>> +               /*
>> +                * Only mention uninitialized submodules when their
>> +                * path have been specified
>> +                */
>> +               if (suc->warn_if_uninitialized)
>> +                       strbuf_addf(out, _("Submodule path '%s' not initialized\n"
>> +                                   "Maybe you want to use 'update --init'?\n"),
>> +                                   displaypath);
>
> oh it's already marked :)
>
> BTW, while you're editing this file, perhaps do this too (maybe in a
> separate patch)? Because die() already prepends "fatal:"

Makes sense. As builtin/submodule--helper.c was introduced in 2.7.0
and translation has already started fr 2.8.0, I'll just pick it up as
part of this series
instead of sending a bugfix patch alone.

Thanks for review!

>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index a6e54fa..6cf47de 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -731,13 +731,13 @@ int cmd_submodule__helper(int argc, const char
> **argv, const char *prefix)
>  {
>         int i;
>         if (argc < 2)
> -               die(_("fatal: submodule--helper subcommand must be "
> +               die(_("submodule--helper subcommand must be "
>                       "called with a subcommand"));
>
>         for (i = 0; i < ARRAY_SIZE(commands); i++)
>                 if (!strcmp(argv[1], commands[i].cmd))
>                         return commands[i].fn(argc - 1, argv + 1, prefix);
>
> -       die(_("fatal: '%s' is not a valid submodule--helper "
> +       die(_("'%s' is not a valid submodule--helper "
>               "subcommand"), argv[1]);
>  }
> --
> Duy