bug#9923: 24.0.91; `where-is' does not find recentf menu items (cmds, not files)

bug#9923: 24.0.91; `where-is' does not find recentf menu items (cmds, not files)

[Drew Adams]

emacs -Q
 
M-x recentf-mode  ; turn it on
 
C-h w recentf-cleanup
 
Says that it is not bound to any keys.  And yet it is on menu `File',
submenu `Open Recent' as item `Cleanup list'.
 
This seems to be a general problem for the `Open Recent' menu, including
for any submenus you might add to it.
 
The bug is not about the file-name entries in the `Open Recent' menu,
but the menu items that represent commands should show up as bindings
for `C-h w', no?  If not, why not?

In GNU Emacs 24.0.91.1 (i386-mingw-nt5.1.2600) of 2011-10-30 on MARVIN
 Windowing system distributor `Microsoft Corp.', version 5.1.2600
 configured using `configure --with-gcc (4.6) --no-opt --cflags
 -I"D:/devel/emacs/libs/libXpm-3.5.8/include"
 -I"D:/devel/emacs/libs/libXpm-3.5.8/src"
 -I"D:/devel/emacs/libs/libpng-dev_1.4.3-1/include"
 -I"D:/devel/emacs/libs/zlib-dev_1.2.5-2/include"
 -I"D:/devel/emacs/libs/giflib-4.1.4-1/include"
 -I"D:/devel/emacs/libs/jpeg-6b-4/include"
 -I"D:/devel/emacs/libs/tiff-3.8.2-1/include"
 -I"D:/devel/emacs/libs/gnutls-2.10.1/include" --ldflags
 -L"D:/devel/emacs/libs/gnutls-2.10.1/lib"'
 

bug#9923: 24.0.91; `where-is' does not find recentf menu items (cmds, not files)

[Stefan Monnier]

> M-x recentf-mode  ; turn it on
 
> C-h w recentf-cleanup
 
> Says that it is not bound to any keys.  And yet it is on menu `File',
> submenu `Open Recent' as item `Cleanup list'.
 
> This seems to be a general problem for the `Open Recent' menu, including
> for any submenus you might add to it.
 
> The bug is not about the file-name entries in the `Open Recent' menu,
> but the menu items that represent commands should show up as bindings
> for `C-h w', no?  If not, why not?

Indeed, where-is won't find bindings in menus defined with easy-menu and
using the :filter thingy.  Hmm...


        Stefan

bug#9923: 24.0.91; `where-is' does not find recentf menu items (cmds, not files)

[Drew Adams]

> Indeed, where-is won't find bindings in menus defined with 
> easy-menu and using the :filter thingy.  Hmm...     Stefan

ping

bug#9923: 24.0.91; `where-is' does not find recentf menu items (cmds, not files)

[Lars Ingebrigtsen]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> The bug is not about the file-name entries in the `Open Recent' menu,
>> but the menu items that represent commands should show up as bindings
>> for `C-h w', no?  If not, why not?
>
> Indeed, where-is won't find bindings in menus defined with easy-menu and
> using the :filter thingy.  Hmm...

This problem is still present in Emacs 28.  I tried briefly following
the logic in where_is_internal and friends, but...

I guess we'd have to check for :filter here, evaluate the filter
function, and then look up the command in the resulting ... menu
entries?  Uhm.

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

bug#9923: 24.0.91; `where-is' does not find recentf menu items (cmds, not files)

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

Lars Ingebrigtsen [2021-08-25 14:18:31] wrote:
> Stefan Monnier <monnier@IRO.UMontreal.CA> writes:
>>> The bug is not about the file-name entries in the `Open Recent' menu,
>>> but the menu items that represent commands should show up as bindings
>>> for `C-h w', no?  If not, why not?
>>
>> Indeed, where-is won't find bindings in menus defined with easy-menu and
>> using the :filter thingy.  Hmm...
>
> This problem is still present in Emacs 28.  I tried briefly following
> the logic in where_is_internal and friends, but...
>
> I guess we'd have to check for :filter here, evaluate the filter
> function, and then look up the command in the resulting ... menu
> entries?  Uhm.

Indeed, and evaluating the :filter may not necessarily return the
"right" result (when that result depends on specific conditions of the
time or context when it is run).

So IIRC `where-is` instead uses the "unfiltered" binding.  So you can
often get what you want by being careful about the BINDING you put into
`(menu-item "foo" BINDING :filter ...).


        Stefan

bug#9923: 24.0.91; `where-is' does not find recentf menu items (cmds, not files)

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> So IIRC `where-is` instead uses the "unfiltered" binding.  So you can
> often get what you want by being careful about the BINDING you put into
> `(menu-item "foo" BINDING :filter ...).

Hm...  but in this case, the filter returns all the items:

    (easy-menu-add-item
     (recentf-menu-bar) recentf-menu-path
     (list recentf-menu-title :filter 'recentf-make-menu-items)
     recentf-menu-before)))

And the command we want to get at with

C-h w recentf-cleanup

is in `recentf-make-menu-items' somewhere.

I'm wondering whether where-is could go at this the opposite direction.
I mean, the menu system knows how to expand all this stuff...  so...
could we ask it to do that, and then look at the results of that instead
of parsing the keymap ourselves in where-is?

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

bug#9923: 24.0.91; `where-is' does not find recentf menu items (cmds, not files)

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

Lars Ingebrigtsen [2021-08-26 15:42:54] wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> So IIRC `where-is` instead uses the "unfiltered" binding.  So you can
>> often get what you want by being careful about the BINDING you put into
>> `(menu-item "foo" BINDING :filter ...).
>
> Hm...  but in this case, the filter returns all the items:
>
>     (easy-menu-add-item
>      (recentf-menu-bar) recentf-menu-path
>      (list recentf-menu-title :filter 'recentf-make-menu-items)
>      recentf-menu-before)))

I think the problem is that it's not the same filter: there's the
`:filter` above which is part of the easymenu syntax and there the
`:filter` in the `menu-item` syntax.

Easymenu does indeed use the menu-item-:filter to implement the
easymenu-:filter but IIRC easymenu uses a menu item of the form

    (menu-item TITLE MENU :filter ...)

where MENU is the actual menu in its original easymenu-syntax form (aka
XEmacs syntax), and it's converted to the keymap-syntax by the :filter.

That would explain why `where-is` can't find any commands in MENU since
`where-is` expects it to use a keymap-syntax rather than an easymenu-syntax.

The relevant code is in `easy-menu-create-menu`:

        (pcase keyword
          (:filter
           (setq filter (lambda (menu)
                          (easy-menu-filter-return (funcall arg menu)
                                                   menu-name))))

as well as:

                     ;; The filter expects the menu in its XEmacs form and the
                     ;; pre-filter form will only be passed to the filter
                     ;; anyway, so we'd better not convert it at all (it will
                     ;; be converted on the fly by easy-menu-filter-return).
                     menu-items
                   (append menu (mapcar #'easy-menu-convert-item menu-items))))

So maybe the comment is wrong (I can't vouch for the guy who wrote it
anyway), and we should put the pre-converted menu in there, and then the
filter function should simply ignore its `menu` argument and use the
`menu-items` instead (or something like that).

Such a change could break something somewhere, tho: AFAICT currently,
just like the menu-item-:filter, the easymenu-:filter doesn't have to be
accompanied with a "menu" in proper syntax but it is required only to
return a menu in proper syntax, so the format of `menu-items` is
actually private to the :filter (tho in practice it's usually a valid
menu).


        Stefan

bug#9923: 24.0.91; `where-is' does not find recentf menu items (cmds, not files)

[Juri Linkov]

> I'm wondering whether where-is could go at this the opposite direction.
> I mean, the menu system knows how to expand all this stuff...  so...
> could we ask it to do that, and then look at the results of that instead
> of parsing the keymap ourselves in where-is?

Another question: is it possible not to evaluate :filter in where-is as all?

Yesterday I added to context-menu-map such lines:

    (let ((fun (mouse-posn-property (event-start last-input-event)
                                    'context-menu-function)))

but now noticed that after typing 'C-h m' it fails with:

Debugger entered--Lisp error: (args-out-of-range 1786)
  mouse-posn-property((#<window 178 on *scratch*> 1786 (0 . 0) 0) context-menu-function)
  context-menu-map()
  (lambda (_) (context-menu-map))(ignore)
  where-is-internal(ignore nil t)
  #f(compiled-function (sym) #<bytecode -0x1145d1e4eb4c2a6>)(ignore)
  help-fns--list-local-commands()
  describe-mode()
  funcall-interactively(describe-mode)
  command-execute(describe-mode)

because context-menu functions are called from where-is-internal
without a mouse click.

Some time ago I already tried to workaround this problem
in help-mode-context-menu with:

  (when (and
         ;; First check if `help-fns--list-local-commands'
         ;; used `where-is-internal' to call this function
         ;; with wrong `last-input-event'.
         (eq (current-buffer) (window-buffer (posn-window (event-start last-input-event))))
         (mouse-posn-property (event-start last-input-event) 'mouse-face))

And now it reared its ugly head again.

bug#9923: 24.0.91; `where-is' does not find recentf menu items (cmds, not files)

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

> Another question: is it possible not to evaluate :filter in where-is as all?

Then it'd find even less commands, I think?

> Yesterday I added to context-menu-map such lines:
>
>     (let ((fun (mouse-posn-property (event-start last-input-event)
>                                     'context-menu-function)))
>
> but now noticed that after typing 'C-h m' it fails with:
>
> Debugger entered--Lisp error: (args-out-of-range 1786)
>   mouse-posn-property((#<window 178 on *scratch*> 1786 (0 . 0) 0) context-menu-function)
>   context-menu-map()

Well, you could just check that last-input-event is a mouse event before
calling that function?

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

bug#9923: 24.0.91; `where-is' does not find recentf menu items (cmds, not files)

[Juri Linkov]

>> Another question: is it possible not to evaluate :filter in where-is as all?
>
> Then it'd find even less commands, I think?

I thought that maybe it's possible somehow to avoid :filter evaluation
for menus.

>> Yesterday I added to context-menu-map such lines:
>>
>>     (let ((fun (mouse-posn-property (event-start last-input-event)
>>                                     'context-menu-function)))
>>
>> but now noticed that after typing 'C-h m' it fails with:
>>
>> Debugger entered--Lisp error: (args-out-of-range 1786)
>>   mouse-posn-property((#<window 178 on *scratch*> 1786 (0 . 0) 0) context-menu-function)
>>   context-menu-map()
>
> Well, you could just check that last-input-event is a mouse event before
> calling that function?

This patch fixes the error, but still e.g. in Info buffer
typing 'C-h m q' moves point in the Info buffer to some fixed position
(maybe related to a position in *Help* buffer), i.e. still something is wrong.

diff --git a/lisp/mouse.el b/lisp/mouse.el
index 7d3ed9a0e4..f4d7d86c28 100644
--- a/lisp/mouse.el
+++ b/lisp/mouse.el
@@ -307,15 +307,19 @@ context-menu-filter-function
 (defun context-menu-map ()
   "Return composite menu map."
   (let ((menu (make-sparse-keymap (propertize "Context Menu" 'hide t))))
-    (let ((fun (mouse-posn-property (event-start last-input-event)
-                                    'context-menu-function)))
+    (let ((fun
+           ;; Check if `where-is-internal' calls this in wrong buffer.
+           (when (eq (window-buffer (posn-window (event-start last-input-event)))
+                     (current-buffer))
+             (mouse-posn-property (event-start last-input-event)
+                                  'context-menu-function))))
       (if (functionp fun)
           (setq menu (funcall fun menu))
         (run-hook-wrapped 'context-menu-functions

bug#9923: 24.0.91; `where-is' does not find recentf menu items (cmds, not files)

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

> This patch fixes the error, but still e.g. in Info buffer
> typing 'C-h m q' moves point in the Info buffer to some fixed position
> (maybe related to a position in *Help* buffer), i.e. still something is wrong.

Hm...  it doesn't seem like anything in `context-menu-map' should be
moving point (unless there's a bug in some of those functions it's
calling, which is possible)...

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

bug#9923: 24.0.91; `where-is' does not find recentf menu items (cmds, not files)

[Juri Linkov]

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

>> This patch fixes the error, but still e.g. in Info buffer
>> typing 'C-h m q' moves point in the Info buffer to some fixed position
>> (maybe related to a position in *Help* buffer), i.e. still something is wrong.
>
> Hm...  it doesn't seem like anything in `context-menu-map' should be
> moving point (unless there's a bug in some of those functions it's
> calling, which is possible)...

Actually, this was caused by thing-at-mouse that uses mouse-set-point
in bug#50256 (a detailed description added in bug#50256).

But regarding this bug#9923, and the question about avoiding :filter
evaluation for context menus - since the context menu is bound to 'ignore':

  `(menu-item ,(purecopy "Context Menu") ignore
              :filter (lambda (_) (context-menu-map)))

does it make sense to ignore 'ignore' like in this patch?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: help-fns--list-local-commands-ignore.patch --]
[-- Type: text/x-diff, Size: 475 bytes --]

diff --git a/lisp/help-fns.el b/lisp/help-fns.el
index 2c7956d968..85f305617d 100644
--- a/lisp/help-fns.el
+++ b/lisp/help-fns.el
@@ -1898,6 +1898,8 @@ help-fns--list-local-commands
     (mapatoms
      (lambda (sym)
        (when (and (commandp sym)
+                  ;; Ignore 'ignore'.
+                  (not (eq sym 'ignore))
                   ;; Ignore aliases.
                   (not (symbolp (symbol-function sym)))
                   ;; Ignore everything bound.

bug#9923: 24.0.91; `where-is' does not find recentf menu items (cmds, not files)

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

> does it make sense to ignore 'ignore' like in this patch?

I think so.  Perhaps Stefan has some comments here?

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

bug#9923: 24.0.91; `where-is' does not find recentf menu items (cmds, not files)

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

Lars Ingebrigtsen [2021-08-31 10:36:28] wrote:
> Juri Linkov <juri@linkov.net> writes:
>> does it make sense to ignore 'ignore' like in this patch?
> I think so.  Perhaps Stefan has some comments here?

Not comments here, no,


        Stefan

bug#9923: 24.0.91; `where-is' does not find recentf menu items (cmds, not files)

[Juri Linkov]

>>> @@ -1898,6 +1898,8 @@ help-fns--list-local-commands
>>>      (mapatoms
>>>       (lambda (sym)
>>>         (when (and (commandp sym)
>>> +                  ;; Ignore errors in context-menu.
>>> +                  (not (eq sym 'ignore))
>>>                    ;; Ignore aliases.
>>>                    (not (symbolp (symbol-function sym)))
>>>                    ;; Ignore everything bound.
>>>
>>> does it make sense to ignore 'ignore' like in this patch?
>>
>> I think so.  Perhaps Stefan has some comments here?
>
> Not comments here, no,

Recently clicking the context menu at the end of the buffer raises an error,
and during the short time period before the problem was noticed and fixed,
the error in one context menu item broke completion of M-x.
It might surprise the users why an error in context menus
affects such unrelated things.  So maybe such patch makes sense
to prevent M-x completion from breaking?

diff --git a/lisp/simple.el b/lisp/simple.el
index b0ff4236e1..5581d41356 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -2117,7 +2117,9 @@ read-extended-command--affixation
     (mapcar
      (lambda (command-name)
        (let* ((fun (and (stringp command-name) (intern-soft command-name)))
-              (binding (where-is-internal fun overriding-local-map t))
+              (binding (unless (eq fun 'ignore)
+                         ;; Ignore errors in context-menu.
+                         (where-is-internal fun overriding-local-map t)))
               (obsolete (get fun 'byte-obsolete-info))
               (alias (symbol-function fun))
               (suffix (cond ((symbolp alias)
-- 

bug#9923: 24.0.91; `where-is' does not find recentf menu items (cmds, not files)

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

> Recently clicking the context menu at the end of the buffer raises an error,
> and during the short time period before the problem was noticed and fixed,
> the error in one context menu item broke completion of M-x.
> It might surprise the users why an error in context menus
> affects such unrelated things.  So maybe such patch makes sense
> to prevent M-x completion from breaking?
>
> diff --git a/lisp/simple.el b/lisp/simple.el
> index b0ff4236e1..5581d41356 100644
> --- a/lisp/simple.el
> +++ b/lisp/simple.el
> @@ -2117,7 +2117,9 @@ read-extended-command--affixation
>      (mapcar
>       (lambda (command-name)
>         (let* ((fun (and (stringp command-name) (intern-soft command-name)))
> -              (binding (where-is-internal fun overriding-local-map t))
> +              (binding (unless (eq fun 'ignore)
> +                         ;; Ignore errors in context-menu.
> +                         (where-is-internal fun overriding-local-map t)))

I think adding code specific to context-menu in
`read-extended-command--affixation` is a bad idea.

So maybe we should add an `with-demoted-errors` and just mention
context-menu as an example of the kind of errors we want to disregard.
Or maybe we should investigate why `ignore` causes an error here?


        Stefan

bug#9923: 24.0.91; `where-is' does not find recentf menu items (cmds, not files)

[Juri Linkov]

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

> So maybe we should add an `with-demoted-errors` and just mention
> context-menu as an example of the kind of errors we want to disregard.

Another case that I posted earlier was where-is-internal failing
in help-fns--list-local-commands because of context-menu errors.
So maybe with-demoted-errors could be added to both?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: where-is-internal-with-demoted-errors.patch --]
[-- Type: text/x-diff, Size: 1370 bytes --]

diff --git a/lisp/help-fns.el b/lisp/help-fns.el
index 6be5cd4a50..ef4eb447a5 100644
--- a/lisp/help-fns.el
+++ b/lisp/help-fns.el
@@ -1950,7 +1950,9 @@ help-fns--list-local-commands
                   ;; Ignore aliases.
                   (not (symbolp (symbol-function sym)))
                   ;; Ignore everything bound.
-                  (not (where-is-internal sym nil t))
+                  (not (with-demoted-errors
+                           ;; Ignore errors in context-menu.
+                           (where-is-internal sym nil t)))
                   (apply #'derived-mode-p (command-modes sym)))
          (push sym functions))))
     (with-temp-buffer
diff --git a/lisp/simple.el b/lisp/simple.el
index 3695415163..d90fd7efdf 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -2120,7 +2120,9 @@ read-extended-command--affixation
     (mapcar
      (lambda (command-name)
        (let* ((fun (and (stringp command-name) (intern-soft command-name)))
-              (binding (where-is-internal fun overriding-local-map t))
+              (binding (with-demoted-errors
+                           ;; Ignore errors in context-menu.
+                           (where-is-internal fun overriding-local-map t)))
               (obsolete (get fun 'byte-obsolete-info))
               (alias (symbol-function fun))
               (suffix (cond ((symbolp alias)

bug#9923: 24.0.91; `where-is' does not find recentf menu items (cmds, not files)

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

> So maybe with-demoted-errors could be added to both?

Sounds good, yes.

> +                  (not (with-demoted-errors
> +                           ;; Ignore errors in context-menu.
> +                           (where-is-internal sym nil t)))

Please provide the FORMAT arg to `with-demoted-errors`.
Also, please clarify the comment so it doesn't make the reader think
that this is specific to the context-menu.


        Stefan

bug#9923: 24.0.91; `where-is' does not find recentf menu items (cmds, not files)

[Juri Linkov]

>> +                  (not (with-demoted-errrs
>> +                           ;; Ignore errors in context-menu.
>> +                           (where-is-internal sym nil t)))
>
> Please provide the FORMAT arg to `with-demoted-errors`.

Oops, I noticed incorrect indentation of the macro body,
but not realized it wants an argument.  I should have tested it,
sorry.

Now that I've tested it, I see no effect from with-demoted-errors.
When debug-on-error is t, it still raises the *Backtrace* buffer.
When debug-on-error is nil, it works fine in both reported cases
even without with-demoted-errors, and displays such a suggested key
in M-x completions:

  ignore (<XF86WakeUp>)

bug#9923: 24.0.91; `where-is' does not find recentf menu items (cmds, not files)

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

> Now that I've tested it, I see no effect from with-demoted-errors.
> When debug-on-error is t, it still raises the *Backtrace* buffer.
> When debug-on-error is nil, it works fine in both reported cases
> even without with-demoted-errors,

Sounds like there's already a `with-demoted-errors` handling this
somewhere in the caller(s).
IOW, there's nothing to fix.

> and displays such a suggested key
> in M-x completions:
>
>   ignore (<XF86WakeUp>)

I suspect `ignore` should be annotated so it's not listed in the
completions of `M-x`, with something like a `completion-predicate`.


        Stefan

bug#9923: 24.0.91; `where-is' does not find recentf menu items (cmds, not files)

[Juri Linkov]

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

> but now noticed that after typing 'C-h m' it fails with:
>
> Debugger entered--Lisp error: (args-out-of-range 1786)
>   mouse-posn-property((#<window 178 on *scratch*> 1786 (0 . 0) 0) context-menu-function)
>   context-menu-map()
>   (lambda (_) (context-menu-map))(ignore)
>   where-is-internal(ignore nil t)
>   #f(compiled-function (sym) #<bytecode -0x1145d1e4eb4c2a6>)(ignore)
>   help-fns--list-local-commands()
>   describe-mode()
>   funcall-interactively(describe-mode)
>   command-execute(describe-mode)

This bug still exists in emacs-28:

emacs-28 -Q
1. context-menu-mode
2. C-h C-n M->
3. C-h m

Debugger entered--Lisp error: (args-out-of-range #<buffer *Help*> 1 37446)
  parse-partial-sexp(1 37446)
  syntax-ppss(37446)
  context-menu-region((keymap #("Context Menu" 0 12 (hide t)) (separator-undo "--") (separator-region "--")) 109)
  #f(compiled-function (fun) #<bytecode -0x1ee96643fef4b180>)(context-menu-region)
  run-hook-wrapped(#f(compiled-function (fun) #<bytecode -0x1ee96643fef4b180>) context-menu-region)
  context-menu-map()
  (lambda (_) (context-menu-map))(ignore)
  where-is-internal(ignore nil t)
  #f(compiled-function (sym) #<bytecode -0x1403be359693d56a>)(ignore)
  help-fns--list-local-commands()
  describe-mode()
  funcall-interactively(describe-mode)
  call-interactively(describe-mode nil nil)
  command-execute(describe-mode)

I don't know if this is the right fix, but it avoids
the error by explicitly switching to window-buffer
when context-menu is called by describe-mode:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: describe-mode-window-buffer.patch --]
[-- Type: text/x-diff, Size: 2090 bytes --]

diff --git a/lisp/mouse.el b/lisp/mouse.el
index 30fe5e99ff..851da77ce0 100644
--- a/lisp/mouse.el
+++ b/lisp/mouse.el
@@ -488,14 +488,15 @@ context-menu-region
       `(menu-item "All"
                   ,(lambda (e) (interactive "e") (mark-thing-at-mouse e 'buffer))
                   :help "Mark the whole buffer for a subsequent cut/copy"))
-    (when (let* ((pos (posn-point (event-end click)))
-                 (char (when pos (char-after pos))))
-            (or (and char (eq (char-syntax char) ?\"))
-                (nth 3 (save-excursion (syntax-ppss pos)))))
-      (define-key-after submenu [mark-string]
-        `(menu-item "String"
-                    ,(lambda (e) (interactive "e") (mark-thing-at-mouse e 'string))
-                    :help "Mark the string at click for a subsequent cut/copy")))
+    (with-current-buffer (window-buffer)
+      (when (let* ((pos (posn-point (event-end click)))
+                   (char (when pos (char-after pos))))
+              (or (and char (eq (char-syntax char) ?\"))
+                  (nth 3 (save-excursion (syntax-ppss pos)))))
+        (define-key-after submenu [mark-string]
+          `(menu-item "String"
+                      ,(lambda (e) (interactive "e") (mark-thing-at-mouse e 'string))
+                      :help "Mark the string at click for a subsequent cut/copy"))))
     (define-key-after submenu [mark-line]
       `(menu-item "Line"
                   ,(lambda (e) (interactive "e") (mark-thing-at-mouse e 'line))
diff --git a/lisp/progmodes/prog-mode.el b/lisp/progmodes/prog-mode.el
index 30b6edf0d9..bf1f619858 100644
--- a/lisp/progmodes/prog-mode.el
+++ b/lisp/progmodes/prog-mode.el
@@ -93,7 +93,9 @@ prog-context-menu
     'mark-whole-buffer)
 
   ;; Include text-mode select menu only in strings and comments.
-  (when (nth 8 (save-excursion (syntax-ppss (posn-point (event-end click)))))
+  (when (nth 8 (save-excursion
+                 (with-current-buffer (window-buffer)
+                   (syntax-ppss (posn-point (event-end click))))))
     (text-mode-context-menu menu click))
 
   menu)

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


An alternative is the patch that I proposed earlier:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: help-fns--list-local-commands-ignore.patch --]
[-- Type: text/x-diff, Size: 475 bytes --]

diff --git a/lisp/help-fns.el b/lisp/help-fns.el
index 2c7956d968..85f305617d 100644
--- a/lisp/help-fns.el
+++ b/lisp/help-fns.el
@@ -1898,6 +1898,8 @@ help-fns--list-local-commands
     (mapatoms
      (lambda (sym)
        (when (and (commandp sym)
+                  ;; Ignore 'ignore'.
+                  (not (eq sym 'ignore))
                   ;; Ignore aliases.
                   (not (symbolp (symbol-function sym)))
                   ;; Ignore everything bound.

bug#9923: 24.0.91; `where-is' does not find recentf menu items (cmds, not files)

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

> I don't know if this is the right fix, but it avoids
> the error by explicitly switching to window-buffer
> when context-menu is called by describe-mode:

I think that makes sense, but perhaps Stefan has some comments here...

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

bug#9923: 24.0.91; `where-is' does not find recentf menu items (cmds, not files)

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

> +    (with-current-buffer (window-buffer)
> +      (when (let* ((pos (posn-point (event-end click)))
> +                   (char (when pos (char-after pos))))
> +              (or (and char (eq (char-syntax char) ?\"))
> +                  (nth 3 (save-excursion (syntax-ppss pos)))))
> +        (define-key-after submenu [mark-string]
> +          `(menu-item "String"
> +                      ,(lambda (e) (interactive "e") (mark-thing-at-mouse e 'string))
> +                      :help "Mark the string at click for a subsequent cut/copy"))))

I think it would be more "obviously correct" if we switched to
(window-buffer (posn-window (event-end click))) instead.

>    ;; Include text-mode select menu only in strings and comments.
> -  (when (nth 8 (save-excursion (syntax-ppss (posn-point (event-end click)))))
> +  (when (nth 8 (save-excursion
> +                 (with-current-buffer (window-buffer)
> +                   (syntax-ppss (posn-point (event-end click))))))

Same here.

> An alternative is the patch that I proposed earlier:
>
> diff --git a/lisp/help-fns.el b/lisp/help-fns.el
> index 2c7956d968..85f305617d 100644
> --- a/lisp/help-fns.el
> +++ b/lisp/help-fns.el
> @@ -1898,6 +1898,8 @@ help-fns--list-local-commands
>      (mapatoms
>       (lambda (sym)
>         (when (and (commandp sym)
> +                  ;; Ignore 'ignore'.
> +                  (not (eq sym 'ignore))
>                    ;; Ignore aliases.
>                    (not (symbolp (symbol-function sym)))
>                    ;; Ignore everything bound.

I'm not sure what to think of this because I don't understand
its motivation.  IOW at the very least it should come with a comment
explaining why we need to ignore `ignore`.


        Stefan

bug#9923: 24.0.91; `where-is' does not find recentf menu items (cmds, not files)

[Juri Linkov]

> I think it would be more "obviously correct" if we switched to
> (window-buffer (posn-window (event-end click))) instead.
>
>>    ;; Include text-mode select menu only in strings and comments.
>> -  (when (nth 8 (save-excursion (syntax-ppss (posn-point (event-end click)))))
>> +  (when (nth 8 (save-excursion
>> +                 (with-current-buffer (window-buffer)
>> +                   (syntax-ppss (posn-point (event-end click))))))
>
> Same here.

Will do this if a simpler solution below is not suitable.

>> +++ b/lisp/help-fns.el
>> @@ -1898,6 +1898,8 @@ help-fns--list-local-commands
>>      (mapatoms
>>       (lambda (sym)
>>         (when (and (commandp sym)
>> +                  ;; Ignore 'ignore'.
>> +                  (not (eq sym 'ignore))
>>                    ;; Ignore aliases.
>>                    (not (symbolp (symbol-function sym)))
>>                    ;; Ignore everything bound.
>
> I'm not sure what to think of this because I don't understand
> its motivation.  IOW at the very least it should come with a comment
> explaining why we need to ignore `ignore`.

The problem is that `describe-mode` calls `context-menu-map`
that it should not do:

  context-menu-map()
  (lambda (_) (context-menu-map))(ignore)
  where-is-internal(ignore nil t)
  help-fns--list-local-commands()
  describe-mode()
  funcall-interactively(describe-mode)
  call-interactively(describe-mode nil nil)
  command-execute(describe-mode)

`context-menu-map' is bound to `ignore', so ignoring `ignore'
will prevent calling it from help-fns--list-local-commands.

bug#9923: 24.0.91; `where-is' does not find recentf menu items (cmds, not files)

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

>> I think it would be more "obviously correct" if we switched to
>> (window-buffer (posn-window (event-end click))) instead.
>>
>>>    ;; Include text-mode select menu only in strings and comments.
>>> -  (when (nth 8 (save-excursion (syntax-ppss (posn-point (event-end click)))))
>>> +  (when (nth 8 (save-excursion
>>> +                 (with-current-buffer (window-buffer)
>>> +                   (syntax-ppss (posn-point (event-end click))))))
>>
>> Same here.
>
> Will do this if a simpler solution below is not suitable.

I think the hunk above (and the one before that) is a good change
regardless of this specific bug: it doesn't make sense to use
`posn-point` without making sure we're in (window-buffer (posn-window ...)).

> The problem is that `describe-mode` calls `context-menu-map`
> that it should not do:
>
>   context-menu-map()
>   (lambda (_) (context-menu-map))(ignore)
>   where-is-internal(ignore nil t)

AFAIK that's because an event is bound to a `menu-item` whose `:filter`
is the lambda above, used to dynamically generate the keymap, so
`where-is-internal` runs the filter to try and see if `ignore` is bound
in the dynamically-generated map.

> `context-menu-map' is bound to `ignore',

Is it?

> so ignoring `ignore' will prevent calling it from
> help-fns--list-local-commands.

Hmm... taking a second look at the `mouse.el` file, I think the problem
is rather than the menu-item has an nominal/default/static binding of
`ignore`.  So I guess the `:filter` isn't run to try and look for
`ignore` in the dynamically-generated map but instead it's run later
(when `where-is-internal` checks that the binding it found is real).

So maybe another way to avoid running the filter is with the
patch below.


        Stefan


diff --git a/lisp/mouse.el b/lisp/mouse.el
index 5c645a4b895..9e902ca25e0 100644
--- a/lisp/mouse.el
+++ b/lisp/mouse.el
@@ -513,7 +513,7 @@ context-menu-ffap
   menu)
 
 (defvar context-menu-entry
-  `(menu-item ,(purecopy "Context Menu") ignore
+  `(menu-item ,(purecopy "Context Menu") ,(make-sparse-keymap)
               :filter (lambda (_) (context-menu-map)))
   "Menu item that creates the context menu and can be bound to a mouse key.")
 

bug#9923: 24.0.91; `where-is' does not find recentf menu items (cmds, not files)

[Juri Linkov]

>>> I think it would be more "obviously correct" if we switched to
>>> (window-buffer (posn-window (event-end click))) instead.
>>
>> Will do this if a simpler solution below is not suitable.
>
> I think the hunk above (and the one before that) is a good change
> regardless of this specific bug: it doesn't make sense to use
> `posn-point` without making sure we're in (window-buffer (posn-window ...)).

So this fix is pushed to emacs-28.

> Hmm... taking a second look at the `mouse.el` file, I think the problem
> is rather than the menu-item has an nominal/default/static binding of
> `ignore`.  So I guess the `:filter` isn't run to try and look for
> `ignore` in the dynamically-generated map but instead it's run later
> (when `where-is-internal` checks that the binding it found is real).
>
> So maybe another way to avoid running the filter is with the
> patch below.
>
> --- a/lisp/mouse.el
> +++ b/lisp/mouse.el
> @@ -513,7 +513,7 @@ context-menu-ffap
>    menu)
>  
>  (defvar context-menu-entry
> -  `(menu-item ,(purecopy "Context Menu") ignore
> +  `(menu-item ,(purecopy "Context Menu") ,(make-sparse-keymap)
>                :filter (lambda (_) (context-menu-map)))
>    "Menu item that creates the context menu and can be bound to a mouse key.")

I confirm that it prevents `where-is-internal` from calling `context-menu-map`,
and everything else works fine, but I'm not sure how this change fits
into the overall design of the menu bindings.  Maybe, `make-sparse-keymap`
means that `context-menu-entry` contains an empty menu initially.

bug#9923: 24.0.91; `where-is' does not find recentf menu items (cmds, not files)

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

> I confirm that it prevents `where-is-internal` from calling `context-menu-map`,
> and everything else works fine, but I'm not sure how this change fits
> into the overall design of the menu bindings.  Maybe, `make-sparse-keymap`
> means that `context-menu-entry` contains an empty menu initially.

No, it's just that `where-is-internal` by-and-large doesn't run the
`:filter`s when doing its search, so instead of seeing a binding to
`ignore` it will see a binding to an empty keymap.

As we've seen `where-is-internal` does end up running the `:filter`
sometimes, but it's only done on those few bindings that match.

When building a menu, the `ignore` or the empty keymap won't be used at
all (their only use will be to pass them to the `:filter` function,
which ignores that argument).  That's why the "binding" is currently
called `ignore`: not because we want to use the `ignore` command but as
a way to say "this is not used".


        Stefan

bug#9923: 24.0.91; `where-is' does not find recentf menu items (cmds, not files)

[Juri Linkov]

>> I confirm that it prevents `where-is-internal` from calling `context-menu-map`,
>> and everything else works fine, but I'm not sure how this change fits
>> into the overall design of the menu bindings.  Maybe, `make-sparse-keymap`
>> means that `context-menu-entry` contains an empty menu initially.
>
> No, it's just that `where-is-internal` by-and-large doesn't run the
> `:filter`s when doing its search, so instead of seeing a binding to
> `ignore` it will see a binding to an empty keymap.
>
> As we've seen `where-is-internal` does end up running the `:filter`
> sometimes, but it's only done on those few bindings that match.
>
> When building a menu, the `ignore` or the empty keymap won't be used at
> all (their only use will be to pass them to the `:filter` function,
> which ignores that argument).  That's why the "binding" is currently
> called `ignore`: not because we want to use the `ignore` command but as
> a way to say "this is not used".

I pushed this fix to master, but not sure if it should be backported to emacs-28.

bug#9923: 24.0.91; `where-is' does not find recentf menu items (cmds, not files)

[Juri Linkov]

> Hmm... taking a second look at the `mouse.el` file, I think the problem
> is rather than the menu-item has an nominal/default/static binding of
> `ignore`.  So I guess the `:filter` isn't run to try and look for
> `ignore` in the dynamically-generated map but instead it's run later
> (when `where-is-internal` checks that the binding it found is real).
>
> So maybe another way to avoid running the filter is with the
> patch below.
>
> diff --git a/lisp/mouse.el b/lisp/mouse.el
> index 5c645a4b895..9e902ca25e0 100644
> --- a/lisp/mouse.el
> +++ b/lisp/mouse.el
> @@ -513,7 +513,7 @@ context-menu-ffap
>    menu)
>  
>  (defvar context-menu-entry
> -  `(menu-item ,(purecopy "Context Menu") ignore
> +  `(menu-item ,(purecopy "Context Menu") ,(make-sparse-keymap)
>                :filter (lambda (_) (context-menu-map)))
>    "Menu item that creates the context menu and can be bound to a mouse key.")

`where-is-internal(ignore)` raises its ugly head again:

0. emacs -Q
1. M-x TAB

Debugger entered--Lisp error: (void-function cl--set-substring)
  cl--set-substring(#("*scratch* x" 0 9 ...
  tab-bar-auto-width(((sep-1 menu-item " " ignore) (current-tab menu-item #("*scratch* x" 0 9 ...
  tab-bar-make-keymap-1()
  tab-bar-make-keymap(ignore)
  where-is-internal(ignore nil t)
  read-extended-command--affixation((#("2C-associate-buffer" ...
  #f(compiled-function (window) ...
  window--display-buffer(#<buffer *Completions*> ...
  display-buffer-at-bottom(#<buffer *Completions*> ...
  display-buffer(#<buffer *Completions*> ...
  temp-buffer-window-show(#<buffer *Completions*> ...
  minibuffer-completion-help(5 5)
  completion--do-completion(5 5)
  completion--in-region-1(5 5)
  completion--in-region(5 5 #f(compiled-function (string pred action) ..
  completion-in-region(5 5 #f(compiled-function (string pred action) ...
  minibuffer-complete()
  funcall-interactively(minibuffer-complete)
  call-interactively(minibuffer-complete nil nil)
  command-execute(minibuffer-complete)
  completing-read-default("M-x " ...
  completing-read("M-x " ...
  read-extended-command-1("M-x " nil)
  read-extended-command()
  call-interactively(execute-extended-command nil nil)
  command-execute(execute-extended-command)

But using `make-sparse-keymap` fixes it again:

```
diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el
index 1baa857e9e9..f040bc9786d 100644
--- a/lisp/tab-bar.el
+++ b/lisp/tab-bar.el
@@ -416,7 +416,7 @@ tab-bar-map
   "S-<wheel-right>" #'tab-bar-move-tab)
 
 (global-set-key [tab-bar]
-                `(menu-item ,(purecopy "tab bar") ignore
+                `(menu-item ,(purecopy "tab bar") ,(make-sparse-keymap)
                             :filter tab-bar-make-keymap))
```

Not sure if this trick should be documented somewhere.

bug#9923: 24.0.91; `where-is' does not find recentf menu items (cmds, not files)

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

> `where-is-internal(ignore)` raises its ugly head again:
>
> 0. emacs -Q
> 1. M-x TAB
>
> Debugger entered--Lisp error: (void-function cl--set-substring)
>   cl--set-substring(#("*scratch* x" 0 9 ...
>   tab-bar-auto-width(((sep-1 menu-item " " ignore) (current-tab menu-item #("*scratch* x" 0 9 ...
[...]
> But using `make-sparse-keymap` fixes it again:

Yay!

But we should also fix the `cl--set-substring` error there.


        Stefan

bug#9923: 24.0.91; `where-is' does not find recentf menu items (cmds, not files)

[Juri Linkov]

>> `where-is-internal(ignore)` raises its ugly head again:
>>
>> 0. emacs -Q
>> 1. M-x TAB
>>
>> Debugger entered--Lisp error: (void-function cl--set-substring)
>>   cl--set-substring(#("*scratch* x" 0 9 ...
>>   tab-bar-auto-width(((sep-1 menu-item " " ignore) (current-tab menu-item #("*scratch* x" 0 9 ...
> [...]
>> But using `make-sparse-keymap` fixes it again:
>
> Yay!
>
> But we should also fix the `cl--set-substring` error there.

I don't know how to fix it when it's not broken anymore
after adding `(make-sparse-keymap)`.