Subject: bug#58007: 28.1; compilation-error-properties skips non-matching FILE indices

From: Axel Forsman <[email-masked]>
Date: Thu, 22 Sep 2022 17:09:55 +0200

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
From d9359efd27bb88b9400ce0147cec38290293fad8 Mon Sep 17 00:00:00 2001
From: Axel Forsman <[email-masked]>
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