* bug#58007: 28.1; compilation-error-properties skips non-matching FILE indices
@ 2022-09-22 15:09 Axel Forsman
2022-09-23 16:10 ` Lars Ingebrigtsen
0 siblings, 1 reply; 7+ messages in thread
From: Axel Forsman @ 2022-09-22 15:09 UTC (permalink / raw)
To: 58007
[-- Attachment #1: Type: text/plain, Size: 1198 bytes --]
Greetings:
The documentation for compilation-error-regexp-alist states
> If FILE, LINE or COLUMN are nil or that index didn’t match, that
> information is not present on the matched line. In that case the
> file name is assumed to be the same as the previous one in the
> buffer, line number defaults to 1 and column defaults to
> beginning of line’s indentation.
which clearly, unambiguously implies that if the FILE index does not match
then the previous file name should be used for the error.
However that is not what the code does currently,
instead it skips such matches.
I believe the documented behavior would be strictly more useful
than the current implemented behavior,
since if unwanted then the file subexpression in the regex should be
non-optional anyway.
My use case would be to more easily match errors stretching multiple lines,
where the file name is only mentioned in a header.
I have attached a patch containing my suggested fix.
On a side note, the code for handling the case when FILE is a function
that returns a (RELATIVE-FILENAME . DIRNAME) cons-cell looks a bit funky.
It could probably be cleaned up.
Kind regard
Axel Forsman
[-- Attachment #2: 0001-compile-Do-not-skip-all-non-matching-FILE-indices.patch --]
[-- Type: text/x-patch, Size: 3681 bytes --]
From d9359efd27bb88b9400ce0147cec38290293fad8 Mon Sep 17 00:00:00 2001
From: Axel Forsman <axelsfor@gmail.com>
Date: Thu, 22 Sep 2022 16:12:19 +0200
Subject: [PATCH] compile: Do not skip all non-matching FILE indices
* lisp/progmodes/compile.el (compilation-error-properties): Use
previous file name in case of non-matching FILE index.
---
lisp/progmodes/compile.el | 58 ++++++++++++++++++---------------------
1 file changed, 26 insertions(+), 32 deletions(-)
diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
index ded5d2130e..90daf10b9b 100644
--- a/lisp/progmodes/compile.el
+++ b/lisp/progmodes/compile.el
@@ -1257,38 +1257,32 @@ POS and RES.")
rule)
(unless (text-property-not-all (match-beginning 0) (point)
'compilation-message nil)
- (if file
- (when (stringp
- (setq file (if (functionp file) (funcall file)
- (match-string-no-properties file))))
- (let ((dir
- (unless (file-name-absolute-p file)
- (let ((pos (compilation--previous-directory
- (match-beginning 0))))
- (when pos
- (or (get-text-property (1- pos) 'compilation-directory)
- (get-text-property pos 'compilation-directory)))))))
- (setq file (cons file (car dir)))))
- ;; This message didn't mention one, get it from previous
- (let ((prev-pos
- ;; Find the previous message.
- (previous-single-property-change (point) 'compilation-message)))
- (if prev-pos
- ;; Get the file structure that belongs to it.
- (let* ((prev
- (or (get-text-property (1- prev-pos) 'compilation-message)
- (get-text-property prev-pos 'compilation-message)))
- (prev-file-struct
- (and prev
- (compilation--loc->file-struct
- (compilation--message->loc prev)))))
-
- ;; Construct FILE . DIR from that.
- (if prev-file-struct
- (setq file (cons (caar prev-file-struct)
- (cadr (car prev-file-struct)))))))
- (unless file
- (setq file '("*unknown*")))))
+ (setq
+ file
+ (if-let ((filename (cond ((functionp file) (funcall file))
+ (file (match-string-no-properties file)))))
+ (let ((dir
+ (unless (file-name-absolute-p filename)
+ (let ((pos (compilation--previous-directory
+ (match-beginning 0))))
+ (when pos
+ (or (get-text-property (1- pos) 'compilation-directory)
+ (get-text-property pos 'compilation-directory)))))))
+ (cons filename (car dir)))
+ (unless (functionp file)
+ ;; This message didn't mention a file, get it from previous
+ (if-let*
+ ((prev-pos
+ ;; Find the previous message.
+ (previous-single-property-change (point) 'compilation-message))
+ ;; Get the file structure that belongs to it.
+ (prev (or (get-text-property (1- prev-pos) 'compilation-message)
+ (get-text-property prev-pos 'compilation-message)))
+ (prev-file-struct (compilation--loc->file-struct
+ (compilation--message->loc prev))))
+ ;; Construct FILE . DIR from that.
+ (cons (caar prev-file-struct) (cadr (car prev-file-struct)))
+ '("*unknown*")))))
;; All of these fields are optional, get them only if we have an index, and
;; it matched some part of the message.
(setq line
--
2.36.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* bug#58007: 28.1; compilation-error-properties skips non-matching FILE indices
2022-09-22 15:09 bug#58007: 28.1; compilation-error-properties skips non-matching FILE indices Axel Forsman
@ 2022-09-23 16:10 ` Lars Ingebrigtsen
2022-09-23 18:29 ` Axel Forsman
0 siblings, 1 reply; 7+ messages in thread
From: Lars Ingebrigtsen @ 2022-09-23 16:10 UTC (permalink / raw)
To: Axel Forsman; +Cc: 58007
Axel Forsman <axelsfor@gmail.com> writes:
> which clearly, unambiguously implies that if the FILE index does not match
> then the previous file name should be used for the error.
> However that is not what the code does currently,
> instead it skips such matches.
Do you have a test case that demonstrates the problem?
> I have attached a patch containing my suggested fix.
I think this change might be too large to apply without having an FSF
copyright assignment on file -- would you be willing to sign such
paperwork?
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#58007: 28.1; compilation-error-properties skips non-matching FILE indices
2022-09-23 16:10 ` Lars Ingebrigtsen
@ 2022-09-23 18:29 ` Axel Forsman
2022-09-24 10:29 ` Lars Ingebrigtsen
2022-09-24 10:53 ` Stefan Kangas
0 siblings, 2 replies; 7+ messages in thread
From: Axel Forsman @ 2022-09-23 18:29 UTC (permalink / raw)
To: Lars Ingebrigtsen; +Cc: 58007
[-- Attachment #1: Type: text/plain, Size: 1407 bytes --]
On Fri, Sep 23, 2022 at 6:10 PM Lars Ingebrigtsen <larsi@gnus.org> wrote:
> Do you have a test case that demonstrates the problem?
Yes, so what I initially wanted to do was parse dialyzer errors
when running using the rebar3 package manager with `rebar3 dialyzer`.
See attachment for example output.
This is what I ended up with
(setq compilation-error-regexp-alist
(cons
'("^\\(?:\n\\(.*\\)\n\\)?Line \\([0-9]+\\)\\(?: Column
\\([0-9]+\\)\\)?: " 1 2 3 1)
(cons
;; This next error regexp should not be necessary
according to documentation.
'("^Line \\([0-9]+\\)\\(?: Column \\([0-9]+\\)\\)?: " nil 1 2 1)
(eval (car (get 'compilation-error-regexp-alist
'standard-value))))))
only with the 2nd error regexp does all errors get parsed.
Though this is unexpected: In the 1st regexp
when the first subexpression containing the file name does not match,
the docs say it should act the same as the 2nd error regexp.
When I have `eval-defun`:ed the patched version of
`compilation-error-properties`,
the 1st error regexp suffices as expected.
> I think this change might be too large to apply without having an FSF
> copyright assignment on file -- would you be willing to sign such
> paperwork?
The patch is mostly all indentation changes,
but yes, if needed I would be willing
though I would have to consult with my employer first.
[-- Attachment #2: rebar3-dialyzer-example-output --]
[-- Type: application/octet-stream, Size: 1193 bytes --]
-*- mode: compilation; default-directory: "~/Documents/rebar3-test/mylib/" -*-
Compilation started at Fri Sep 23 20:06:43
rebar3 dialyzer
===> Verifying dependencies...
===> Analyzing applications...
===> Compiling mylib
===> Dialyzer starting, this may take a while...
===> Updating plt...
===> Resolving files...
===> Checking 205 files in _build/default/rebar3_24.2_plt...
===> Doing success typing analysis...
===> Resolving files...
===> Analyzing 2 files with _build/default/rebar3_24.2_plt...
src/mylib_app.erl
Line 12 Column 1: Function test/1 has no local return
Line 12 Column 1: The pattern [] can never match the type 4
Line 15 Column 1: Function start/2 has no local return
Line 16 Column 7: The call mylib_app:test(4) will never return since it differs in the 1st argument from the success typing arguments: ([])
src/mylib_sup.erl
Line 16 Column 1: Function fin/1 has no local return
Line 16 Column 1: The pattern [] can never match the type 4
Line 19 Column 1: Function start_link/0 has no local return
===> Warnings written to _build/default/24.2.dialyzer_warnings
===> Warnings occurred running dialyzer: 7
Compilation exited abnormally with code 1 at Fri Sep 23 20:06:51
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#58007: 28.1; compilation-error-properties skips non-matching FILE indices
2022-09-23 18:29 ` Axel Forsman
@ 2022-09-24 10:29 ` Lars Ingebrigtsen
2023-09-06 22:50 ` Stefan Kangas
2022-09-24 10:53 ` Stefan Kangas
1 sibling, 1 reply; 7+ messages in thread
From: Lars Ingebrigtsen @ 2022-09-24 10:29 UTC (permalink / raw)
To: Axel Forsman; +Cc: 58007
Axel Forsman <axelsfor@gmail.com> writes:
> Yes, so what I initially wanted to do was parse dialyzer errors
> when running using the rebar3 package manager with `rebar3 dialyzer`.
> See attachment for example output.
>
> This is what I ended up with
>
> (setq compilation-error-regexp-alist
> (cons
> '("^\\(?:\n\\(.*\\)\n\\)?Line \\([0-9]+\\)\\(?: Column
> \\([0-9]+\\)\\)?: " 1 2 3 1)
> (cons
> ;; This next error regexp should not be necessary
> according to documentation.
> '("^Line \\([0-9]+\\)\\(?: Column \\([0-9]+\\)\\)?: " nil 1 2 1)
> (eval (car (get 'compilation-error-regexp-alist
> 'standard-value))))))
>
> only with the 2nd error regexp does all errors get parsed.
> Though this is unexpected: In the 1st regexp
> when the first subexpression containing the file name does not match,
> the docs say it should act the same as the 2nd error regexp.
>
> When I have `eval-defun`:ed the patched version of
> `compilation-error-properties`,
> the 1st error regexp suffices as expected.
Ah, thanks.
>> I think this change might be too large to apply without having an FSF
>> copyright assignment on file -- would you be willing to sign such
>> paperwork?
>
> The patch is mostly all indentation changes,
> but yes, if needed I would be willing
> though I would have to consult with my employer first.
Reading the patch again, you're right -- since most of the changes are
indentation changes, we can apply the patch without a copyright
assignment.
However, your patch leads to a test failure -- can you have a look at
that?
Test compile-test-functions backtrace:
file-name-absolute-p(("my-file"))
compilation-error-properties((closure (t) nil '("my-file")) (closure
compilation-parse-errors(1 17)
(let ((rule (nth 0 test)) (str (nth 1 test)) (pos (nth 2 test)) (col
(let ((ert--infos (cons (cons "testcase: " (format "%S" test)) ert--
compile--test-error-line((my-rule "My error message" 1 (39 . 24) (12
(let ((compilation-num-errors-found 0) (compilation-num-warnings-fou
(progn (font-lock-mode -1) (let ((compilation-num-errors-found 0) (c
(unwind-protect (progn (font-lock-mode -1) (let ((compilation-num-er
(save-current-buffer (set-buffer temp-buffer) (unwind-protect (progn
(let ((temp-buffer (generate-new-buffer " *temp*" t))) (save-current
(let* ((file-fun #'(lambda nil '("my-file"))) (line-start-fun #'(lam
(closure (t) nil (let* ((file-fun #'(lambda nil '("my-file"))) (line
ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
ert-run-test(#s(ert-test :name compile-test-functions :documentation
ert-run-or-rerun-test(#s(ert--stats :selector ... :tests ... :test-m
ert-run-tests((not (or (tag :expensive-test) (tag :unstable))) #f(co
ert-run-tests-batch((not (or (tag :expensive-test) (tag :unstable)))
ert-run-tests-batch-and-exit((not (or (tag :expensive-test) (tag :un
eval((ert-run-tests-batch-and-exit '(not (or (tag :expensive-test) (
command-line-1(("-L" ":." "-l" "ert" "-l" "lisp/progmodes/compile-te
command-line()
normal-top-level()
Test compile-test-functions condition:
testcase: (my-rule "My error message" 1 (39 . 24) (123 . 134) "my-file" 2)
(wrong-type-argument stringp
("my-file"))
FAILED 2/3 compile-test-functions (0.000089 sec) at lisp/progmodes/compile-tests.el:522
passed 3/3 compile-test-grep-regexps (0.001284 sec)
Ran 3 tests, 2 results as expected, 1 unexpected (2022-09-24 12:26:55+0200, 0.083845 sec)
1 unexpected results:
FAILED compile-test-functions
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#58007: 28.1; compilation-error-properties skips non-matching FILE indices
2022-09-23 18:29 ` Axel Forsman
2022-09-24 10:29 ` Lars Ingebrigtsen
@ 2022-09-24 10:53 ` Stefan Kangas
1 sibling, 0 replies; 7+ messages in thread
From: Stefan Kangas @ 2022-09-24 10:53 UTC (permalink / raw)
To: Axel Forsman, Lars Ingebrigtsen; +Cc: 58007
Axel Forsman <axelsfor@gmail.com> writes:
> The patch is mostly all indentation changes,
> but yes, if needed I would be willing
> though I would have to consult with my employer first.
It's not a bad idea to start the process now, if you plan to contribute
more to the Emacs in the future. We hope that you do, of course.
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#58007: 28.1; compilation-error-properties skips non-matching FILE indices
2022-09-24 10:29 ` Lars Ingebrigtsen
@ 2023-09-06 22:50 ` Stefan Kangas
2024-01-14 6:18 ` Stefan Kangas
0 siblings, 1 reply; 7+ messages in thread
From: Stefan Kangas @ 2023-09-06 22:50 UTC (permalink / raw)
To: Lars Ingebrigtsen; +Cc: Axel Forsman, 58007
Lars Ingebrigtsen <larsi@gnus.org> writes:
> Reading the patch again, you're right -- since most of the changes are
> indentation changes, we can apply the patch without a copyright
> assignment.
>
> However, your patch leads to a test failure -- can you have a look at
> that?
That was one year ago.
Axel, did you have a chance to look into the below test failure?
> Test compile-test-functions backtrace:
> file-name-absolute-p(("my-file"))
> compilation-error-properties((closure (t) nil '("my-file")) (closure
> compilation-parse-errors(1 17)
> (let ((rule (nth 0 test)) (str (nth 1 test)) (pos (nth 2 test)) (col
> (let ((ert--infos (cons (cons "testcase: " (format "%S" test)) ert--
> compile--test-error-line((my-rule "My error message" 1 (39 . 24) (12
> (let ((compilation-num-errors-found 0) (compilation-num-warnings-fou
> (progn (font-lock-mode -1) (let ((compilation-num-errors-found 0) (c
> (unwind-protect (progn (font-lock-mode -1) (let ((compilation-num-er
> (save-current-buffer (set-buffer temp-buffer) (unwind-protect (progn
> (let ((temp-buffer (generate-new-buffer " *temp*" t))) (save-current
> (let* ((file-fun #'(lambda nil '("my-file"))) (line-start-fun #'(lam
> (closure (t) nil (let* ((file-fun #'(lambda nil '("my-file"))) (line
> ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
> ert-run-test(#s(ert-test :name compile-test-functions :documentation
> ert-run-or-rerun-test(#s(ert--stats :selector ... :tests ... :test-m
> ert-run-tests((not (or (tag :expensive-test) (tag :unstable))) #f(co
> ert-run-tests-batch((not (or (tag :expensive-test) (tag :unstable)))
> ert-run-tests-batch-and-exit((not (or (tag :expensive-test) (tag :un
> eval((ert-run-tests-batch-and-exit '(not (or (tag :expensive-test) (
> command-line-1(("-L" ":." "-l" "ert" "-l" "lisp/progmodes/compile-te
> command-line()
> normal-top-level()
> Test compile-test-functions condition:
> testcase: (my-rule "My error message" 1 (39 . 24) (123 . 134) "my-file" 2)
> (wrong-type-argument stringp
> ("my-file"))
> FAILED 2/3 compile-test-functions (0.000089 sec) at lisp/progmodes/compile-tests.el:522
> passed 3/3 compile-test-grep-regexps (0.001284 sec)
>
> Ran 3 tests, 2 results as expected, 1 unexpected (2022-09-24 12:26:55+0200, 0.083845 sec)
>
> 1 unexpected results:
> FAILED compile-test-functions
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#58007: 28.1; compilation-error-properties skips non-matching FILE indices
2023-09-06 22:50 ` Stefan Kangas
@ 2024-01-14 6:18 ` Stefan Kangas
0 siblings, 0 replies; 7+ messages in thread
From: Stefan Kangas @ 2024-01-14 6:18 UTC (permalink / raw)
To: Lars Ingebrigtsen; +Cc: Axel Forsman, 58007
Stefan Kangas <stefankangas@gmail.com> writes:
> Lars Ingebrigtsen <larsi@gnus.org> writes:
>
>> Reading the patch again, you're right -- since most of the changes are
>> indentation changes, we can apply the patch without a copyright
>> assignment.
>>
>> However, your patch leads to a test failure -- can you have a look at
>> that?
>
> That was one year ago.
>
> Axel, did you have a chance to look into the below test failure?
Ping.
>> Test compile-test-functions backtrace:
>> file-name-absolute-p(("my-file"))
>> compilation-error-properties((closure (t) nil '("my-file")) (closure
>> compilation-parse-errors(1 17)
>> (let ((rule (nth 0 test)) (str (nth 1 test)) (pos (nth 2 test)) (col
>> (let ((ert--infos (cons (cons "testcase: " (format "%S" test)) ert--
>> compile--test-error-line((my-rule "My error message" 1 (39 . 24) (12
>> (let ((compilation-num-errors-found 0) (compilation-num-warnings-fou
>> (progn (font-lock-mode -1) (let ((compilation-num-errors-found 0) (c
>> (unwind-protect (progn (font-lock-mode -1) (let ((compilation-num-er
>> (save-current-buffer (set-buffer temp-buffer) (unwind-protect (progn
>> (let ((temp-buffer (generate-new-buffer " *temp*" t))) (save-current
>> (let* ((file-fun #'(lambda nil '("my-file"))) (line-start-fun #'(lam
>> (closure (t) nil (let* ((file-fun #'(lambda nil '("my-file"))) (line
>> ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
>> ert-run-test(#s(ert-test :name compile-test-functions :documentation
>> ert-run-or-rerun-test(#s(ert--stats :selector ... :tests ... :test-m
>> ert-run-tests((not (or (tag :expensive-test) (tag :unstable))) #f(co
>> ert-run-tests-batch((not (or (tag :expensive-test) (tag :unstable)))
>> ert-run-tests-batch-and-exit((not (or (tag :expensive-test) (tag :un
>> eval((ert-run-tests-batch-and-exit '(not (or (tag :expensive-test) (
>> command-line-1(("-L" ":." "-l" "ert" "-l" "lisp/progmodes/compile-te
>> command-line()
>> normal-top-level()
>> Test compile-test-functions condition:
>> testcase: (my-rule "My error message" 1 (39 . 24) (123 . 134) "my-file" 2)
>> (wrong-type-argument stringp
>> ("my-file"))
>> FAILED 2/3 compile-test-functions (0.000089 sec) at lisp/progmodes/compile-tests.el:522
>> passed 3/3 compile-test-grep-regexps (0.001284 sec)
>>
>> Ran 3 tests, 2 results as expected, 1 unexpected (2022-09-24 12:26:55+0200, 0.083845 sec)
>>
>> 1 unexpected results:
>> FAILED compile-test-functions
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-01-14 6:18 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-22 15:09 bug#58007: 28.1; compilation-error-properties skips non-matching FILE indices Axel Forsman
2022-09-23 16:10 ` Lars Ingebrigtsen
2022-09-23 18:29 ` Axel Forsman
2022-09-24 10:29 ` Lars Ingebrigtsen
2023-09-06 22:50 ` Stefan Kangas
2024-01-14 6:18 ` Stefan Kangas
2022-09-24 10:53 ` Stefan Kangas
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).