bug#52389: 29.0.50; Imenu does not pop up the Completions eagerly

bug#52389: 29.0.50; Imenu does not pop up the Completions eagerly

[Protesilaos Stavrou]

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

Dear maintainers,

I think the current behaviour of 'imenu-eager-completion-buffer' is
counter-intuitive.  Its default value is non-nil, while its doc string
tells us that:

    If non-nil, eagerly popup the completion buffer.

This, however, is not what actually happens.  With 'emacs -Q':

+ Include a demo function in the scratch buffer: (defun test-fn ())
+ M-x imenu
+ Here we expect the Completions' buffer to eagerly pop up, yet it does
  not.
+ M-: (setq imenu-eager-completion-buffer nil)
+ M-x imenu
+ The Completions' buffer now pops up automatically.

Do you agree that this is counter-intuitive?  If so, please find
attached a patch that should fix the issue.  I was not sure this was
NEWSworthy, but I included an entry regardless.

All the best,
Protesilaos

-- 
Protesilaos Stavrou
https://protesilaos.com

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-semantics-of-imenu-eager-completion-buffer.patch --]
[-- Type: text/x-patch, Size: 2379 bytes --]

From b4625110cb5b40f079c134a73e9be8240818fa33 Mon Sep 17 00:00:00 2001
Message-Id: <b4625110cb5b40f079c134a73e9be8240818fa33.1639055998.git.info@protesilaos.com>
From: Protesilaos Stavrou <info@protesilaos.com>
Date: Thu, 9 Dec 2021 15:19:48 +0200
Subject: [PATCH] Fix semantics of imenu-eager-completion-buffer

* etc/NEWS: Document the change.
* lisp/imenu.el (imenu-eager-completion-buffer): Set the default value
to nil, as that was the intended one.
(imenu--completion-buffer): Update the condition to check for a
non-nil value to 'imenu-eager-completion-buffer'.
---
 etc/NEWS      | 7 +++++++
 lisp/imenu.el | 6 +++---
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index b9134d850e..a83f2d2a86 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -706,6 +706,13 @@ the Netscape web browser was released in February, 2008.
 This support has been obsolete since Emacs 25.1.  The final version of
 the Galeon web browser was released in September, 2008.
 
+** Imenu
+The default value of 'imenu-eager-completion-buffer' is set to nil to
+better convey the intended semantics of optionally popping up the
+Completions' buffer automatically after invoking M-x imenu (with the
+default settings from previous versions, the Completions would not pop
+up automatically and the same is true now).
+
 \f
 * New Modes and Packages in Emacs 29.1
 
diff --git a/lisp/imenu.el b/lisp/imenu.el
index 22412d5f88..bf12e110e5 100644
--- a/lisp/imenu.el
+++ b/lisp/imenu.el
@@ -99,10 +99,10 @@ (defcustom imenu-use-popup-menu 'on-mouse
 		 (const :tag "Never" nil)
                  (other :tag "Always" t)))
 
-(defcustom imenu-eager-completion-buffer t
+(defcustom imenu-eager-completion-buffer nil
   "If non-nil, eagerly popup the completion buffer."
   :type 'boolean
-  :version "22.1")
+  :version "29.1")
 
 (defcustom imenu-after-jump-hook nil
   "Hooks called after jumping to a place in the buffer.
@@ -727,7 +727,7 @@ (defun imenu--completion-buffer (index-alist &optional prompt)
                          name))))
     (let ((minibuffer-setup-hook minibuffer-setup-hook))
       ;; Display the completion buffer.
-      (if (not imenu-eager-completion-buffer)
+      (when imenu-eager-completion-buffer
 	  (add-hook 'minibuffer-setup-hook 'minibuffer-completion-help))
       (setq name (completing-read prompt
 				  prepared-index-alist
-- 
2.34.1

bug#52389: 29.0.50; Imenu does not pop up the Completions eagerly

[Lars Ingebrigtsen]

Protesilaos Stavrou <info@protesilaos.com> writes:

> * lisp/imenu.el (imenu-eager-completion-buffer): Set the default value
> to nil, as that was the intended one.
> (imenu--completion-buffer): Update the condition to check for a
> non-nil value to 'imenu-eager-completion-buffer'.

But you're reversing the logic, so that people who have set this to nil
now won't get an eager pop up?

I don't think we can do that.  The documentation (and indeed the name of
the variable) seems to be wrong, though.

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

bug#52389: 29.0.50; Imenu does not pop up the Completions eagerly

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Protesilaos Stavrou <info@protesilaos.com> writes:
>
>> * lisp/imenu.el (imenu-eager-completion-buffer): Set the default value
>> to nil, as that was the intended one.
>> (imenu--completion-buffer): Update the condition to check for a
>> non-nil value to 'imenu-eager-completion-buffer'.
>
> But you're reversing the logic, so that people who have set this to nil
> now won't get an eager pop up?
>
> I don't think we can do that.  The documentation (and indeed the name of
> the variable) seems to be wrong, though.

I think the safest way to solve this long-standing problem without
surprising users is to introduce a new variable whose logic is handled
correctly, and mark imenu-eager-completion-buffer as deprecated.

bug#52389: 29.0.50; Imenu does not pop up the Completions eagerly

[Lars Ingebrigtsen]

Daniel Martín <mardani29@yahoo.es> writes:

> I think the safest way to solve this long-standing problem without
> surprising users is to introduce a new variable whose logic is handled
> correctly, and mark imenu-eager-completion-buffer as deprecated.

Sounds good to me.  (The new variable could be an alias of the old name,
but have a name with the opposite meaning, and then things would
continue to work for everybody.)

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

bug#52389: 29.0.50; Imenu does not pop up the Completions eagerly

[Juri Linkov]

>> I think the safest way to solve this long-standing problem without
>> surprising users is to introduce a new variable whose logic is handled
>> correctly, and mark imenu-eager-completion-buffer as deprecated.
>
> Sounds good to me.  (The new variable could be an alias of the old name,
> but have a name with the opposite meaning, and then things would
> continue to work for everybody.)

This is a good opportunity for generalization.
Like there is icomplete-show-matches-on-no-input,
would it be possible to create a new user option
e.g. minibuffer-auto-show-completions, and allow
to customize what commands should use it.
Then it could be like completion-auto-select
recently proposed by Philip.

bug#52389: 29.0.50; Imenu does not pop up the Completions eagerly

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

> This is a good opportunity for generalization.
> Like there is icomplete-show-matches-on-no-input,
> would it be possible to create a new user option
> e.g. minibuffer-auto-show-completions, and allow
> to customize what commands should use it.

Hm...  I think that sounds rather confusing...

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

bug#52389: 29.0.50; Imenu does not pop up the Completions eagerly

[Protesilaos Stavrou]

On 2021-12-11, 22:16 +0200, Juri Linkov <juri@linkov.net> wrote:

>>> I think the safest way to solve this long-standing problem without
>>> surprising users is to introduce a new variable whose logic is handled
>>> correctly, and mark imenu-eager-completion-buffer as deprecated.
>>
>> Sounds good to me.  (The new variable could be an alias of the old name,
>> but have a name with the opposite meaning, and then things would
>> continue to work for everybody.)
>
> This is a good opportunity for generalization.
> Like there is icomplete-show-matches-on-no-input,
> would it be possible to create a new user option
> e.g. minibuffer-auto-show-completions, and allow
> to customize what commands should use it.
> Then it could be like completion-auto-select
> recently proposed by Philip.

That is possible with my mct package on GNU ELPA.  It has the option of
a passlist for commands that should eagerly pop up the Completions'
buffer.

[ Philip mentioned mct in those patches. ]

Note, however, that the case here with Imenu is different because even
if you pop the completions eagerly upon M-x imenu, you still need to
handle the case of nested Imenu entries.  That is what the user option
imenu-eager-completion-buffer helps deal with.  Actually, what prompted
me to report this bug was due to an issue in the mct repo, where I
discovered the confusing doc string of that Imenu user option:
<https://gitlab.com/protesilaos/mct/-/issues/12>.

-- 
Protesilaos Stavrou
https://protesilaos.com

bug#52389: 29.0.50; Imenu does not pop up the Completions eagerly

[Juri Linkov]

>> This is a good opportunity for generalization.
>> Like there is icomplete-show-matches-on-no-input,
>> would it be possible to create a new user option
>> e.g. minibuffer-auto-show-completions, and allow
>> to customize what commands should use it.
>> Then it could be like completion-auto-select
>> recently proposed by Philip.
>
> That is possible with my mct package on GNU ELPA.  It has the option of
> a passlist for commands that should eagerly pop up the Completions'
> buffer.
>
> [ Philip mentioned mct in those patches. ]
>
> Note, however, that the case here with Imenu is different because even
> if you pop the completions eagerly upon M-x imenu, you still need to
> handle the case of nested Imenu entries.  That is what the user option
> imenu-eager-completion-buffer helps deal with.  Actually, what prompted
> me to report this bug was due to an issue in the mct repo, where I
> discovered the confusing doc string of that Imenu user option:
> <https://gitlab.com/protesilaos/mct/-/issues/12>.

Thanks for the reference, I see the problem now: after adding ‘imenu’ to
mct-completion-passlist, ‘M-x imenu RET’ auto-displays completions
correctly.  But the problem is that selecting e.g. a nested completion
doesn't auto-display the nested completion list, because in the first
case ‘this-command’ is ‘imenu’, but in the second case it's a command
that selects the nested completion.

Fortunately, it's possible to solve this problem using
the new variable introduced in 28.1: ‘current-minibuffer-command’
that is like ‘this-command’, but preserves its value
during the nested minibuffer activities.  To make it
backward-compatibile with Emacs 27.1, 'bound-and-true-p'
could help to check if the variable is defined, e.g.

#+begin_src emacs-lisp
   ((memq (or (bound-and-true-p current-minibuffer-command)
              this-command)
          mct-completion-passlist)
#+end_src

Then 'imenu' in 'mct-completion-passlist' will be available
for the nested minibuffer commands.

PS: Overall, your package mct makes progress towards better
minibuffer/completions interactions.  However, its behaviour
can't be enabled by default in Emacs, because e.g. such keys
as up/down arrows traditionally are used for history navigation.

bug#52389: 29.0.50; Imenu does not pop up the Completions eagerly

[Protesilaos Stavrou]

On 2021-12-12, 19:36 +0200, Juri Linkov <juri@linkov.net> wrote:

>>> This is a good opportunity for generalization.
>>> Like there is icomplete-show-matches-on-no-input,
>>> would it be possible to create a new user option
>>> e.g. minibuffer-auto-show-completions, and allow
>>> to customize what commands should use it.
>>> Then it could be like completion-auto-select
>>> recently proposed by Philip.
>>
>> That is possible with my mct package on GNU ELPA.  It has the option of
>> a passlist for commands that should eagerly pop up the Completions'
>> buffer.
>>
>> [ Philip mentioned mct in those patches. ]
>>
>> Note, however, that the case here with Imenu is different because even
>> if you pop the completions eagerly upon M-x imenu, you still need to
>> handle the case of nested Imenu entries.  That is what the user option
>> imenu-eager-completion-buffer helps deal with.  Actually, what prompted
>> me to report this bug was due to an issue in the mct repo, where I
>> discovered the confusing doc string of that Imenu user option:
>> <https://gitlab.com/protesilaos/mct/-/issues/12>.
>
> Thanks for the reference, I see the problem now: after adding ‘imenu’ to
> mct-completion-passlist, ‘M-x imenu RET’ auto-displays completions
> correctly.  But the problem is that selecting e.g. a nested completion
> doesn't auto-display the nested completion list, because in the first
> case ‘this-command’ is ‘imenu’, but in the second case it's a command
> that selects the nested completion.
>
> Fortunately, it's possible to solve this problem using
> the new variable introduced in 28.1: ‘current-minibuffer-command’
> that is like ‘this-command’, but preserves its value
> during the nested minibuffer activities.  To make it
> backward-compatibile with Emacs 27.1, 'bound-and-true-p'
> could help to check if the variable is defined, e.g.
>
> #+begin_src emacs-lisp
>    ((memq (or (bound-and-true-p current-minibuffer-command)
>               this-command)
>           mct-completion-passlist)
> #+end_src
>
> Then 'imenu' in 'mct-completion-passlist' will be available
> for the nested minibuffer commands.

Update things accordingly.  Thank you!

> PS: Overall, your package mct makes progress towards better
> minibuffer/completions interactions.  However, its behaviour
> can't be enabled by default in Emacs, because e.g. such keys
> as up/down arrows traditionally are used for history navigation.

That's okay.  I thought it was worth putting it on GNU ELPA as a
proof-of-concept.  Patching Emacs the way Philip suggests should be the
right way forward.

* * *

Regarding this bug report, I am not sure what the status is.  Should I
rewrite the patch to specify a define-obsolete-variable-alias instead?
Or is someone else taking care of it?

-- 
Protesilaos Stavrou
https://protesilaos.com

bug#52389: 29.0.50; Imenu does not pop up the Completions eagerly

[Juri Linkov]

> Regarding this bug report, I am not sure what the status is.  Should I
> rewrite the patch to specify a define-obsolete-variable-alias instead?
> Or is someone else taking care of it?

It seems everyone agrees here it would be preferable to
rewrite the patch using define-obsolete-variable-alias.

Any generalization that uses current-minibuffer-command
could be implemented later, without impeding the fix
for this bug report.

bug#52389: 29.0.50; Imenu does not pop up the Completions eagerly

[Protesilaos Stavrou]

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

On 2021-12-13, 10:53 +0200, Juri Linkov <juri@linkov.net> wrote:

>> Regarding this bug report, I am not sure what the status is.  Should I
>> rewrite the patch to specify a define-obsolete-variable-alias instead?
>> Or is someone else taking care of it?
>
> It seems everyone agrees here it would be preferable to
> rewrite the patch using define-obsolete-variable-alias.

Please find attached the patch that implements the new user option.  I
did not include a NEWS entry as things should work the way they did
before.

The combined use of "not" and "no" concepts in this case feels awkward.
I prefer my original approach of changing the logic, though I understand
it would be backward-incompatible.

-- 
Protesilaos Stavrou
https://protesilaos.com

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Deprecate-Imenu-option-for-eager-completion-buffer.patch --]
[-- Type: text/x-patch, Size: 1932 bytes --]

From 2487aaba2f531563e1acc0979a738ade7bb322f2 Mon Sep 17 00:00:00 2001
Message-Id: <2487aaba2f531563e1acc0979a738ade7bb322f2.1639741622.git.info@protesilaos.com>
From: Protesilaos Stavrou <info@protesilaos.com>
Date: Fri, 17 Dec 2021 13:46:29 +0200
Subject: [PATCH] Deprecate Imenu option for eager completion buffer

The now-deprecated user option had confusing language as it produced
the opposite effect of what it meant.  Read bug#52389.

* imenu.el (imenu-eager-completion-buffer): Remove old user option.
(imenu-no-eager-completion-buffer): Write alias for the deprecated
user option.
(imenu--completion-buffer): Check for the new variable.
---
 lisp/imenu.el | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/lisp/imenu.el b/lisp/imenu.el
index 22412d5f88..86f7d2d3ff 100644
--- a/lisp/imenu.el
+++ b/lisp/imenu.el
@@ -99,10 +99,15 @@ (defcustom imenu-use-popup-menu 'on-mouse
 		 (const :tag "Never" nil)
                  (other :tag "Always" t)))
 
-(defcustom imenu-eager-completion-buffer t
-  "If non-nil, eagerly popup the completion buffer."
+(define-obsolete-variable-alias
+  'imenu-eager-completion-buffer
+  'imenu-no-eager-completion-buffer
+  "29.1")
+
+(defcustom imenu-no-eager-completion-buffer t
+  "If non-nil, do not eagerly popup the completion buffer."
   :type 'boolean
-  :version "22.1")
+  :version "29.1")
 
 (defcustom imenu-after-jump-hook nil
   "Hooks called after jumping to a place in the buffer.
@@ -727,7 +732,7 @@ (defun imenu--completion-buffer (index-alist &optional prompt)
                          name))))
     (let ((minibuffer-setup-hook minibuffer-setup-hook))
       ;; Display the completion buffer.
-      (if (not imenu-eager-completion-buffer)
+      (if (not imenu-no-eager-completion-buffer)
 	  (add-hook 'minibuffer-setup-hook 'minibuffer-completion-help))
       (setq name (completing-read prompt
 				  prepared-index-alist
-- 
2.34.1

bug#52389: 29.0.50; Imenu does not pop up the Completions eagerly

[André A. Gomes]

Protesilaos Stavrou <info@protesilaos.com> writes:

> On 2021-12-13, 10:53 +0200, Juri Linkov <juri@linkov.net> wrote:
>
>>> Regarding this bug report, I am not sure what the status is.  Should I
>>> rewrite the patch to specify a define-obsolete-variable-alias instead?
>>> Or is someone else taking care of it?
>>
>> It seems everyone agrees here it would be preferable to
>> rewrite the patch using define-obsolete-variable-alias.
>
> Please find attached the patch that implements the new user option.  I
> did not include a NEWS entry as things should work the way they did
> before.
>
> The combined use of "not" and "no" concepts in this case feels awkward.
> I prefer my original approach of changing the logic, though I understand
> it would be backward-incompatible.

Agreed.  I don't understand how the decision moved in this direction.
Perhaps the maintainers will reconsider.


-- 
André A. Gomes
"Free Thought, Free World"

bug#52389: 29.0.50; Imenu does not pop up the Completions eagerly

[Eli Zaretskii]

> From: Protesilaos Stavrou <info@protesilaos.com>
> Date: Fri, 17 Dec 2021 13:57:52 +0200
> Cc: Lars Ingebrigtsen <larsi@gnus.org>, 52389@debbugs.gnu.org,
>  Daniel Martín <mardani29@yahoo.es>
> 
> +(defcustom imenu-no-eager-completion-buffer t
> +  "If non-nil, do not eagerly popup the completion buffer."

The doc string should go on to explain what does it mean to "eagerly
popup a buffer".  It is not self-evident.

(Yes, I know this was in the original text, but while we are at that,
we might as well fix that omission.)

Thanks.

bug#52389: 29.0.50; Imenu does not pop up the Completions eagerly

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> +(defcustom imenu-no-eager-completion-buffer t
>> +  "If non-nil, do not eagerly popup the completion buffer."
>
> The doc string should go on to explain what does it mean to "eagerly
> popup a buffer".  It is not self-evident.
>
> (Yes, I know this was in the original text, but while we are at that,
> we might as well fix that omission.)

And I don't think we should call a variable
`imenu-no-eager-completion-buffer' -- it's very confusing.
`imenu-inhibit-eager-completion-buffer' is better.

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