bug#44849: [PATCH] Make XEmacs compat variable warning-level-aliases obsolete

bug#44849: [PATCH] Make XEmacs compat variable warning-level-aliases obsolete

[Stefan Kangas]

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

Severity: wishlist

The `warning-level-aliases' exists only to provide compatibility with
XEmacs warning levels.  I don't think this is very useful these days.

I propose to make it obsolete and warn when one of these levels are
used.  Please see the attached patch.

[-- Attachment #2: 0001-Make-XEmacs-compat-variable-warning-level-aliases-ob.patch --]
[-- Type: text/x-diff, Size: 2335 bytes --]

From 64973830bb453ed45c96cdce0c2a0fd6a584e787 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefan@marxist.se>
Date: Tue, 24 Nov 2020 18:54:57 +0100
Subject: [PATCH] Make XEmacs compat variable warning-level-aliases obsolete

* lisp/emacs-lisp/warnings.el (warning-level-aliases): Make obsolete.
(display-warning): Warn when using one of the warning levels defined
in above obsolete variable.
* lisp/url/url-proxy.el (url-find-proxy-for-url): Replace obsolete
warning type 'critical with :error.
---
 lisp/emacs-lisp/warnings.el | 7 +++++--
 lisp/url/url-proxy.el       | 2 +-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/lisp/emacs-lisp/warnings.el b/lisp/emacs-lisp/warnings.el
index f525ea433a..1c1eb14a24 100644
--- a/lisp/emacs-lisp/warnings.el
+++ b/lisp/emacs-lisp/warnings.el
@@ -67,6 +67,7 @@ warning-level-aliases
 Each element looks like (ALIAS . LEVEL) and defines ALIAS as
 equivalent to LEVEL.  LEVEL must be defined in `warning-levels';
 it may not itself be an alias.")
+(make-obsolete-variable 'warning-level-aliases 'warning-levels "28.1")
 \f
 (define-obsolete-variable-alias 'display-warning-minimum-level
   'warning-minimum-level "28.1")
@@ -256,8 +257,10 @@ display-warning
       (setq level :warning))
     (unless buffer-name
       (setq buffer-name "*Warnings*"))
-    (if (assq level warning-level-aliases)
-	(setq level (cdr (assq level warning-level-aliases))))
+    (with-suppressed-warnings ((obsolete warning-level-aliases))
+      (when-let ((new (cdr-safe (assq level warning-level-aliases))))
+        (warn "Warning level `%s' is obsolete; use `%s' instead" level new)
+        (setq level new)))
     (or (< (warning-numeric-level level)
 	   (warning-numeric-level warning-minimum-log-level))
 	(warning-suppress-p type warning-suppress-log-types)
diff --git a/lisp/url/url-proxy.el b/lisp/url/url-proxy.el
index 698a87098b..ad04a2d94a 100644
--- a/lisp/url/url-proxy.el
+++ b/lisp/url/url-proxy.el
@@ -59,7 +59,7 @@ url-find-proxy-for-url
      ((string-match "^socks +" proxy)
       (concat "socks://" (substring proxy (match-end 0))))
      (t
-      (display-warning 'url (format "Unknown proxy directive: %s" proxy) 'critical)
+      (display-warning 'url (format "Unknown proxy directive: %s" proxy) :error)
       nil))))
 
 (autoload 'url-http "url-http")
-- 
2.29.2

bug#44849: [PATCH] Make XEmacs compat variable warning-level-aliases obsolete

[Basil L. Contovounesios]

Stefan Kangas <stefan@marxist.se> writes:

> @@ -256,8 +257,10 @@ display-warning
>        (setq level :warning))
>      (unless buffer-name
>        (setq buffer-name "*Warnings*"))
> -    (if (assq level warning-level-aliases)
> -	(setq level (cdr (assq level warning-level-aliases))))
> +    (with-suppressed-warnings ((obsolete warning-level-aliases))
> +      (when-let ((new (cdr-safe (assq level warning-level-aliases))))

Why the cdr-safe?

Thanks,

-- 
Basil

bug#44849: [PATCH] Make XEmacs compat variable warning-level-aliases obsolete

[Stefan Kangas]

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> Stefan Kangas <stefan@marxist.se> writes:
>
>> @@ -256,8 +257,10 @@ display-warning
>>        (setq level :warning))
>>      (unless buffer-name
>>        (setq buffer-name "*Warnings*"))
>> -    (if (assq level warning-level-aliases)
>> -	(setq level (cdr (assq level warning-level-aliases))))
>> +    (with-suppressed-warnings ((obsolete warning-level-aliases))
>> +      (when-let ((new (cdr-safe (assq level warning-level-aliases))))
>
> Why the cdr-safe?

To avoid this:

(and (assq level warning-level-aliases)
     (cdr (assq level warning-level-aliases)))

bug#44849: [PATCH] Make XEmacs compat variable warning-level-aliases obsolete

[Basil L. Contovounesios]

Stefan Kangas <stefan@marxist.se> writes:

> "Basil L. Contovounesios" <contovob@tcd.ie> writes:
>
>> Why the cdr-safe?
>
> To avoid this:
>
> (and (assq level warning-level-aliases)
>      (cdr (assq level warning-level-aliases)))

But (cdr nil) is well-defined as being nil.

-- 
Basil

bug#44849: [PATCH] Make XEmacs compat variable warning-level-aliases obsolete

[Stefan Kangas]

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> Stefan Kangas <stefan@marxist.se> writes:
>
>> "Basil L. Contovounesios" <contovob@tcd.ie> writes:
>>
>>> Why the cdr-safe?
>>
>> To avoid this:
>>
>> (and (assq level warning-level-aliases)
>>      (cdr (assq level warning-level-aliases)))
>
> But (cdr nil) is well-defined as being nil.

Oops.  Indeed.

bug#44849: [PATCH] Make XEmacs compat variable warning-level-aliases obsolete

[Lars Ingebrigtsen]

Stefan Kangas <stefan@marxist.se> writes:

> The `warning-level-aliases' exists only to provide compatibility with
> XEmacs warning levels.  I don't think this is very useful these days.
>
> I propose to make it obsolete and warn when one of these levels are
> used.  Please see the attached patch.

I'm slightly worried that these aliases are used out there in
out-of-tree packages.  As a test, I grepped the GNU ELPA tree, but this
was the only place where the aliases are used:

(defun muse-display-warning (message)
  "Display the given MESSAGE as a warning."
  (if (fboundp 'display-warning)
      (display-warning 'muse message
                       (if (featurep 'xemacs)
                           'warning
                         :warning))

And that's behind a featurep check, so it's fine, too.

So I think the proposed change is probably fine.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#44849: [PATCH] Make XEmacs compat variable warning-level-aliases obsolete

[Stefan Kangas]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Stefan Kangas <stefan@marxist.se> writes:
>
>> The `warning-level-aliases' exists only to provide compatibility with
>> XEmacs warning levels.  I don't think this is very useful these days.
>>
>> I propose to make it obsolete and warn when one of these levels are
>> used.  Please see the attached patch.
>
> I'm slightly worried that these aliases are used out there in
> out-of-tree packages.  As a test, I grepped the GNU ELPA tree, but this
> was the only place where the aliases are used:
>
> (defun muse-display-warning (message)
>   "Display the given MESSAGE as a warning."
>   (if (fboundp 'display-warning)
>       (display-warning 'muse message
>                        (if (featurep 'xemacs)
>                            'warning
>                          :warning))
>
> And that's behind a featurep check, so it's fine, too.
>
> So I think the proposed change is probably fine.

Thanks.  I agree, and the way it is written it will simply show a
warning in case such a usage is not behind a featurep check.

There have been no further comments within 2 weeks, so I intend to push
the change in the next couple of days unless there are any objections.

bug#44849: [PATCH] Make XEmacs compat variable warning-level-aliases obsolete

[Stefan Kangas]

tags 44849 fixed
close 44849 28.1
thanks

Stefan Kangas <stefan@marxist.se> writes:

> There have been no further comments within 2 weeks, so I intend to push
> the change in the next couple of days unless there are any objections.

No further comments within a couple of days; pushed to master as commit
4c41a8acc0.