Re: [RFC PATCH v2 0/4] developer: support pedantic
From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-09-01 11:34:06
[I should have included this in my just-sent [1], but forgot] On Wed, Sep 01 2021, Jeff King wrote:
On Wed, Sep 01, 2021 at 02:19:37AM -0700, Carlo Marcelo Arenas Belón wrote:quoted
first patch was suggested[1] by Peff, so hopefully my commit message and his assumed SoB are still worth not mixing it with patch 2 (which has a slight different but related focus and touches the same files) but since it is no longer a single patch, lets go wild.My SoB is fine there (though really Ævar did the actual thinking; I just deleted a lot of lines in vim :) ). Patch 2 looks good to me, though I kind of wonder if it is even worth having an option to turn it off.quoted
patches 3 and 4 are optional and mostly for RFC, so that a solution to any possible issue that the retiring of USE_PARENS_AROUND_GETTEXT_N are addressed.IMHO the issue it is trying to find is not worth the inevitable problems that hacky perl parsing of C will cause (both false positives and negatives). Not a statement on your perl code, but just based on previous experience. So I'd probably take the first two patches, and leave the others.
Agreed. Per the rationale in my version of the commit messsage for Carlo's 1/4 at [1] I don't think this was ever worth it. I.e. it wasn't even an N_()-specific issue to begin with, but just a migration from usage() (takes a string) to usage_with_options() (takes an array of strings). I just submitted a related series at [2] to fix the alignment of continued strings containing "\n" in parse-options.c, which is the reason we need to support "\n"-continued strings at all in parse-options.c. So I think (per [1]) that we should just remove USE_PARENS_AROUND_GETTEXT_N, and that the 3/4 here isn't needed at all (aside from concerns about parsing C with Perl). But in the future we needed any assertion for this sort of thing at all it would be better built on top of my [2]. I.e. parse-options.c could do some basic sanity checking on the usage array it takes, we'd then end up detecting the issue USE_PARENS_AROUND_GETTEXT_N was trying to address, and more (such as the alignment problems I fixed in 1/2 of my [2]). 1. https://lore.kernel.org/git/patch-1.1-d24f1df5d49-20210901T112248Z-avarab@gmail.com (local) 2. https://lore.kernel.org/git/cover-0.2-00000000000-20210901T110917Z-avarab@gmail.com (local)