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

[Junio C Hamano <gitster@pobox.com>]

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

> 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.