compilation-error-regexp does not always match test outputs

[mplanchard]

I'm not totally sure when this changed, but the compilation-error-regexp defined by rustic no longer matches the the output I get when a test assertion fails.

My test output generally looks like this:

thread 'module::submodule::subsubmodule' panicked at lib/rs/tests/test-module/models/db-model.rs:454:5:

The value added for rustic-panic in compilation-error-regexp-alist-alist is currently:

"thread '[^']+' panicked at '[^']+', \\([^\n]+\\):\\([0-9]+\\):\\([0-9]+\\)"

The inclusion of the `'[^']+', after "panicked at" is what seems to prevent the match.

I have worked around it locally by adding the following to my use-package config for rustic:

(use-package rustic
  :hook
  :hook
  (rustic-cargo-test-mode
   . (lambda () (add-to-list 'compilation-error-regexp-alist
                             `(,(rx "thread '"
                                    (one-or-more (not "'"))
                                    "' panicked at "
                                    (group (one-or-more not-newline))
                                    ":"
                                    (group (one-or-more digit))
                                    ":"
                                    (group (one-or-more digit)))
                               1 2 3))))

which generates the regexp "thread '[^']+' panicked at \\(.+\\):\\([[:digit:]]+\\):\\([[:digit:]]+\\)".

With this in place, jumping back and forth between tests in the rustic test output compilation buffer works again as expected, along with jumping directly to the failing test.

[neosimsim]

Interestingly I was working on the same issue today, with a similar workaround. I'm wondering if you're sure that this ever worked, because compilation-error-regexp-alist only contains matches for rustic-panic, rustic-info, rustic-warning and rustic-error.

I was thinking of providing a MR which adds a regex like yours to compilation-error-regexp-alist on define-derived-mode for cargo-test-mode once I find some time in the next days. That is, if that would be an appropriate solution.

[mplanchard]

I'm wondering if you're sure that this ever worked, because compilation-error-regexp-alist only contains matches for rustic-panic, rustic-info, rustic-warning and rustic-error.

The compilation-error-regexp-alist is a list either of alist-style entries like the one I inserted above or symbols pointing to entries in compilation-error-regexp-alist-alist, for whatever reason. The layer of indirection makes it confusing, but the item for rustic-panic is the one I mentioned above. It's only there in cargo compilation buffers, but if you go to one of those and then do M-x describe-variable and look at the alist-alist one, you'll see:

Value in #<buffer *cargo-clippy*>
((rustic-panic
  "thread '[^']+' panicked at '[^']+', \\([^\n]+\\):\\([0-9]+\\):\\([0-9]+\\)"
  1 2 3)
 (rustic-info "^ *::: \\([^\n]+\\):\\([0-9]+\\):\\([0-9]+\\)" 1 2 3 0)
 (rustic-warning
  "^warning:[^\n]*\n *--> \\([^\n]+\\):\\([0-9]+\\):\\([0-9]+\\)" 1 2
  3 1)
 (rustic-error
  "^error[^:]*:[^\n]*\n *--> \\([^\n]+\\):\\([0-9]+\\):\\([0-9]+\\)" 1
  2 3))

I think the most correct solution might be to add a named entry to compilation-error-regexp-alist-alist like

(add-to-list 'compilation-error-regexp-alist-alist
             (cons 'rustic-new
                   `(,(rx "thread '"
                          (one-or-more (not "'"))
                          "' panicked at "
                          (group (one-or-more not-newline))
                          ":"
                          (group (one-or-more digit))
                          ":"
                          (group (one-or-more digit)))
                     1 2 3)))

and then just add rustic-panic-new to the compilation-error-regexp-alist. If we could figure out which version of rust introduced the change, it would be nice to name it like rustic-panic-1-74 or whatever, according to the version, that way anyone using an older version will still have the old matching logic in there to use.

Seems like the current logic is defined here:

rustic/rustic-compile.el

Line 190 in 39423d1

(setq-local compilation-error-regexp-alist-alist nil)

I do believe the current regex did work at some point in the past. I don't remember when it stopped working, but I've just been living with it for a while. Probably a recent-ish version of rust updated the test output. I've been working on setting up a from-scratch emacs config over the past few days though, so it was an opportunity to go and re-investigate.

[mplanchard]

Oh as soon as I posted I realized I might have misunderstood. If you're saying the rustic-panic regex matches a panic in regular cargo run output but not test output, that is definitely possible (I haven't looked). I do know that it used to work for tests, too, though

[neosimsim]

I pushed a suggestion here #574.

[neosimsim]

Ah, I see now. rustic-compilation-panic expects the filepath to be sorrunded by single quotes which is not the case for panics in tests. I believe I've never seen a panic at compile time. So I don't know how those panics are printed.
Maybe two regexps are required or it rust has indeed changed the output.

[neosimsim]

The inclusion of the `'[^']+', after "panicked at" is what seems to prevent the match.

I believe you're right @mplanchard . That seems to be the issue.

So a proper fix would be to just remove that part?

[neosimsim]

@mplanchard did you find some time to have a look at #575?
Does this change make more sense to you compared to #574?

[mplanchard]

Yeah I think #575 makes sense to me, since I don't know of any cases where the old regex matches

[neosimsim]

I created a PR fixing this in emacs-rustic/rustic#31.