git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Johannes Sixt <j6t@kdbg.org>
Cc: Junio C Hamano <gitster@pobox.com>,
	git@vger.kernel.org, Phillip Wood <phillip.wood123@gmail.com>
Subject: Re: [PATCH v3 7/7] git-sh-setup: don't mark trees not used in-tree for i18n
Date: Sun, 03 Apr 2022 17:22:21 +0200	[thread overview]
Message-ID: <220403.86fsmukvp5.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <cc9aaba3-6904-4f96-db50-a368b9d9f0f2@kdbg.org>


On Sat, Apr 02 2022, Johannes Sixt wrote:

> Am 02.04.22 um 12:44 schrieb Ævar Arnfjörð Bjarmason:
>> 
>> On Thu, Mar 31 2022, Johannes Sixt wrote:
>> 
>>> Am 31.03.22 um 13:15 schrieb Ævar Arnfjörð Bjarmason:
>>>> I do have some WIP changes to tear down most of the *.sh and *.perl i18n
>>>> infrastructure (the parts still in use would still have translations),
>>>> and IIRC it's at least a 2k line negative diffstat, and enables us to do
>>>> more interesting things in i18n (e.g. getting rid of the libintl
>>>> dependency).
>>>
>>> Why? Why? Why? Does the status quo have a problem somewhere? All this
>>> sounds like a change for the sake of change.
>> 
>> So this is quite the digression, but, hey, you asked for it.
>
> Oh, no, this is not a digression *at all*. What your write below is the
> kind of text that is needed to judge the value of a change. Without it,
> a change that does not have an otherwise obvious improvement[*] is just
> for the change's sake.

Well let's be clear here.

It's been your claim that the proposed change must not be worth doing
because you don't place the same value on having a 1=1 mapping between
strings we ask translators to work on, and those that we'll actually
present as part of git's UI.

Which is fair enough, and something we can respectfully disagree on.

But that's not the same as claiming that the stated reason for the
upthread patch is incomplete or insufficient.

I can tell you that as the person who implemented this whole i18n
facility that providing translations for someone's random shellscript
was never the point, at all.

It just so happens that because we implemented some bits of
functionality of the porcelain as shellscripts, and at the same time had
a shellscript library which (regrettably or not) seems to invite both
in-tree and out-of-tree users to use it,that the two went hand-in-hand.

But now that they don't anymore I don't see anything "handwaving" about
simply removing the translation markings. I don't think they serve any
purpose anymore.

> [*] In my book, getting rid of a libintl dependency is not an obvious
> improvement. I may be biased in this case, because that dependency was
> never a problem for me. Might be because my personal builds all have
> NO_GETTEXT set.

So not only don't you use a translated version of git, but you don't
even compile one with it?

Yes, I can imagine that hasn't exposed you to any of the problems with
it :)

>>>> But I also don't think that such a series is probably not possible in
>>>> the near term if we're going to insist that all shellscript output must
>>>> byte-for-byte be the same (for boring reasons I won't go into, but it's
>>>> mainly to do with sh-i18n--envsubst.c).
>>>
>>> Such an insistence can easily be lifted if the change is justified
>>> sufficiently. I haven't seen such a justification, yet.
>> 
>> Sure, but re the "chicken & egg" problem I might do all the work to do
>> all that, and someone such as yourself might rightly point out that it
>> would break someone's obscure use-case, scuttling the whole thing.
>> 
>> Which isn't an exaggeration b.t.w., if you e.g. look through our
>> remaining gettext.sh usage you'll find that we carry the entirety of
>> sh-i18n--ensubst.c and everything around it (at least ~1k lines) for
>> emitting a single word in a single message in git-sh-setup.sh, that's
>> it.
>
> See, someone thought it was a good idea to have i18n in shell scripts
> and others agreed that it was worth having ~1k lines of code to do that.
> So the code went in. From then on, these ~1k lines are *not a problem*
> in themselves. From then on, the decision of having ~1k lines or not
> having them can only be based on what effect they have, but no longer on
> "oh, wow, that's 1k lines to write a single word; do we really want that"?

Aside from i18n. I don't agree with that in general.

Yes, code that's in-tree and working needs to be under less scrutiny as
a new addition, and refactoring something isn't always worth it. We'll
also need to review the removals.

But there's also a cost to keeping things around, as you can e.g. see
from various portability and correctness fixes to this code we've
perma-forked from the GNU GPLv2 version.

There's some tipping point wherea refactoring isn't worth it, but
emitting the word "usage" with ~1k lines is a pretty clear candidate in
my mind for a "git rm".

>> Because the whole reason eval_gettext exists, and everything to support
>> it, is to support the use-case of feeding *arbitrary input* into the
>> translation engine, i.e. not the string you yourself have in your source
>> code & trust (it avoids shell "eval").
>> 
>> But if you think that's of paramount importance (that word is "usage"
>> b.t.w., and the equivalent in usage.c isn't even translated) there
>> wouldn't be any way to make forward progress towards the next step of
>> making the remaining shellscript translations call some "git sh--i18n"
>> helper to get their output.
>> 
>> So, to the extent that I was going to pursue the above plan at all I
>> wanted to do it in small steps, especially now as git-submodule.sh et al
>> are going away.
>> 
>> So.
>> 
>> It would be nice to get some leeway in some areas, especially for
>> something like this where I implemented this entire i18n system to begin
>> with, so I'd think it would be clear that it's not some drive-by
>> contribution. I clearly care about the end-goal, and have been sticking
>> with this particular topic for more than a decade.
>> 
>> Not everything can always be a single atomically understood patch that
>> carries all possible reasons to make the change with it, some things are
>> more of a longer term incremental effort.
>> 
>> And since we all have limited time on this spinning ball of mud
>> sometimes it can make sense to trickle in initial changes to see if some
>> larger end-goal is even attainable, or will be blocked due to some
>> unforeseen (or underestimated) reasons.
>
> You can't have leeway for a change whose conclusion is "removes some
> miniscule feature". But if you add "Here is the secret plan to Scrat's
> golden nut; let's start with this change, even though it removes some
> miniscule feature", things are vastly different.

I mean leeway on the topic that I probably have some idea of what I'm
talking about when it comes to git's i18n support, and whether it's
worth the effort to keep certain things around or not.

I.e. you started this thread by claiming that the removal of these
translations would be "castrating [out-of-tree] functionality, [which
is] unacceptable.".

As noted above I don't think that assessment is correct, and if I'm
understanding you correctly you don't even use git's i18n mechanism at
all.

Which I think presents only two possible conclusions.

One is that I, the person who added the i18n mechanism in the first
place, am so clueless about how it work or what it's for, that I'm
(intentionally or not) submitting patches that "castrate" it.

The other is that you've understandably missed some of the nuance, such
as why we're even marking strings for translation, and what the intended
audience of them.


  reply	other threads:[~2022-04-03 15:56 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-19 12:46 [PATCH 0/6] various: remove dead code Ævar Arnfjörð Bjarmason
2021-11-19 12:46 ` [PATCH 1/6] xdiff/xmacros.h: remove unused XDL_PTRFREE Ævar Arnfjörð Bjarmason
2021-11-19 14:32   ` Jeff King
2021-11-19 12:46 ` [PATCH 2/6] pack-bitmap-write: remove unused bitmap_reset() function Ævar Arnfjörð Bjarmason
2021-11-19 14:33   ` Jeff King
2021-11-19 12:46 ` [PATCH 3/6] object-store.h: remove unused has_sha1_file*() Ævar Arnfjörð Bjarmason
2021-11-19 14:34   ` Jeff King
2021-11-19 12:46 ` [PATCH 4/6] alloc.[ch]: remove alloc_report() function Ævar Arnfjörð Bjarmason
2021-11-19 19:42   ` Junio C Hamano
2021-11-19 12:46 ` [PATCH 5/6] strbuf: remove unused istarts_with() function Ævar Arnfjörð Bjarmason
2021-11-19 14:40   ` Jeff King
2021-11-19 19:14   ` Junio C Hamano
2021-11-19 12:46 ` [PATCH 6/6] json-writer.[ch]: remove unused formatting functions Ævar Arnfjörð Bjarmason
2021-11-19 14:41   ` Jeff King
2021-11-19 21:53     ` Junio C Hamano
2021-11-19 14:42 ` [PATCH 0/6] various: remove dead code Jeff King
2021-11-19 20:26 ` [PATCH v2 0/5] " Ævar Arnfjörð Bjarmason
2021-11-19 20:26   ` [PATCH v2 1/5] xdiff/xmacros.h: remove unused XDL_PTRFREE Ævar Arnfjörð Bjarmason
2021-11-19 20:26   ` [PATCH v2 2/5] pack-bitmap-write: remove unused bitmap_reset() function Ævar Arnfjörð Bjarmason
2021-11-19 20:26   ` [PATCH v2 3/5] object-store.h: remove unused has_sha1_file*() Ævar Arnfjörð Bjarmason
2021-11-19 20:26   ` [PATCH v2 4/5] alloc.[ch]: remove alloc_report() function Ævar Arnfjörð Bjarmason
2021-11-19 20:26   ` [PATCH v2 5/5] json-writer.[ch]: remove unused formatting functions Ævar Arnfjörð Bjarmason
2022-03-26 17:14   ` [PATCH v3 0/7] various: remove dead code, drop i18n not used in-tree Ævar Arnfjörð Bjarmason
2022-03-26 17:14     ` [PATCH v3 1/7] run-command.h: remove always unused "clean_on_exit_handler_cbdata" Ævar Arnfjörð Bjarmason
2022-03-26 17:14     ` [PATCH v3 2/7] configure.ac: remove USE_PIC comment Ævar Arnfjörð Bjarmason
2022-03-26 17:14     ` [PATCH v3 3/7] xdiff/xmacros.h: remove unused XDL_PTRFREE Ævar Arnfjörð Bjarmason
2022-03-26 17:14     ` [PATCH v3 4/7] pack-bitmap-write: remove unused bitmap_reset() function Ævar Arnfjörð Bjarmason
2022-03-26 17:14     ` [PATCH v3 5/7] object-store.h: remove unused has_sha1_file*() Ævar Arnfjörð Bjarmason
2022-03-26 17:14     ` [PATCH v3 6/7] alloc.[ch]: remove alloc_report() function Ævar Arnfjörð Bjarmason
2022-03-26 17:14     ` [PATCH v3 7/7] git-sh-setup: don't mark trees not used in-tree for i18n Ævar Arnfjörð Bjarmason
2022-03-27 10:47       ` Johannes Sixt
2022-03-27 11:15         ` Ævar Arnfjörð Bjarmason
2022-03-28  6:04           ` Johannes Sixt
2022-03-28 12:16             ` Ævar Arnfjörð Bjarmason
2022-03-28 20:06               ` Johannes Sixt
2022-03-31 10:23               ` Phillip Wood
2022-03-31 11:15                 ` Ævar Arnfjörð Bjarmason
2022-03-31 20:27                   ` Johannes Sixt
2022-04-02 10:44                     ` Ævar Arnfjörð Bjarmason
2022-04-02 14:16                       ` Johannes Sixt
2022-04-03 15:22                         ` Ævar Arnfjörð Bjarmason [this message]
2022-04-04 20:20                           ` Johannes Sixt
2022-03-31  1:45     ` [PATCH v4 0/6] various: remove dead code Ævar Arnfjörð Bjarmason
2022-03-31  1:45       ` [PATCH v4 1/6] run-command.h: remove always unused "clean_on_exit_handler_cbdata" Ævar Arnfjörð Bjarmason
2022-03-31 22:03         ` Junio C Hamano
2022-03-31  1:45       ` [PATCH v4 2/6] configure.ac: remove USE_PIC comment Ævar Arnfjörð Bjarmason
2022-03-31  1:45       ` [PATCH v4 3/6] xdiff/xmacros.h: remove unused XDL_PTRFREE Ævar Arnfjörð Bjarmason
2022-03-31  1:45       ` [PATCH v4 4/6] pack-bitmap-write: remove unused bitmap_reset() function Ævar Arnfjörð Bjarmason
2022-03-31 22:06         ` Junio C Hamano
2022-03-31  1:45       ` [PATCH v4 5/6] object-store.h: remove unused has_sha1_file*() Ævar Arnfjörð Bjarmason
2022-03-31  1:45       ` [PATCH v4 6/6] alloc.[ch]: remove alloc_report() function Æ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=220403.86fsmukvp5.gmgdl@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    --cc=phillip.wood123@gmail.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).