bug#43300: [PATCH] Make M-x show new commands for obsolete aliases

bug#43300: [PATCH] Make M-x show new commands for obsolete aliases

[Stefan Kangas]

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

Severity: wishlist

Please find attached two patches that tweaks how M-x
(execute-extended-command) works:

1. Show obsolete commands, and give their new name as an annotation.

M-x recently got the capability to show the keybindings for commands in
an annotation (in parenthesis after the command name).  This patch makes
it show new names for obsolete aliases in the same way, instead of just
refusing to show them.  This should help users ease into the new name
less abruptly and disruptively, instead of having it just disappearing
from M-x and be nowhere to be found.

As an added bonus, we could more confidently mark an alias such as
`display-time-world' obsolete (without worrying that it will just be
gone in the next release).

(Yes, if you type out the full name, you can still call it, but chances
are that many users are very reliant on tab completion and will assume
that it's just gone if it doesn't show up.)

2. Show the function that aliases point to in the same way.

[-- Attachment #2: 0001-Make-M-x-show-obsolete-commands.patch --]
[-- Type: text/x-diff, Size: 2124 bytes --]

From 870e392fc31da0f1c8a84c3196b16441908a3455 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefan@marxist.se>
Date: Thu, 10 Sep 2020 01:32:24 +0200
Subject: [PATCH 1/2] Make M-x show obsolete commands

* lisp/simple.el (read-extended-command): Don't hide obsolete
commands.
(read-extended-command--annotation): Show an annotation for obsolete
commands that says what their new name is.
---
 lisp/simple.el | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/lisp/simple.el b/lisp/simple.el
index 3b2b5c92e9..37e9a95bad 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -1881,22 +1881,17 @@ read-extended-command
 	   '(metadata
 	     (annotation-function . read-extended-command--annotation)
 	     (category . command))
-         (let ((pred
-                (if (memq action '(nil t))
-                    ;; Exclude obsolete commands from completions.
-                    (lambda (sym)
-                      (and (funcall pred sym)
-                           (or (equal string (symbol-name sym))
-                               (not (get sym 'byte-obsolete-info)))))
-                  pred)))
-           (complete-with-action action obarray string pred))))
+         (complete-with-action action obarray string pred)))
      #'commandp t nil 'extended-command-history)))
 
 (defun read-extended-command--annotation (command-name)
-  (let* ((function (and (stringp command-name) (intern-soft command-name)))
-         (binding (where-is-internal function overriding-local-map t)))
-    (when (and binding (not (stringp binding)))
-      (format " (%s)" (key-description binding)))))
+  (let* ((fun (and (stringp command-name) (intern-soft command-name)))
+         (binding (where-is-internal fun overriding-local-map t))
+         (obsolete (get fun 'byte-obsolete-info)))
+    (cond (obsolete
+           (format " (%s)" (car obsolete)))
+          ((and binding (not (stringp binding)))
+           (format " (%s)" (key-description binding))))))
 
 (defcustom suggest-key-bindings t
   "Non-nil means show the equivalent key-binding when M-x command has one.
-- 
2.28.0


[-- Attachment #3: 0002-Make-M-x-show-what-aliases-point-to.patch --]
[-- Type: text/x-diff, Size: 1180 bytes --]

From c6a60a657f52712d40a80b115484904dc88e4629 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefan@marxist.se>
Date: Thu, 10 Sep 2020 01:42:53 +0200
Subject: [PATCH 2/2] Make M-x show what aliases point to

* lisp/simple.el (read-extended-command--annotation): Show an
annotation for aliases saying what it points to.
---
 lisp/simple.el | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lisp/simple.el b/lisp/simple.el
index 37e9a95bad..73dd938e63 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -1887,8 +1887,11 @@ read-extended-command
 (defun read-extended-command--annotation (command-name)
   (let* ((fun (and (stringp command-name) (intern-soft command-name)))
          (binding (where-is-internal fun overriding-local-map t))
-         (obsolete (get fun 'byte-obsolete-info)))
-    (cond (obsolete
+         (obsolete (get fun 'byte-obsolete-info))
+         (alias (symbol-function fun)))
+    (cond ((symbolp alias)
+           (format " (%s)" alias))
+          (obsolete
            (format " (%s)" (car obsolete)))
           ((and binding (not (stringp binding)))
            (format " (%s)" (key-description binding))))))
-- 
2.28.0

bug#43300: [PATCH] Make M-x show new commands for obsolete aliases

[Lars Ingebrigtsen]

Stefan Kangas <stefan@marxist.se> writes:

> As an added bonus, we could more confidently mark an alias such as
> `display-time-world' obsolete (without worrying that it will just be
> gone in the next release).

Sounds like a good idea.

> 2. Show the function that aliases point to in the same way.

Makes sense.

> * lisp/simple.el (read-extended-command): Don't hide obsolete
> commands.
> (read-extended-command--annotation): Show an annotation for obsolete
> commands that says what their new name is.

I haven't tried the patch, but it looks good to me.

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

bug#43300: [PATCH] Make M-x show new commands for obsolete aliases

[Stefan Kangas]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> I haven't tried the patch, but it looks good to me.

Thanks.  Pushed to master as commit 1b0a922a19 and 06d86b954d.

bug#43300: [PATCH] Make M-x show new commands for obsolete aliases

[Juri Linkov]

unarchive 43300
quit

> Please find attached two patches that tweaks how M-x
> (execute-extended-command) works:
>
> 1. Show obsolete commands, and give their new name as an annotation.

I noticed that some commands have " (nil)" appended as annotations
in M-x completions, e.g. 'M-x browse- TAB'.  Looks like " (nil)"
is returned in read-extended-command--annotation:

          (obsolete
           (format " (%s)" (car obsolete)))

bug#43300: [PATCH] Make M-x show new commands for obsolete aliases

[Basil L. Contovounesios]

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

Juri Linkov <juri@linkov.net> writes:

>> Please find attached two patches that tweaks how M-x
>> (execute-extended-command) works:
>>
>> 1. Show obsolete commands, and give their new name as an annotation.
>
> I noticed that some commands have " (nil)" appended as annotations
> in M-x completions, e.g. 'M-x browse- TAB'.  Looks like " (nil)"
> is returned in read-extended-command--annotation:
>
>           (obsolete
>            (format " (%s)" (car obsolete)))

Is the following sufficient?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: m-x.diff --]
[-- Type: text/x-diff, Size: 879 bytes --]

diff --git a/lisp/simple.el b/lisp/simple.el
index bb28145502..2acceef6a1 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -1964,13 +1964,14 @@ read-extended-command
      #'commandp t nil 'extended-command-history)))
 
 (defun read-extended-command--annotation (command-name)
-  (let* ((fun (and (stringp command-name) (intern-soft command-name)))
+  "Return annotation for COMMAND-NAME in M-x completion."
+  (let* ((fun (intern-soft command-name))
          (binding (where-is-internal fun overriding-local-map t))
          (obsolete (get fun 'byte-obsolete-info))
          (alias (symbol-function fun)))
     (cond ((symbolp alias)
            (format " (%s)" alias))
-          (obsolete
+          ((car obsolete)
            (format " (%s)" (car obsolete)))
           ((and binding (not (stringp binding)))
            (format " (%s)" (key-description binding))))))

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


Thanks,

-- 
Basil

bug#43300: [PATCH] Make M-x show new commands for obsolete aliases

[Stefan Kangas]

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

> Juri Linkov <juri@linkov.net> writes:
>
>>> Please find attached two patches that tweaks how M-x
>>> (execute-extended-command) works:
>>>
>>> 1. Show obsolete commands, and give their new name as an annotation.
>>
>> I noticed that some commands have " (nil)" appended as annotations
>> in M-x completions, e.g. 'M-x browse- TAB'.  Looks like " (nil)"
>> is returned in read-extended-command--annotation:
>>
>>           (obsolete
>>            (format " (%s)" (car obsolete)))

Thanks for reporting this.

> Is the following sufficient?

The old behavior was to never show obsolete commands in completion.  You
could therefore only run one if you remembered and manually typed in the
old name.  If you made a command obsolete, it would therefore just
appear to disappear from one release to the next.

The new behavior was intended to be less abrupt for cases where the old
command is anyways just an alias for a new command.  In this way, we can
more gently nudge users to use the new command instead.

I therefore think we should revert back to the old behavior for commands
that are obsolete without an alternative, that is we should not add such
commands to the list of completion candidates.

bug#43300: [PATCH] Make M-x show new commands for obsolete aliases

[Juri Linkov]

>> Is the following sufficient?
>
> The old behavior was to never show obsolete commands in completion.  You
> could therefore only run one if you remembered and manually typed in the
> old name.  If you made a command obsolete, it would therefore just
> appear to disappear from one release to the next.
>
> The new behavior was intended to be less abrupt for cases where the old
> command is anyways just an alias for a new command.  In this way, we can
> more gently nudge users to use the new command instead.
>
> I therefore think we should revert back to the old behavior for commands
> that are obsolete without an alternative, that is we should not add such
> commands to the list of completion candidates.

Or maybe still some indication is needed to be displayed, even if there is
no alias for a new command.  Maybe display just " (obsolete)"?

bug#43300: [PATCH] Make M-x show new commands for obsolete aliases

[Drew Adams]

> > I therefore think we should revert back to the old behavior for
> > commands that are obsolete without an alternative, that is we
> > should not add such commands to the list of completion candidates.
> 
> Or maybe still some indication is needed to be displayed, even if there
> is no alias for a new command.  Maybe display just " (obsolete)"?

Why?  That can give the false impression that the
command doesn't _work_.

(And if for some reason the command really doesn't
work then it's not only obsolete.  And a user will
soon enough understand that it doesn't work.)

bug#43300: [PATCH] Make M-x show new commands for obsolete aliases

[Stefan Kangas]

Juri Linkov <juri@linkov.net> writes:

>>> Is the following sufficient?
>>
>> The old behavior was to never show obsolete commands in completion.  You
>> could therefore only run one if you remembered and manually typed in the
>> old name.  If you made a command obsolete, it would therefore just
>> appear to disappear from one release to the next.
>>
>> The new behavior was intended to be less abrupt for cases where the old
>> command is anyways just an alias for a new command.  In this way, we can
>> more gently nudge users to use the new command instead.
>>
>> I therefore think we should revert back to the old behavior for commands
>> that are obsolete without an alternative, that is we should not add such
>> commands to the list of completion candidates.
>
> Or maybe still some indication is needed to be displayed, even if there is
> no alias for a new command.  Maybe display just " (obsolete)"?

It's an alternative, yes.  It has the drawback that now things in
parenthesis can mean not only things you can run (keybindings and
commands) but also just general information.

FWIW, I tend to lean towards maintaining the old behavior here.
I also note that Stefan Monnier didn't particularly like the new
behavior, as it doesn't discourage users enough from not using the
commands.  So maybe we could meet him half-way and maintain the old
behavior, even if only for some cases.
See: https://lists.gnu.org/r/emacs-devel/2020-09/msg01102.html

bug#43300: [PATCH] Make M-x show new commands for obsolete aliases

[Stefan Kangas]

unarchive 43300
reopen 43300
thanks

(This bug is about showing obsolete aliases for commands in the list of
completion candidates.)

Stefan Kangas <stefan@marxist.se> writes:

> FWIW, I tend to lean towards maintaining the old behavior here.
> I also note that Stefan Monnier didn't particularly like the new
> behavior, as it doesn't discourage users enough from not using the
> commands.  So maybe we could meet him half-way and maintain the old
> behavior, even if only for some cases.
> See: https://lists.gnu.org/r/emacs-devel/2020-09/msg01102.html

I dropped the ball here a little bit, but I note that Stefan Monnier did
not like the new behavior, and in hindsight (and with more experience
using this feature) I think he is correct.

I also note that the new behavior fails in cases like:

    query-replace-regexp-eval (use the `\,' feature of `query-replace-regexp'
    for interactive calls, and `search-forward-regexp'/`replace-match'
    for Lisp calls.)

IOW, even though I was the one who suggested and implemented this
feature, I now think that it was a mistake.  I would therefore suggest
reverting commit 06d86b954d2 to go back to the previous, Emacs 27
behavior.

Any objections?

bug#43300: [PATCH] Make M-x show new commands for obsolete aliases

[Lars Ingebrigtsen]

Stefan Kangas <stefan@marxist.se> writes:

> I dropped the ball here a little bit, but I note that Stefan Monnier did
> not like the new behavior, and in hindsight (and with more experience
> using this feature) I think he is correct.

I think the original idea here was good -- make command deprecation less
abrupt, and teach users about the new aliases.  

> I also note that the new behavior fails in cases like:
>
>     query-replace-regexp-eval (use the `\,' feature of `query-replace-regexp'
>     for interactive calls, and `search-forward-regexp'/`replace-match'
>     for Lisp calls.)
>
> IOW, even though I was the one who suggested and implemented this
> feature, I now think that it was a mistake.  I would therefore suggest
> reverting commit 06d86b954d2 to go back to the previous, Emacs 27
> behavior.

Couldn't those cases be fixed?

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

bug#43300: [PATCH] Make M-x show new commands for obsolete aliases

[Stefan Kangas]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Stefan Kangas <stefan@marxist.se> writes:
>
>> I dropped the ball here a little bit, but I note that Stefan Monnier did
>> not like the new behavior, and in hindsight (and with more experience
>> using this feature) I think he is correct.
>
> I think the original idea here was good -- make command deprecation less
> abrupt, and teach users about the new aliases.

I find myself going back and forth: with the new filtering, there is
suddenly a hope that `M-x' can produce a clean list of useful commands.

The feature discussed here makes the list less clean, for reasons that
are only temporarily useful.  This is compounded by the fact that we
maintain backwards compatibility aliases for such a long time.  But of
course there are also benefits to this more gentle obsoletion, as you
say.

Here's an idea:

How about showing only obsolete aliases that are new in this major
version?  That could give us almost all the benefits without any of the
drawbacks.

> Couldn't those cases be fixed?

Yes, it should be trivial.

bug#43300: [PATCH] Make M-x show new commands for obsolete aliases

[Lars Ingebrigtsen]

Stefan Kangas <stefan@marxist.se> writes:

> I find myself going back and forth: with the new filtering, there is
> suddenly a hope that `M-x' can produce a clean list of useful commands.

Yeah, that's true -- `M-x gnus-summary-toggle-truncation' is pretty
annoying to have appear here (it's an obsolete alias)...  On the other
hand, it could be tagged with a mode (somehow) and that would also make
it go away (outside of Gnus summary buffers).

> The feature discussed here makes the list less clean, for reasons that
> are only temporarily useful.  This is compounded by the fact that we
> maintain backwards compatibility aliases for such a long time.  But of
> course there are also benefits to this more gentle obsoletion, as you
> say.
>
> Here's an idea:
>
> How about showing only obsolete aliases that are new in this major
> version?  That could give us almost all the benefits without any of the
> drawbacks.

Hm!  That's a pretty attractive option.  So commands deprecate faster
than functions...  makes sense to me.

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

bug#43300: [PATCH] Make M-x show new commands for obsolete aliases

[Stefan Kangas]

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

>> How about showing only obsolete aliases that are new in this major
>> version?  That could give us almost all the benefits without any of the
>> drawbacks.
>
> Hm!  That's a pretty attractive option.  So commands deprecate faster
> than functions...  makes sense to me.

Here's a patch that does that.

[-- Attachment #2: 0001-Don-t-consider-obsolete-commands-for-obsoletion-in-s.patch --]
[-- Type: text/x-diff, Size: 2786 bytes --]

From 10715922a84d067b838fef82bd8b3ed8782d65ff Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefan@marxist.se>
Date: Wed, 17 Feb 2021 10:45:00 +0100
Subject: [PATCH] Don't consider obsolete commands for obsoletion in some cases

* lisp/simple.el (read-extended-command): Exclude obsolete commands
that are either lacking a 'current-name' or were obsoleted in a
previous major version.  (Bug#43300)
---
 etc/NEWS       |  5 +++--
 lisp/simple.el | 19 ++++++++++++++++++-
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 943ad6ac59..7b257b9de0 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -323,9 +323,10 @@ setting the variable 'auto-save-visited-mode' buffer-locally to nil.
 description of the properties.  Likewise 'button-describe' does the
 same for a button.
 
-** Obsolete commands are no longer hidden from command completion.
+** Obsolete aliases are no longer hidden from command completion.
 Completion of command names now considers obsolete aliases as
-candidates.  Invoking a command via an obsolete alias now mentions the
+candidates, if they were marked obsolete in the current major version
+of Emacs.  Invoking a command via an obsolete alias now mentions the
 obsolescence fact and shows the new name of the command.
 
 +++
diff --git a/lisp/simple.el b/lisp/simple.el
index 215f4399f4..08155c36db 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -1967,7 +1967,24 @@ read-extended-command
 	     '(metadata
 	       (affixation-function . read-extended-command--affixation)
 	       (category . command))
-           (complete-with-action action obarray string pred)))
+           (let ((pred
+                  (if (memq action '(nil t))
+                      ;; Exclude from completions obsolete commands
+                      ;; lacking a `current-name', or where `when' is
+                      ;; not the current major version.
+                      (lambda (sym)
+                        (let ((obsolete (get sym 'byte-obsolete-info)))
+                          (and (funcall pred sym)
+                               (or (equal string (symbol-name sym))
+                                   (not obsolete)
+                                   (and
+                                    ;; has a current-name
+                                    (functionp (cadr obsolete))
+                                    ;; when >= emacs-major-version
+                                    (>= (car (version-to-list (caddr obsolete)))
+                                        emacs-major-version))))))
+                    pred)))
+             (complete-with-action action obarray string pred))))
        (lambda (sym)
          (and (commandp sym)
               (funcall read-extended-command-predicate sym buffer)))
-- 
2.30.0

bug#43300: [PATCH] Make M-x show new commands for obsolete aliases

[Lars Ingebrigtsen]

Stefan Kangas <stefan@marxist.se> writes:

> Here's a patch that does that.

[...]

> * lisp/simple.el (read-extended-command): Exclude obsolete commands
> that are either lacking a 'current-name' or were obsoleted in a
> previous major version.  (Bug#43300)

Looks good to me.

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

bug#43300: [PATCH] Make M-x show new commands for obsolete aliases

[Basil L. Contovounesios]

Stefan Kangas <stefan@marxist.se> writes:

> +                      ;; Exclude from completions obsolete commands
> +                      ;; lacking a `current-name', or where `when' is
> +                      ;; not the current major version.
> +                      (lambda (sym)
> +                        (let ((obsolete (get sym 'byte-obsolete-info)))
> +                          (and (funcall pred sym)
> +                               (or (equal string (symbol-name sym))

AKA string=.

Thanks,

-- 
Basil

bug#43300: [PATCH] Make M-x show new commands for obsolete aliases

[Lars Ingebrigtsen]

Stefan Kangas <stefan@marxist.se> writes:

>> Hm!  That's a pretty attractive option.  So commands deprecate faster
>> than functions...  makes sense to me.
>
> Here's a patch that does that.

I've now applied it to Emacs 28 (after some testing and a small fix).
Seems to work quite logically to me.

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