Subject: Re: bug#70794: 30.0.50; Add Rust compilation regex

From: Ergus <[email-masked]>
Date: Mon, 3 Jun 2024 16:41:42 +0200

Hi Mattias:

Thanks for your interest on this. As I said in my first message I just
proposed what I have locally, but I didn't intent to insert the regex
there as it was.

On Fri, May 31, 2024 at 07:15:52PM GMT, Mattias Engdegï¿½rd wrote:
>Ergus, thank you for your contribution! Some questions, if you don't mind:
>
>* There aren't any tests. If you care anything at all about your pattern working, and keep working, you should add some.
>
>We'll help you out, of course, but it's probably best if you give it a go yourself first so that we know exactly what you want to match. Take a look at compile-tests.el, `compile-tests--test-regexps-data`. You will also need to increase the expected counts at the end of `compile-test-error-regexps`.
>
Actually Eli added this commit with the poor information I provided in
my mails because I don't have time to do it myself these days. Sorry for
not being more specific. I actually expected someone else more "rusty" to
provide more complete information (I actually does not use rust with
emacs very often)

>* The regexp doesn't match the variant with error code inside square brackets, the
>
>error[E0425]: cannot find function `ruun` in module `broot::cli`
>
>kind of message. Surely you intended it to?
>
You are right, Checking my local version I have a \\(\\[.+\\]\\)? before
the : in my pattern, not sure when I added that since my first mail.

>Here is your regexp again, translated to rx and with line numbers for
>ease of discussion:
I don't really understand the rx notatio :/ but I will try

>
>1 (rx (| (group-n 4 "error") (group-n 5 "warning"))
>2     ":"
>3     (+? (not (in "\x00")))
>4     "--> "
>5     (group-n 1 (+ (not (in ":"))))
>6     ":"
>7     (group-n 2 (+ digit))
>8     ":"
>9     (group-n 3 (+ digit)))
>

>* (Line 1): From the examples it appears that the messages start at the
>beginning of a line. We very much prefer to anchor matches for reasons
>of performance and to avoid collisions with other patterns. How about
>we add a line-start anchor at the very beginning?
>
It is ok. Probaly you may want to add also a line-end

>* (Line 3): This is an open-ended expression which will merrily match
>just about anything including newlines. This can become very
>expensive. And where does that NUL come from?
>
The NUL is actually negated to match anything up to the -->

>In the examples you supplied, the message is on the first line, and the " --> " on the start of the second. Can we rely on that? If not, what can we rely on?
>
I think yes; that's at least what I tried to match. If you have a
better/more efficient alternative fell free to change it.

>* (Line 5): This excludes Windows file names which can include colon or do cargo messages have a special notation for those? In any case, it's usually a good idea to exclude newline as well to prevent a runaway repetition.
>
In this case I think not, because the filename is always relative in
cargo and on MSWindows the colon comes after the drive in absolute paths
right? But I could be wrong.

>* Both the rust-mode and rustic packages appear to include regexps that match the same messages. How do they compare to yours?
>
You can take those indeed. I think they must be pretty much equivalent
and probably more complete because they probably match the "note:" case
which I preferred to ignore as emacs does not have an equivalent for
that.

IIRC they just don't use numeric capture groups, which I like a bit more
t avoid weird errors, but at the end the regex is pretty simple, any
alternative is fine.

>What is the effect of someone using those modes with your regexp in Emacs?
>
>
IIUC the packages pushes the entries in front of
compilation-error-regexp-alist-alist, so in principle their regexp will
match first and the loop stops.

So, the main changes you propose are something like:

"^\\(?:\\(?4:error\\)\\|\\(?5:warning\\)\\)\\(?:\\[.+\\]\\)?:[^\0]+?--> \\(?1:[^:]+\\):\\(?2:[[:digit:]]+\\):\\(?3:[[:digit:]]+\\)"