GNU bug report logs - #58007
28.1; compilation-error-properties skips non-matching FILE indices

Previous Next

To reply to this bug, email your comments to 58007 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Axel Forsman <axelsfor <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 28.1; compilation-error-properties skips non-matching FILE indices
Date: Thu, 22 Sep 2022 17:09:55 +0200

[Message part 1 (text/plain, inline)]

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

[0001-compile-Do-not-skip-all-non-matching-FILE-indices.patch (text/x-patch, attachment)]

Message #8 received at 58007 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Axel Forsman <axelsfor <at> gmail.com>
Cc: 58007 <at> debbugs.gnu.org
Subject: Re: bug#58007: 28.1; compilation-error-properties skips
 non-matching FILE indices
Date: Fri, 23 Sep 2022 18:10:14 +0200

Axel Forsman <axelsfor <at> 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?

Message #13 received at 58007 <at> debbugs.gnu.org (full text, mbox):

From: Axel Forsman <axelsfor <at> gmail.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 58007 <at> debbugs.gnu.org
Subject: Re: bug#58007: 28.1; compilation-error-properties skips non-matching
 FILE indices
Date: Fri, 23 Sep 2022 20:29:12 +0200

[Message part 1 (text/plain, inline)]

On Fri, Sep 23, 2022 at 6:10 PM Lars Ingebrigtsen <larsi <at> 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.

[rebar3-dialyzer-example-output (application/octet-stream, attachment)]

Message #16 received at 58007 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Axel Forsman <axelsfor <at> gmail.com>
Cc: 58007 <at> debbugs.gnu.org
Subject: Re: bug#58007: 28.1; compilation-error-properties skips
 non-matching FILE indices
Date: Sat, 24 Sep 2022 12:29:53 +0200

Axel Forsman <axelsfor <at> 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

Message #19 received at 58007 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Axel Forsman <axelsfor <at> gmail.com>, Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 58007 <at> debbugs.gnu.org
Subject: Re: bug#58007: 28.1; compilation-error-properties skips non-matching
 FILE indices
Date: Sat, 24 Sep 2022 06:53:21 -0400

Axel Forsman <axelsfor <at> 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.

Message #22 received at 58007 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Axel Forsman <axelsfor <at> gmail.com>, 58007 <at> debbugs.gnu.org
Subject: Re: bug#58007: 28.1; compilation-error-properties skips non-matching
 FILE indices
Date: Wed, 6 Sep 2023 15:50:25 -0700

Lars Ingebrigtsen <larsi <at> 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

Message #25 received at 58007 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Axel Forsman <axelsfor <at> gmail.com>, 58007 <at> debbugs.gnu.org
Subject: Re: bug#58007: 28.1; compilation-error-properties skips non-matching
 FILE indices
Date: Sun, 14 Jan 2024 00:18:36 -0600

Stefan Kangas <stefankangas <at> gmail.com> writes:

> Lars Ingebrigtsen <larsi <at> 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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.