Re: [PATCH 1/9] t7415: remove out-dated comment about translation

From: Jeff King <peff@peff.net>
Date: May 3, 2021, 20:29 UTC
Message-ID: <YJBdPXkogxeERdi8@coredump.intra.peff.net>
In-Reply-To: <874kfkxhxe.fsf@evledraar.gmail.com>

On Mon, May 03, 2021 at 11:46:52AM +0200, Ævar Arnfjörð Bjarmason wrote:

Show 14 quoted lines

> > Since GETTEXT_POISON does not exist anymore, there is no point warning
> > people about whether we should use test_i18ngrep. This is doubly
> > confusing because the comment was describing why it was OK to use grep,
> > but it got caught up in the mass conversion of 674ba34038 (fsck: mark
> > strings for translation, 2018-11-10).
> >
> > Note there are other uses of test_i18ngrep in this script which are now
> > obsolete; I'll save those for a mass-cleanup. My goal here was just to
> > fix the confusing comment in code I'm about to refactor.
> 
> For what it's worth between [1] and [2] I'm not sure what to do about
> the test_i18ngrep cleanup. I think your patch below is fine, but the
> "test_i18ngrep" has mutated into a "grep with debugging", not just
> something needed for GETTEXT_POISON.

Yes, I can see some value in that. Though if that's what it's going to be, IMHO we should use it consistently and give a better name (test_grep or something).

Show 6 quoted lines

> So that part of your patch right now is making it less friendly for
> debugging. I don't care, and think if we want that we'd be better of
> scraping the trace ouput for such common cases and/or use "verbose grep
> [...]" and teach the "verbose" wrapper about these common cases, but
> knowing of that objection + having other outstanding things has been why
> I haven't sent s/test_i18ngrep/grep/g patches.

I was the one who introduced "verbose" long ago, and I did have dreams that people would do "verbose grep" everywhere. But in the end, we made "-x" a lot nicer to use, and I think that largely obsoletes it. Using "verbose" makes the non-x verbose output slightly nicer, perhaps. But not having to remember to sprinkle "verbose" through the test code (nor read it) seems like a bigger win to me.

For the same reason, I'm pretty ambivalent about having test_grep. I suppose one could make a similar argument about "test_path_is_missing", etc, though some of those helpers are also encapsulating technique (e.g., test_dir_is_empty).

-Peff