On Thu, Sep 05, 2024 at 12:57:54PM +0200, Patrick Steinhardt wrote:
> > I just happened to notice one of these, so I grepped for more.
>
> Do we maybe want to add a script that detects these via a Coccinelle
> script? As it turns out, Coccinelle has an embedded Python interpreter
> that allow us to extend it with arbitrary checks. So the following
> script would check for trailing newlines in `die()`, `die_errno()`,
> `error()` and `warning()`:
If you want to pursue this, I certainly have no objection, but
personally I have burned enough time trying to make Coccinelle work that
I don't want to fall down that particular rabbit hole again. ;)
I think long ago we tried to avoid using the embedded python, because
not all builds had it (notably Debian's). But I think that may not be
the case anymore.
If you did want to do automated quality checks of error messages, there
are probably a few other common pitfalls we could detect (like ending with
a full stop ".", and starting with a capital).
> unpack-trees.c:408: trailing newline in "the following paths have collided (e.g. case-sensitive paths\n" "on a case-insensitive filesystem) and only one from the same\n" "colliding group is in the working tree:\n"
>
> The last three lines are a bit of a false positive, I think. All of
> these calls are `warning()` messages though, so we could potentially
> just drop those conversions.
Yeah, mine didn't find those because it looked for "warning(" on the
same source code line as the trailing newline. Which is sort of sloppy,
but also kind of works because any message long enough to require
multiple lines probably meant the author knew what they were doing. :)
-Peff
