git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Jean-Noël AVILA" <jn.avila@free.fr>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Junio C Hamano" <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: ja/i18n-common-messages
Date: Sat, 05 Feb 2022 13:18:37 +0100	[thread overview]
Message-ID: <7408402.9NgVXbFLuH@cayenne> (raw)
In-Reply-To: <xmqq5ypuo9it.fsf@gitster.g>

On Friday, 4 February 2022 17:56:42 CET Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> 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@gmail.com>
> >
> > 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@evledraar.gmail.com/
> >
> > 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.





  reply	other threads:[~2022-02-05 12:18 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-04  5:22 What's cooking in git.git (Feb 2022, #01; Thu, 3) Junio C Hamano
2022-02-04  7:00 ` Eric Sunshine
2022-02-05  8:00   ` Junio C Hamano
2022-02-04  8:13 ` Opinions on merging questions (Was: What's cooking in git.git (Feb 2022, #01; Thu, 3)) Elijah Newren
2022-02-04 11:20   ` en/remerge-diff (was: Opinions on merging questions (Was: What's cooking in git.git (Feb 2022, #01; Thu, 3))) Ævar Arnfjörð Bjarmason
2022-02-04 16:42     ` en/remerge-diff Junio C Hamano
2022-02-06 11:41   ` Opinions on merging questions (Was: What's cooking in git.git (Feb 2022, #01; Thu, 3)) Eric Sunshine
2022-02-09 14:57     ` Derrick Stolee
2022-02-07 14:11   ` Phillip Wood
2022-02-04 11:25 ` jc/doc-log-messages (was: " Ævar Arnfjörð Bjarmason
2022-02-04 12:23 ` rj/receive-pack-abort-upon-disconnect " Ævar Arnfjörð Bjarmason
2022-02-04 12:25 ` cb/clear-quarantine-early-on-all-ref-update-errors " Ævar Arnfjörð Bjarmason
2022-02-04 16:45   ` cb/clear-quarantine-early-on-all-ref-update-errors Junio C Hamano
2022-02-04 12:29 ` ja/i18n-common-messages (was: What's cooking in git.git (Feb 2022, #01; Thu, 3)) Ævar Arnfjörð Bjarmason
2022-02-04 16:56   ` ja/i18n-common-messages Junio C Hamano
2022-02-05 12:18     ` Jean-Noël AVILA [this message]
2022-02-06  8:12       ` ja/i18n-common-messages Bagas Sanjaya
2022-02-06 10:03         ` ja/i18n-common-messages Jean-Noël AVILA
2022-02-04 12:31 ` ps/avoid-unnecessary-hook-invocation-with-packed-refs (was: What's cooking in git.git (Feb 2022, #01; Thu, 3)) Ævar Arnfjörð Bjarmason
2022-02-04 12:36 ` ab/ambiguous-object-name " Ævar Arnfjörð Bjarmason
2022-02-04 13:05 ` tl/ls-tree-oid-only " Ævar Arnfjörð Bjarmason
2022-02-04 13:11 ` ab/only-single-progress-at-once " Ævar Arnfjörð Bjarmason

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7408402.9NgVXbFLuH@cayenne \
    --to=jn.avila@free.fr \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).