Re: [PATCH v2 3/3] credential: handle `credential.<partial-URL>.<key>` again

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Junio,

On Thu, 23 Apr 2020, Junio C Hamano wrote:

> Junio C Hamano <gitster@xxxxxxxxx> writes:
>
> > Carlo Marcelo Arenas Belón <carenas@xxxxxxxxx> writes:
> >
> >> On Thu, Apr 23, 2020 at 11:43:17PM +0000, Johannes Schindelin via GitGitGadget wrote:
> >>> diff --git a/credential.c b/credential.c
> >>> index 52965a5122c..3505f6356d8 100644
> >>> --- a/credential.c
> >>> +++ b/credential.c
> >>> @@ -53,7 +53,13 @@ static int credential_config_callback(const char *var, const char *value,
> >>>  		char *url = xmemdupz(key, dot - key);
> >>>  		int matched;
> >>>
> >>> -		credential_from_url(&want, url);
> >>> +		if (credential_from_url_gently(&want, url, 1, 0) < 0) {
> >>
> >> definitely not worth a reroll, but just wondering if would make sense to call
> >> credential_from_url_gently(!quiet) here, just for consistency?
> >
> > Speaking of which, it is not clear which one of "...url, 1, 0)" is
> > the "quiet" bit.  I somehow thought that somebody suggested to roll
> > these two into a flags word and give quiet and the other bit a name,
> > and after seeing this line, I tend to agree that would be great for
> > readability.
>
> Ah, I should have checked before opening my mouth.  It was this
> message <20200422233854.GE140314@xxxxxxxxxx> from Jonathan Nieder.
>
> I also am OK with his "two thin wrappers around the underlying
> helper that takes two separate arguments", if that makes the
> resulting code easier to follow.  I have a feeling that the caller
> knows (from the context, or the reason why it calls the
> credential-from-url code) if it wants strict or nonstrict variant
> and that is not something the caller is told from its caller.  And
> if that is the case, "two thin wrappers" approach does make a lot of
> sense.

All right, two wrapper functions it is.

Ciao,
Dscho

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux