Re: [PATCHv19 09/11] git submodule update: have a dedicated helper for cloning
- From
Stefan Beller <sbeller@google.com>
- Date
- Feb 29, 2016, 19:03 UTC
- Message-ID
- <CAGZ79kZYqXWAqjmYJtHM5rPUPbCS_NxOsrbspkOS1WAVV44wzg@mail.gmail.com>
- In-Reply-To
- <CACsJy8DmhJrUkp=d4kSst=0yGWn+Hk=MqxCmhH3WqfPzhTt+Uw@mail.gmail.com>
On Sat, Feb 27, 2016 at 12:40 AM, Duy Nguyen <pclouds@gmail.com> wrote:
Show 72 quoted lines
> 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!
Show 24 quoted lines
>
> 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