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

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 24, 2020, 00:40 UTC
Message-ID: <xmqqlfmlemxk.fsf@gitster.c.googlers.com>
In-Reply-To: <xmqqpnbxen6h.fsf@gitster.c.googlers.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 22 quoted lines

> Carlo Marcelo Arenas Belón <carenas@gmail.com> 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@google.com> 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.

Thanks.