On Thu, Nov 01 2018, Duy Nguyen wrote:

> On Thu, Nov 1, 2018 at 3:04 PM Junio C Hamano <gits...@pobox.com> wrote:
>>
>> Ævar Arnfjörð Bjarmason <ava...@gmail.com> writes:
>>
>> > Could you please pick up
>> > https://public-inbox.org/git/20181024114725.3927-1-ava...@gmail.com/ ?
>> > It seems to have fallen between the cracks and addressed the feedback on
>> > v1, and looks good to me (and nobody's objected so far...).
>>
>> If this is the runtime-gettext-poison thing, this did not fall thru
>> the cracks---I didn't get the impression that "no objection was a
>> sign of being excellent"; rather I recall feeling that you were the
>> only person who were excited about it, while everybody else was
>> "Meh".
>
> I would be more excited if the scrambling patches go first (*)

Sorry to be so unexciting :)

I've sent a v3 in
https://public-inbox.org/git/20181101193115.32681-1-ava...@gmail.com/T/#u
which which hopefully addresses the concerns you & SZEDER had.

> and then we start to make "make test" poisoned by default. Scrambled
> output is still very readable and it will make people not forget about
> grepping translated stuff the wrong way. Of course *i18n* functions in
> the test suite need to be updated to to grep/compare for real, not
> just exit early like they do now.

I think now that this is a runtime option we'd instead just do stuff
like:

    GIT_TEST_GETTEXT_POISON= git ... 2>err &&
    grep str err

Which has the advantages of:

 1) You can grep for error messages you find in the code and find tests
    that check for them.

 2) When you run the tests and something goes wrong, it's not some
    scrambled output, so you can quickly e.g. search for that error in
    the code / on Google without needing to hunt for some "how did I
    disable the scrambling again...?" knob.

After all, the entire point of the facility is to catch us updating
strings which have existing tests without "GIT_TEST_GETTEXT_POISON=" (or
test_i18n...), but once we find those (with my patch) we can just
selectively turn the whole thing off.

> (*) The pseudolocalization idea is also good, but printing unicode by
> default may be a bit of a stretch. Not everybody is running the test
> suite with a unicode-capable terminal.

Reply via email to