Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > On Thu, Dec 09 2021, Glen Choo wrote: > >> Josh Steadmon <steadmon@xxxxxxxxxx> writes: >> >>>> > @@ -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@xxxxxxxxxxxxxxxxxxx