On Friday, 4 February 2022 17:56:42 CET Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > > > On Thu, Feb 03 2022, Junio C Hamano wrote: > > > >> * ja/i18n-common-messages (2022-01-31) 5 commits > >> - i18n: fix some misformated placeholders in command synopsis > >> - i18n: remove from i18n strings that do not hold translatable parts > >> - i18n: factorize "invalid value" messages > >> - SQUASH??? > >> - i18n: factorize more 'incompatible options' messages > >> > >> Unify more messages to help l10n. > >> > >> Will merge to 'next' after squashing the fix-up in. > >> source: <pull.1123.v4.git.1643666870.gitgitgadget@xxxxxxxxx> > > > > I had a comment on the API direction in parse-options.c, which I think > > should be done differently, but I also think it would be fine to just > > change it up later: > > > > https://lore.kernel.org/git/220201.86a6fa9tmr.gmgdl@xxxxxxxxxxxxxxxxxxx/ > > > > I replied to v2 instead of v4 due to some (now fixed) mail delays at the > > time, but that comment still applies to the latest version. > > I do not think the change at the parse-options level would mix well > with what this topic wants to do. Large parts of the code this > series touches will have to be rewritten once again. > > It will open us to new complaints we would not hear with this series > from users who first say "git cmd -a -b -c", get stopped with "a and > b are incompatible", then say "git cmd -a -c", get stopped again > with "a and c are incompatible", and utter "well you could have told > me upfront that these three are not to be used together" in > frustration. > Bad news: this implementation is already crippled. For instance, calling `git commit --fixup -m -c` would first bring up: "options '-c' and '--fixup' cannot be used together" Then remove --fixup and you get "options '-m' and '-c' cannot be used together" This is because (according to the code, I don't know if it's really what's wanted) the real logic is Exclusive ("-C", "-c", "-F", Or("--fixup", "-m")) This is a lot more complex than what can be achieved with simple tests. Designing a generic facility for this is quite an endeavour and the messages to guide the user precisely may need to be built with sentence lego, defeating the initial purpose of this series about localization. > I do not mind waiting for a few days to see what Jean-Noël would > say, but my take on this is that between starting from the current > code base and from the state after applying this series, there will > not be much difference in amount of the effort necessary to extend > CMDMODE to mark and detect combinations of incompatible options at > parse-options level. So I am inclined to merge this series down. > After this series, * the messages are more precise, except in few edge cases * they can be localized and we can get new messages for free * the code is marginally easier to read While reformatting the code, a striking observation was that now that the option strings are extracted from the localizable parts, they are repeated all around the code, and this begs for another factorization, to make it dry. A new proposed framework would enhance greatly this third point, and I'm all for it to happen, as long as it does not regress on the first two points. In the mean time, this patch is the best I can propose.
