On Fri, Dec 10 2021, Glen Choo wrote: > Æ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 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.