On 9/11/2020 2:25 PM, Sean Barag via GitGitGadget wrote:
> + strbuf_addf(&resolved_refspec, "refs/heads/test:refs/remotes/%s/test", option_origin);
> + if (!valid_fetch_refspec(resolved_refspec.buf))
> + /* TRANSLATORS: %s will be the user-provided --origin / -o option */
> + die(_("'%s' is not a valid origin name"), option_origin);
Looking at this again, I'm not sure the translators note is
necessary. Also, I would say "is not a valid remote name".
That makes the string align with the already-translated string
in builtin/remote.c.
This code is duplicated from builtin/remote.c, so I'd rather
see this be a helper method in refspec.c and have both
builtin/clone.c and builtin/remote.c call that helper.
Here is the helper:
void valid_remote_name(const char *name)
{
int result;
struct strbuf refspec = STRBUF_INIT;
strbuf_addf(&refspec, "refs/heads/test:refs/remotes/%s/test", name);
result = valid_fetch_refspec(refspec.buf);
strbuf_release(&refspec);
return result;
}
And here is the use in builtin/clone.c:
if (!valid_remote_name(option_origin))
die(_("'%s' is not a valid remote name"), option_origin);
and in builtin/remote.c:
if (!valid_remote_name(name))
die(_("'%s' is not a valid remote name"), name);
> +test_expect_success 'rejects invalid -o/--origin' '
> +
> + test_expect_code 128 git clone -o "bad...name" parent clone-bad-name 2>err &&
> + test_debug "cat err" &&
> + test_i18ngrep "'\''bad...name'\'' is not a valid origin name" err
> +
> +'
> +
Double newlines here! I personally appreciate newlines to
spread out content, but it doesn't fit our guidelines.
Thanks,
-Stolee
