Subject: Re: [PATCH v5] tracking branches: add advice to ambiguous refspec error

From: Junio C Hamano <[email-masked]>
Date: Wed, 30 Mar 2022 13:37:53 -0700

Ævar Arnfjörð Bjarmason <[email-masked]> writes:

> On Wed, Mar 30 2022, Tao Klerks via GitGitGadget wrote:
>
>> From: Tao Klerks <[email-masked]>
>
>> +		case 2:
>> +			// there are at least two remotes; backfill the first one
>
> Nit: I think it's been Junio's preference not to introduce C99 comments,

I often mention the rule to new contributors, simply because it has
been that way in our code base, regardless of what my personal
preference might be, and sticking to the style will be more
consistent.  It's not like I am forcing my personal preference on
developers.  Do not mislead new people into thinking so.  It is
especially irritating to see ...

> despite other C99 features now being used (and I think it should work in
> practice as far as portability goes, see
> https://lore.kernel.org/git/[email-masked]/)

... a mention like this, when you know that it is not about
portability but is about consistency, and also you know I've
mentioned more than once on the list if we want to loosen some
written CodingGuidelines rules, especially those that tools do not
necessarily catch.

>> +	if (tracking.matches > 1) {
>> +		int status = die_message(_("not tracking: ambiguous information for ref %s"),
>> +					    orig_ref);
>
> This isn't per-se new, but I wonder if while we're at it we shold just
> quote '%s' here, which we'd usually do. I.e. this message isn't new, but
> referring again to "ref %s" (and not "ref '%s'") below is.

Good.

>> +				 "To support setting up tracking branches, ensure that\n"
>> +				 "different remotes' fetch refspecs map into different\n"
>> +				 "tracking namespaces."),
>> +			       orig_ref,
>> +			       remotes_advice.buf
>> +			       );
>
> Nit: The usual style for multi-line arguments is to "fill" lines until
> you're at 79 characters, so these last three lines (including the ");")
> can all go on the "tracking namespaces" line (until they're at 79, then
> wrap)>

I didn't know about the magic "79" number.  It makes the resulting
source code extremely hard to read, though, while making it easier
to grep for specific messages.