Thread (75 messages) 75 messages, 10 authors, 2021-09-28

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)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help