Re: master d08c947: Make compilation-mode regexp matching case-sensitive (bug#40119)

Re: master d08c947: Make compilation-mode regexp matching case-sensitive (bug#40119)

[Stefan Monnier]

> +(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")

This makes no sense as a user-config.

The regular expressions are written either with or without case-folding
in mind, so changing it globally is a recipe for problems.

If we want to allow case-insensitive regexp matching, then we should
extend the syntax of `compilation-error-regexp-alist` so that a given
entry there can request it to be used with case-folding.


        Stefan


PS: The same applies to the "zero/one-based column numbers" and
    the "char-colums vs visual-columns vs byte-columns", admittedly.

Re: master d08c947: Make compilation-mode regexp matching case-sensitive (bug#40119)

[Mattias Engdegård]

25 mars 2020 kl. 21.54 skrev Stefan Monnier <monnier@iro.umontreal.ca>:

> The regular expressions are written either with or without case-folding
> in mind, so changing it globally is a recipe for problems.

Agreed, but I doubt that anyone will ever set the option. It was added just in case someone has a tool emitting messages that happens to match one of the built-in rules if case-folding is used, or if a user has added a sloppily written regexp. In either case, adding a properly written regexp is of course the recommended action.

> If we want to allow case-insensitive regexp matching, then we should
> extend the syntax of `compilation-error-regexp-alist` so that a given
> entry there can request it to be used with case-folding.

It would be more flexible, but I'd rather not add the complexity without evidence of a need for it.

> PS: The same applies to the "zero/one-based column numbers" and
>    the "char-colums vs visual-columns vs byte-columns", admittedly.

Yes, those settings would make more sense if they were rule-specific. Still, actual examples of tools needing them would be useful.

Re: master d08c947: Make compilation-mode regexp matching case-sensitive (bug#40119)

[Stefan Monnier]

>> The regular expressions are written either with or without case-folding
>> in mind, so changing it globally is a recipe for problems.
> Agreed, but I doubt that anyone will ever set the option.

Then let's make it into a simple `defvar` rather than a defcustom and
make the docstring clarify that it's only useful for temporary
backward compatibility.

>> PS: The same applies to the "zero/one-based column numbers" and
>>    the "char-colums vs visual-columns vs byte-columns", admittedly.
>
> Yes, those settings would make more sense if they were rule-specific.
> Still, actual examples of tools needing them would be useful.

In sml-mode.el (GNU ELPA) I set `compilation-error-screen-columns`
buffer-locally but it really should be rule-local.  In tuareg-mode (not
in GNU ELPA, sadly), I set both `compilation-first-column` and
`compilation-error-screen-columns` buffer-locally for the same reason.


        Stefan

Re: master d08c947: Make compilation-mode regexp matching case-sensitive (bug#40119)

[Mattias Engdegård]

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

26 mars 2020 kl. 04.42 skrev Stefan Monnier <monnier@iro.umontreal.ca>:

> Then let's make it into a simple `defvar` rather than a defcustom and
> make the docstring clarify that it's only useful for temporary
> backward compatibility.

Right; patch attached.

> In sml-mode.el (GNU ELPA) I set `compilation-error-screen-columns`
> buffer-locally but it really should be rule-local.  In tuareg-mode (not
> in GNU ELPA, sadly), I set both `compilation-first-column` and
> `compilation-error-screen-columns` buffer-locally for the same reason.

And here I've been staring at OCaml type errors for years and never noticed...

* Gcc and Clang also count characters rather than columns. (Bytes, in fact. Perhaps we need a setting for that as well?)

* Instead of making compilation-error-regexp-alist entries even more complex and overloaded, perhaps we should use plists? For example, each entry could be

 (REGEXP :file FILE :line LINE ...)

Adding :first-column and :screen-columns would then be more a more natural way of doing it.

* There could be multiple tools with different column-number semantics sharing a single pattern.

* Rule-specific column-number semantics can be done today by specifying a function as COLUMN parameter, so perhaps no extension is necessary.


[-- Attachment #2: 0001-Turn-compilation-error-case-fold-search-into-a-defva.patch --]
[-- Type: application/octet-stream, Size: 2305 bytes --]

From fb842363736881ad045764751d86aae6d1ee4abe Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Thu, 26 Mar 2020 10:58:30 +0100
Subject: [PATCH] Turn compilation-error-case-fold-search into a defvar

See discussion at
https://lists.gnu.org/archive/html/emacs-devel/2020-03/msg00653.html

* lisp/progmodes/compile.el (compilation-error-case-fold-search):
Turn into a defvar.
* etc/NEWS: Update.
---
 etc/NEWS                  |  6 ++++--
 lisp/progmodes/compile.el | 10 ++++++----
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 910d9fa2d2..6569158f70 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -199,8 +199,10 @@ Defaults to Libravatar, with Unicornify and Gravatar as options.
 ** 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.
+The variable 'compilation-error-case-fold-search' can be set for
+case-insensitive matching of messages when the old behaviour is
+required, but the recommended solution is to use a correctly matching
+regexp instead.
 
 \f
 * New Modes and Packages in Emacs 28.1
diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
index f4532b7edb..e5878b28f9 100644
--- a/lisp/progmodes/compile.el
+++ b/lisp/progmodes/compile.el
@@ -646,13 +646,15 @@ compilation-error-regexp-alist
   :link `(file-link :tag "example file"
 		    ,(expand-file-name "compilation.txt" data-directory)))
 
-(defcustom compilation-error-case-fold-search nil
+(defvar 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")
+If nil, matching is case-sensitive.
+
+This variable should only be set for backward compatibility as a temporary
+measure.  The proper solution is to use a regexp that matches the
+messages without case-folding.")
 
 ;;;###autoload(put 'compilation-directory 'safe-local-variable 'stringp)
 (defvar compilation-directory nil
-- 
2.21.1 (Apple Git-122.3)


[-- Attachment #3: Type: text/plain, Size: 2 bytes --]