Thread (69 messages) flat view 69 messages, 5 authors, 2022-01-20

Re: [PATCH v5 1/2] branch: accept multiple upstream branches for tracking

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-12-11 02:24:47

On Fri, Dec 10 2021, Glen Choo wrote:
Ævar Arnfjörð Bjarmason [off-list ref] writes:
quoted
On Thu, Dec 09 2021, Glen Choo wrote:
quoted
Josh Steadmon [off-list ref] writes:
quoted
quoted
quoted
@@ -121,11 +168,18 @@ int install_branch_config(int flag, const char *local, const char *origin, const
 	advise(_(tracking_advice),
 	       origin ? origin : "",
 	       origin ? "/" : "",
-	       shortname ? shortname : remote);
+	       remotes->items[0].string);
 
 	return -1;
 }
When there is more than one item in remotes->items, this advice is
_technically_ incorrect because --set-upstream-to only takes a single
upstream branch. I think that supporting multiple upstreams in
--set-upstream-to is a fairly niche use case and is out of scope of this
series, so let's not pursue that option.

Another option would be to replace the mention of --set-upstream-to with
"git config add", but that's unfriendly to the >90% of the user
population that doesn't want multiple merge entries.

If we leave the advice as-is, even though it is misleading, a user who
is sophisticated enough to set up multiple merge entries should also
know that --set-upstream-to won't solve their problems, and would
probably be able to fix their problems by mucking around with
.git/config or git config.

So I think it is ok to not change the advice and to only mention the
first merge item. However, it might be worth marking this as NEEDSWORK
so that subsequent readers of this file understand that this advice is
overly-simplistic and might be worth fixing.
Sounds like we should just have separate advice strings for single vs.
multiple merge configs?
That sounds like a good idea if it's not too much work. Otherwise, a
NEEDSWORK is still acceptable to me (but that said, I'm not an authority
on this matter).
We haven't used Q_() with advise() yet, but there's no reason not to:

	advise(Q_("fix your branch by doing xyz",
		  "fix your branches by doing xyz",
                  branches_nr));
Neat, that should do it in most cases. I think this one is a little
trickier because the plural advice messages requires constructing a
list, e.g.

Singular: 
  "\n"
  "After fixing the error cause you may try to fix up\n"
  "the remote tracking information by invoking\n"
  "\"git branch --set-upstream-to=%s%s%s\"."

Plural:
  "\n"
  "After fixing the error cause you may try to fix up\n"
  "the remote tracking information by invoking\n"
  "\"git config --add my_new_remote remote_name\"
  "\"git config --add my_new_upstream1 upstream_name1\"
  "\"git config --add my_new_upstream2 upstream_name2\"

But perhaps this is not too hard since you've already included examples
of how to format lists of strings [1]

[1] https://lore.kernel.org/git/211207.86mtlcpyu4.gmgdl@evledraar.gmail.com (local)
You don't need to use the plural facility for that sort of
message.

Plurals in translated messages are specifically for the case where you
need to compose a sentence like:

    I had %d glasses of water with breakfast

But it's not needed for a message that can be rehrased as a heading
followed by a list of 1 or more items, e.g.:

    Things I've consumed in liquid form during breakfast this week:
    - Water
    - Tea
    - Coffe

If that list stopped at "Water" it would be OK. Every language has some
way of referring to items on a list in general terms, without knowing if
that list is composed of only one item, two etc.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help