Hi Carlo,
On Fri, 24 Apr 2020, Carlo Marcelo Arenas Belón wrote:
> On Fri, Apr 24, 2020 at 11:49:52AM +0000, Johannes Schindelin via GitGitGadget wrote:
> > diff --git a/credential.c b/credential.c
> > index 7dbbf26f174..c1a9ca4e485 100644
> > --- a/credential.c
> > +++ b/credential.c
> > @@ -35,6 +35,10 @@ int credential_match(const struct credential *want,
> > #undef CHECK
> > }
> >
> > +
> > +static int credential_from_potentially_partial_url(struct credential *c,
> > + const char *url);
> > +
> > static int credential_config_callback(const char *var, const char *value,
> > void *data)
> > {
>
> something like credential_from_url_partial might had been more grep friendly
> but this would work as well.
It might be more grep'able, but it sounds really awkward to me, that's why
I did not use that name (it was my first candidate).
> > diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh
> > index efed3ea2955..f796bbfd48b 100755
> > --- a/t/t0300-credentials.sh
> > +++ b/t/t0300-credentials.sh
> > @@ -448,4 +448,43 @@ test_expect_success 'credential system refuses to work with missing protocol' '
> > test_i18ncmp expect stderr
> > '
> >
> > +test_expect_success 'credential config with partial URLs' '
> > + echo "echo password=yep" | write_script git-credential-yep &&
> > + test_write_lines url=https://user@xxxxxxxxxxx/repo.git >stdin &&
> > + for partial in \
> > + example.com \
> > + user@xxxxxxxxxxx \
>
> even if it works, configurations with a username might not be worth the
> trouble to support IMHO
>
> maybe better not to include them in the tests then either?
Let me counter this:
- It would take extra code _to prevent_ the username from being used, and
- There is precedent where the user name _does_ matter: it is relatively
normal to access different orgs' repositories at
https://dev.azure.com/<org>/<repo>/_git via different accounts.
Together, those points convince me that special-casing the username _and
explicitly ignoring it_ would not make sense.
> other than that, like the previous version (which is functionally equivalent)
> should be IMHO good to go.
Thank you for reviewing it!
Ciao,
Dscho
