On Fri, 11 Sep 2020 at 15:24:15 -0400, Derrick Stolee writes:
> 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.
Makes perfect sense! My original intention was to provide a separate
use-case specific message, but you're right: "is not a valid remote
name" (as in `builtin/remote.c`) is still very clear.
> 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);
That's a fantastic idea -- thanks for the suggestion! I'll do that for
v2.
> > +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.
Darn, I missed this one. Thanks for the heads-up :)
--
Sean
