bug#40119: [PATCH] Make compilation-mode regexp matching case-sensitive

bug#40119: [PATCH] Make compilation-mode regexp matching case-sensitive

[Mattias Engdegård]

[-- Attachment #1: Type: text/plain, Size: 431 bytes --]

Since the regexps being matched in compilation-mode are numerous, independently written, and often intersect, it makes sense to use case-sensitive matching. After all, compiler messages do not change case between runs.

Doing so improves performance: case-insensitive matching is slightly slower and stricter matching allows for earlier rejection. It also reduces collisions, and it is probably what everybody assumed anyway.


[-- Attachment #2: 0001-Make-compilation-mode-regexp-matching-case-sensitive.patch --]
[-- Type: application/octet-stream, Size: 1718 bytes --]

From c48d1607d2401f1bd6e05a3f44c7f9ec8ec30b91 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Wed, 18 Mar 2020 16:01:02 +0100
Subject: [PATCH] Make compilation-mode regexp matching case-sensitive

The number of regexps is large, they are often written independently
of one another, and they frequently intersect.  Enforcing
case-sensitive matching improves separation and performance, and
is probably what everyone have being assuming all along.

* lisp/progmodes/compile.el (compilation-parse-errors):
Bind case-fold-search to nil during matching.
* etc/NEWS: Announce.
---
 etc/NEWS                  | 4 ++++
 lisp/progmodes/compile.el | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/etc/NEWS b/etc/NEWS
index 87e634f2c1..8bea9b2d5f 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -174,6 +174,10 @@ key             binding
 / v             package-menu-filter-by-version
 / /             package-menu-filter-clear
 
+** Compilation mode
+
+*** Regexp matching of messages is now case-sensitive.
+
 \f
 * New Modes and Packages in Emacs 28.1
 
diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
index 455f181f50..aefaa1707a 100644
--- a/lisp/progmodes/compile.el
+++ b/lisp/progmodes/compile.el
@@ -1435,7 +1435,8 @@ compilation-parse-errors
     (if (symbolp item)
         (setq item (cdr (assq item
                               compilation-error-regexp-alist-alist))))
-    (let ((file (nth 1 item))
+    (let ((case-fold-search nil)
+          (file (nth 1 item))
           (line (nth 2 item))
           (col (nth 3 item))
           (type (nth 4 item))
-- 
2.21.1 (Apple Git-122.3)

bug#40119: [PATCH] Make compilation-mode regexp matching case-sensitive

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Wed, 18 Mar 2020 16:31:49 +0100
> 
> +** Compilation mode
> +
> +*** Regexp matching of messages is now case-sensitive.
> +
>  \f
>  * New Modes and Packages in Emacs 28.1
>  
> diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
> index 455f181f50..aefaa1707a 100644
> --- a/lisp/progmodes/compile.el
> +++ b/lisp/progmodes/compile.el
> @@ -1435,7 +1435,8 @@ compilation-parse-errors
>      (if (symbolp item)
>          (setq item (cdr (assq item
>                                compilation-error-regexp-alist-alist))))
> -    (let ((file (nth 1 item))
> +    (let ((case-fold-search nil)
> +          (file (nth 1 item))
>            (line (nth 2 item))
>            (col (nth 3 item))
>            (type (nth 4 item))

What if we are wrong and some compiler needs case-insensitive
matching?  Your change makes it impossible to get back the old
behavior, not even optionally.

Thanks.

bug#40119: [PATCH] Make compilation-mode regexp matching case-sensitive

[Mattias Engdegård]

18 mars 2020 kl. 19.05 skrev Eli Zaretskii <eliz@gnu.org>:

> What if we are wrong and some compiler needs case-insensitive
> matching?  Your change makes it impossible to get back the old
> behavior, not even optionally.

Do you mean that we should add a defcustom to permit the user to control the behaviour, or that doing so is impossible once rules are added which rely on the case-sensitivity?

Either way, it seems much more likely that it is case-insensitivity that is an obstacle to adding new rules, not the contrary. Could you give an example of what such a hypothetical compiler needing case-insensitive matching would look like?

bug#40119: [PATCH] Make compilation-mode regexp matching case-sensitive

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Wed, 18 Mar 2020 19:30:23 +0100
> Cc: 40119@debbugs.gnu.org
> 
> 18 mars 2020 kl. 19.05 skrev Eli Zaretskii <eliz@gnu.org>:
> 
> > What if we are wrong and some compiler needs case-insensitive
> > matching?  Your change makes it impossible to get back the old
> > behavior, not even optionally.
> 
> Do you mean that we should add a defcustom to permit the user to control the behaviour, or that doing so is impossible once rules are added which rely on the case-sensitivity?

The former.

> Either way, it seems much more likely that it is case-insensitivity that is an obstacle to adding new rules, not the contrary.

I don't understand what adding new rules has to do with this.  I'm
talking about a specific rule that needs to be matched
case-insensitively.

> Could you give an example of what such a hypothetical compiler needing case-insensitive matching would look like?

Sorry, I don't understand the question.  What I meant is that some of
the rules were written _knowing_ that the match is case-insensitive,
and will fail to match if that is changed.  I don't think there's
anyone around here that can claim detailed knowledge of every rule and
the compiler(s) that use them, so we have no one to ask whether this
danger is real or imaginary.

bug#40119: [PATCH] Make compilation-mode regexp matching case-sensitive

[Mattias Engdegård]

[-- Attachment #1: Type: text/plain, Size: 1382 bytes --]

18 mars 2020 kl. 20.29 skrev Eli Zaretskii <eliz@gnu.org>:

> What I meant is that some of
> the rules were written _knowing_ that the match is case-insensitive,
> and will fail to match if that is changed.  I don't think there's
> anyone around here that can claim detailed knowledge of every rule and
> the compiler(s) that use them, so we have no one to ask whether this
> danger is real or imaginary.

Thank you for clarifying! All the tests pass after the change, and etc/compilation.txt is rendered almost identically. There is only one difference: the example for 'watcom' did not actually match that rule, but rule 'edg-1' (which comes earlier in the list) instead. With case-fold-search set to nil, the 'watcom' example is matched by its rule and not by edg-1.

In other words, it is unlikely that the existing rules in compile.el expect case-insensitive matching of the output for which they are written. Conversely, there is already evidence that case-insensitive matching causes the wrong rule to be used.

Of course we cannot exclude that rules are used with compilers other than for which the patterns were designed. The correct way to handle this should be to add a suitable rule, not to change how all rules match. Nevertheless, I added a user switch to turn case-insensitivity back on again for those who prefer it that way.

Revised patch follows.

[-- Attachment #2: 0001-Make-compilation-mode-regexp-matching-case-sensitive.patch --]
[-- Type: application/octet-stream, Size: 2612 bytes --]

From 311d51e33d290c7d28147dfd3a1701964f102ad3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Wed, 18 Mar 2020 16:01:02 +0100
Subject: [PATCH] Make compilation-mode regexp matching case-sensitive

The number of regexps is large, they are often written independently
of one another, and they frequently intersect.  Using case-sensitive
matching improves separation and performance, and is probably what
everyone have being assuming all along.

* lisp/progmodes/compile.el (compilation-error-case-fold-search): New.
* lisp/progmodes/compile.el (compilation-parse-errors):
Bind case-fold-search to compilation-error-case-fold-search during matching.
* etc/NEWS: Announce.
---
 etc/NEWS                  |  6 ++++++
 lisp/progmodes/compile.el | 11 ++++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/etc/NEWS b/etc/NEWS
index 87e634f2c1..14a4bb4891 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -174,6 +174,12 @@ key             binding
 / v             package-menu-filter-by-version
 / /             package-menu-filter-clear
 
+** Compilation mode
+
+*** Regexp matching of messages is now case-sensitive by default.
+The user option 'compilation-error-case-fold-search' can be set
+for case-insensitive matching of messages.
+
 \f
 * New Modes and Packages in Emacs 28.1
 
diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
index 455f181f50..f4532b7edb 100644
--- a/lisp/progmodes/compile.el
+++ b/lisp/progmodes/compile.el
@@ -646,6 +646,14 @@ compilation-error-regexp-alist
   :link `(file-link :tag "example file"
 		    ,(expand-file-name "compilation.txt" data-directory)))
 
+(defcustom compilation-error-case-fold-search nil
+  "If non-nil, use case-insensitive matching of compilation errors
+by the regexps of `compilation-error-regexp-alist' and
+`compilation-error-regexp-alist-alist'.
+If nil, matching is case-sensitive."
+  :type 'boolean
+  :version "28.1")
+
 ;;;###autoload(put 'compilation-directory 'safe-local-variable 'stringp)
 (defvar compilation-directory nil
   "Directory to restore to when doing `recompile'.")
@@ -1435,7 +1443,8 @@ compilation-parse-errors
     (if (symbolp item)
         (setq item (cdr (assq item
                               compilation-error-regexp-alist-alist))))
-    (let ((file (nth 1 item))
+    (let ((case-fold-search compilation-error-case-fold-search)
+          (file (nth 1 item))
           (line (nth 2 item))
           (col (nth 3 item))
           (type (nth 4 item))
-- 
2.21.1 (Apple Git-122.3)

bug#40119: [PATCH] Make compilation-mode regexp matching case-sensitive

[Mattias Engdegård]

No objections detected -- patch pushed to master.

bug#40119: [PATCH] Make compilation-mode regexp matching case-sensitive

[Dmitry Gutov]

On 18.03.2020 21:29, Eli Zaretskii wrote:
> What I meant is that some of
> the rules were written_knowing_  that the match is case-insensitive,
> and will fail to match if that is changed.

Suppose some author discovers that about one of their rules (or 
several). Would it be too hard to fix those rules instead of changing 
the user option?

Because said variable can affect all other rules as well. And other 
compilation modes, if used incorrectly.