bug#74615: 30.0.92; js-find-symbol automatically activates ido-mode

bug#74615: 30.0.92; js-find-symbol automatically activates ido-mode

[Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

js-find-symbol/js--read-symbol reads a symbol via ido-completing-read
and automatically activates ido-mode. For users of other completion
systems (default completion, Icomplete/Fido, Vertico, Helm, ...) this is
not desired. Icomplete/Fido/Vertico/etc. replace Ido and provide similar
behavior for general completing-read.

Patch attached.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-progmodes-js-Use-completing-read-instead-of-ido-comp.patch --]
[-- Type: text/x-diff, Size: 1271 bytes --]

From 3b4ccb677a16fc4934d62fed7e1f01b52399e633 Mon Sep 17 00:00:00 2001
From: Daniel Mendler <mail@daniel-mendler.de>
Date: Wed, 1 Mar 2023 19:27:38 +0100
Subject: [PATCH] progmodes/js: Use completing-read instead of
 ido-completing-read

---
 lisp/progmodes/js.el | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index f74b8ab1c46..29566d38109 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -58,11 +58,8 @@
 
 (eval-when-compile
   (require 'cl-lib)
-  (require 'ido)
   (require 'rx))
 
-(defvar ido-cur-list)
-(declare-function ido-mode "ido" (&optional arg))
 (declare-function treesit-parser-create "treesit.c")
 (declare-function treesit-induce-sparse-tree "treesit.c")
 (declare-function treesit-search-subtree "treesit.c")
@@ -3288,11 +3285,7 @@ js--read-symbol
 initial input INITIAL-INPUT.  Return a cons of (SYMBOL-NAME
 . LOCATION), where SYMBOL-NAME is a string and LOCATION is a
 marker."
-  (unless ido-mode
-    (ido-mode 1)
-    (ido-mode -1))
-
-  (let ((choice (ido-completing-read
+  (let ((choice (completing-read
                  prompt
                  (cl-loop for key being the hash-keys of symbols-table
                           collect key)
-- 
2.45.2

bug#74615: 30.0.92; js-find-symbol automatically activates ido-mode

[Eli Zaretskii]

> Cc: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Sat, 30 Nov 2024 07:37:27 +0100
> From:  Daniel Mendler via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> js-find-symbol/js--read-symbol reads a symbol via ido-completing-read
> and automatically activates ido-mode. For users of other completion
> systems (default completion, Icomplete/Fido, Vertico, Helm, ...) this is
> not desired. Icomplete/Fido/Vertico/etc. replace Ido and provide similar
> behavior for general completing-read.

This code is there since the day js.el was added, so I don't think
removing the activation of ido-mode is something we can do, because
users might expect that by now.  Maybe we could make that optional or
something.

I've added Daniel Colascione to the discussion, in case he has an
opinion or comments about this.

bug#74615: 30.0.92; js-find-symbol automatically activates ido-mode

[Daniel Colascione]

On November 30, 2024 3:02:55 AM EST, Eli Zaretskii <eliz@gnu.org> wrote:
>> Cc: Stefan Monnier <monnier@iro.umontreal.ca>
>> Date: Sat, 30 Nov 2024 07:37:27 +0100
>> From:  Daniel Mendler via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>> 
>> js-find-symbol/js--read-symbol reads a symbol via ido-completing-read
>> and automatically activates ido-mode. For users of other completion
>> systems (default completion, Icomplete/Fido, Vertico, Helm, ...) this is
>> not desired. Icomplete/Fido/Vertico/etc. replace Ido and provide similar
>> behavior for general completing-read.
>
>This code is there since the day js.el was added, so I don't think
>removing the activation of ido-mode is something we can do, because
>users might expect that by now.  Maybe we could make that optional or
>something.
>
>I've added Daniel Colascione to the discussion, in case he has an
>opinion or comments about this.

I think we can safely regard the hard coding of ido here as a bug and switch it to regular conpleting-read without breaking user workflows. Not every behavior users expect is one we should keep. I'm supportive of UX changes in the interest of making the system more regular and orthogonal.

bug#74615: 30.0.92; js-find-symbol automatically activates ido-mode

[Dmitry Gutov]

On 30/11/2024 10:02, Eli Zaretskii wrote:
>> js-find-symbol/js--read-symbol reads a symbol via ido-completing-read
>> and automatically activates ido-mode. For users of other completion
>> systems (default completion, Icomplete/Fido, Vertico, Helm, ...) this is
>> not desired. Icomplete/Fido/Vertico/etc. replace Ido and provide similar
>> behavior for general completing-read.
> This code is there since the day js.el was added, so I don't think
> removing the activation of ido-mode is something we can do

FWIW we've removed this command from the default js-mode bindings last 
year (9cbf0bb3ee88) and haven't heard any complaints, so it might not 
have too many users these days. Cleaning it up sounds reasonable.

bug#74615: 30.0.92; js-find-symbol automatically activates ido-mode

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> This code is there since the day js.el was added, so I don't think
> removing the activation of ido-mode is something we can do, because
> users might expect that by now.

User backward-compatibility is nice but not important enough to imply
"cannot do".  I think in this case, IDO has lost enough popularity by
now that more users will be happy rather than annoyed by the change.

> Maybe we could make that optional or something.

Users can enable IDO already, and if really some users like IDO for this
command but not elsewhere (which seems highly unlikely to me), we can
add a small ELisp snippet in NEWS using `advice-add`, for them to copy
to their init file.


        Stefan

bug#74615: 30.0.92; js-find-symbol automatically activates ido-mode

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Daniel Mendler <mail@daniel-mendler.de>,  Daniel Colascione
>  <dancol@dancol.org>,  74615@debbugs.gnu.org
> Date: Sat, 30 Nov 2024 14:05:14 -0500
> 
> > This code is there since the day js.el was added, so I don't think
> > removing the activation of ido-mode is something we can do, because
> > users might expect that by now.
> 
> User backward-compatibility is nice but not important enough to imply
> "cannot do".  I think in this case, IDO has lost enough popularity by
> now that more users will be happy rather than annoyed by the change.
> 
> > Maybe we could make that optional or something.
> 
> Users can enable IDO already, and if really some users like IDO for this
> command but not elsewhere (which seems highly unlikely to me), we can
> add a small ELisp snippet in NEWS using `advice-add`, for them to copy
> to their init file.

Sounds like there's consensus to make this change, so I've now
installed it on the master branch, and I'm therefore closing this bug.