Re: emacs-30 7c50e4cc8cc 2/2: Improve 'compilation-transform-file-match-alist' documentation

Re: emacs-30 7c50e4cc8cc 2/2: Improve 'compilation-transform-file-match-alist' documentation

[Robert Pluim]

>>>>> On Wed,  4 Dec 2024 09:20:55 -0500 (EST), Robert Pluim <rpluim@gmail.com> said:

    Robert>    :type '(repeat (list regexp (choice (const :tag "No replacement" nil)
    Robert>                                        string)))
    Robert>    :version "27.1")

I almost did the following as well on emacs-30, on the grounds that it
doesnʼt actually affect the real type of the user option, only the
display, but since I wasnʼt sure that was OK for the release branch I
thought Iʼd ask here.

diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
index ee7fc6aaecb..6a640a3c056 100644
--- a/lisp/progmodes/compile.el
+++ b/lisp/progmodes/compile.el
@@ -77,8 +77,9 @@ compilation-transform-file-match-alist
 Similarly, to remove a prefix \"bar/\", use:
 
     (\"\\\\=`bar/\" \"\")"
-  :type '(repeat (list regexp (choice (const :tag "No replacement" nil)
-                                      string)))
+  :type '(repeat (list (regexp :tag "Match")
+                       (radio (const :tag "Not an error" nil)
+                              (string :tag "Replace with"))))
   :version "27.1")
 
 (defvar compilation-filter-hook nil

Robert
-- 

Re: emacs-30 7c50e4cc8cc 2/2: Improve 'compilation-transform-file-match-alist' documentation

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Date: Wed, 04 Dec 2024 15:25:08 +0100
> 
> >>>>> On Wed,  4 Dec 2024 09:20:55 -0500 (EST), Robert Pluim <rpluim@gmail.com> said:
> 
>     Robert>    :type '(repeat (list regexp (choice (const :tag "No replacement" nil)
>     Robert>                                        string)))
>     Robert>    :version "27.1")
> 
> I almost did the following as well on emacs-30, on the grounds that it
> doesnʼt actually affect the real type of the user option, only the
> display, but since I wasnʼt sure that was OK for the release branch I
> thought Iʼd ask here.
> 
> diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
> index ee7fc6aaecb..6a640a3c056 100644
> --- a/lisp/progmodes/compile.el
> +++ b/lisp/progmodes/compile.el
> @@ -77,8 +77,9 @@ compilation-transform-file-match-alist
>  Similarly, to remove a prefix \"bar/\", use:
>  
>      (\"\\\\=`bar/\" \"\")"
> -  :type '(repeat (list regexp (choice (const :tag "No replacement" nil)
> -                                      string)))
> +  :type '(repeat (list (regexp :tag "Match")
> +                       (radio (const :tag "Not an error" nil)
> +                              (string :tag "Replace with"))))
>    :version "27.1")

I don't think I'd mind to install on emacs-30, but if we are doing
this, why not do a better job?  The tags above are not quite clear, to
say the least.  "Match"? "Replace with"? does that really explain
itself?

Thanks.

Re: emacs-30 7c50e4cc8cc 2/2: Improve 'compilation-transform-file-match-alist' documentation

[Robert Pluim]

>>>>> On Wed, 04 Dec 2024 16:56:04 +0200, Eli Zaretskii <eliz@gnu.org> said:

    Eli> I don't think I'd mind to install on emacs-30, but if we are doing
    Eli> this, why not do a better job?  The tags above are not quite clear, to
    Eli> say the least.  "Match"? "Replace with"? does that really explain
    Eli> itself?

The docstring is supposed to explain it :-) How about:

diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
index ee7fc6aaecb..99385fcca07 100644
--- a/lisp/progmodes/compile.el
+++ b/lisp/progmodes/compile.el
@@ -77,8 +77,9 @@ compilation-transform-file-match-alist
 Similarly, to remove a prefix \"bar/\", use:
 
     (\"\\\\=`bar/\" \"\")"
-  :type '(repeat (list regexp (choice (const :tag "No replacement" nil)
-                                      string)))
+  :type '(repeat (list (regexp :tag "Match filename(s)")
+                       (radio (const :tag "Not an error" nil)
+                              (string :tag "Replace match with"))))
   :version "27.1")
 
 (defvar compilation-filter-hook nil

Robert
-- 

Re: emacs-30 7c50e4cc8cc 2/2: Improve 'compilation-transform-file-match-alist' documentation

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Wed, 04 Dec 2024 16:08:23 +0100
> 
> >>>>> On Wed, 04 Dec 2024 16:56:04 +0200, Eli Zaretskii <eliz@gnu.org> said:
> 
>     Eli> I don't think I'd mind to install on emacs-30, but if we are doing
>     Eli> this, why not do a better job?  The tags above are not quite clear, to
>     Eli> say the least.  "Match"? "Replace with"? does that really explain
>     Eli> itself?
> 
> The docstring is supposed to explain it :-) How about:
> 
> diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
> index ee7fc6aaecb..99385fcca07 100644
> --- a/lisp/progmodes/compile.el
> +++ b/lisp/progmodes/compile.el
> @@ -77,8 +77,9 @@ compilation-transform-file-match-alist
>  Similarly, to remove a prefix \"bar/\", use:
>  
>      (\"\\\\=`bar/\" \"\")"
> -  :type '(repeat (list regexp (choice (const :tag "No replacement" nil)
> -                                      string)))
> +  :type '(repeat (list (regexp :tag "Match filename(s)")
> +                       (radio (const :tag "Not an error" nil)
> +                              (string :tag "Replace match with"))))
>    :version "27.1")

I think "Filename that matches", "Do not consider as error", and
"Replace matched filename with", or something to that effect, will be
better.

Re: emacs-30 7c50e4cc8cc 2/2: Improve 'compilation-transform-file-match-alist' documentation

[Robert Pluim]

>>>>> On Wed, 04 Dec 2024 17:37:46 +0200, Eli Zaretskii <eliz@gnu.org> said:

    Eli> I think "Filename that matches", "Do not consider as error", and
    Eli> "Replace matched filename with", or something to that effect, will be
    Eli> better.

Done.

Robert
-- 

Re: emacs-30 7c50e4cc8cc 2/2: Improve 'compilation-transform-file-match-alist' documentation

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Wed, 04 Dec 2024 17:02:33 +0100
> 
> >>>>> On Wed, 04 Dec 2024 17:37:46 +0200, Eli Zaretskii <eliz@gnu.org> said:
> 
>     Eli> I think "Filename that matches", "Do not consider as error", and
>     Eli> "Replace matched filename with", or something to that effect, will be
>     Eli> better.
> 
> Done.

Thanks.