bug#6305: 24.0.50; Regression: most, but not all, keyboard shortcuts are missing

bug#6305: 24.0.50; Regression: most, but not all, keyboard shortcuts are missing

[Drew Adams]

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

This is a regression from 23.1 to 23.2.

I don't have a recipe from emacs -Q.  I'm sure you could create one.
 
In Emacs 22 and 23.1, keyboard shortcuts ("equivalents", if you
prefer) are indicated correctly in menus.  In both Emacs 23.2 and
the recent build noted below, this is horribly broken.

See the three attached screenshots, for Emacs 22 (OK), Emacs 23.1
(OK), and Emacs 23.2 (broken - build noted below or the release 23.2).
 
There is absolutely nothing particular about the three shortcuts
that Emacs 23.2 does happen to show correctly.  The code defining
their menu items is the same as the code that defines the other
items whose shortcuts do not show.

The code that builds the menu is straightforward:
 
(defvar bookmarkp-jump-map nil "...")
(define-prefix-command 'bookmarkp-jump-map)
(define-key ctl-x-map "j" bookmarkp-jump-map)

(define-key bookmarkp-jump-map "B" 'bookmarkp-bookmark-list-jump)
(define-key bookmarkp-jump-map "v" 'bookmarkp-varlist-jump)
...
(define-key bookmarkp-bmenu-jump-menu ; SHORTCUT `C-x j v' OK
  [bookmarkp-varlist-jump]
  '(menu-item "Variable List..." bookmarkp-varlist-jump
  :help "..."))
(define-key bookmarkp-bmenu-jump-menu ; SHORTCUT `C-x j B' MISSING
  [bookmarkp-bookmark-list-jump]
  '(menu-item "Bookmark List..." bookmarkp-bookmark-list-jump
    :help "..."))
...

 
In GNU Emacs 24.0.50.1 (i386-mingw-nt5.1.2600)
 of 2010-05-23 on G41R2F1
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (3.4) --no-opt --cflags -Ic:/xpm/include'
 

[-- Attachment #2: bug-emacs-23-2-menu-shortcuts.png --]
[-- Type: image/png, Size: 10001 bytes --]

[-- Attachment #3: bug-emacs-22-menu-shortcuts.png --]
[-- Type: image/png, Size: 12053 bytes --]

[-- Attachment #4: bug-emacs-23-1-menu-shortcuts.png --]
[-- Type: image/png, Size: 11130 bytes --]

bug#6305: 24.0.50; Regression: most, but not all, keyboard shortcuts are missing

[Lennart Borgman]

On Sat, May 29, 2010 at 11:38 PM, Drew Adams <drew.adams@oracle.com> wrote:
> This is a regression from 23.1 to 23.2.
>
> I don't have a recipe from emacs -Q.  I'm sure you could create one.
>
> In Emacs 22 and 23.1, keyboard shortcuts ("equivalents", if you
> prefer) are indicated correctly in menus.  In both Emacs 23.2 and
> the recent build noted below, this is horribly broken.
>
> See the three attached screenshots, for Emacs 22 (OK), Emacs 23.1
> (OK), and Emacs 23.2 (broken - build noted below or the release 23.2).
>
> There is absolutely nothing particular about the three shortcuts
> that Emacs 23.2 does happen to show correctly.  The code defining
> their menu items is the same as the code that defines the other
> items whose shortcuts do not show.
>
> The code that builds the menu is straightforward:
>
> (defvar bookmarkp-jump-map nil "...")
> (define-prefix-command 'bookmarkp-jump-map)


I have also noticed that key bindings are not always shown in the
menus, but I wondered if that was because I was using
overriding-terminal-local-map there. I havn't had time to check,
however key bindings in global map and normal minor mode maps seems to
show up fine in the menus.

bug#6305: 24.0.50; Regression: most, but not all, keyboard shortcuts are missing

[Drew Adams]

> I have also noticed that key bindings are not always shown in the
> menus, but I wondered if that was because I was using
> overriding-terminal-local-map there. I havn't had time to check,
> however key bindings in global map and normal minor mode maps seems to
> show up fine in the menus.

I am not using `overriding-terminal-local-map'.

---

Note that a similar bug was reported as #6171.  That was also a regression, but
that problem started with 23.1.  Stefan reported that he fixed that bug after
23.1.

The current bug is a problem that does not show up in 23.1.  Dunno whether the
fix of #6171 might have introduced this bug.  Just mentioning #6171 in case it
helps somehow.

bug#6305: 24.0.50; Regression: most, but not all, keyboard shortcuts are missing

[Lennart Borgman]

On Sun, May 30, 2010 at 12:18 AM, Drew Adams <drew.adams@oracle.com> wrote:
>> I have also noticed that key bindings are not always shown in the
>> menus, but I wondered if that was because I was using
>> overriding-terminal-local-map there. I havn't had time to check,
>> however key bindings in global map and normal minor mode maps seems to
>> show up fine in the menus.
>
> I am not using `overriding-terminal-local-map'.


OK. Then it is at least partly another bug.

In the C code it looks up key bindings like this:

  (where-is-internal 'ido-kill-buffer nil t nil nil)

However it seems like key remapping is forgotten there.
where-is-internal does not handle remapping.

Could someone who knows remapping please look at that?

bug#6305: 24.0.50; Regression: most, but not all, keyboard shortcuts are missing

[Lennart Borgman]

On Sun, May 30, 2010 at 3:45 AM, Lennart Borgman
<lennart.borgman@gmail.com> wrote:
> On Sun, May 30, 2010 at 12:18 AM, Drew Adams <drew.adams@oracle.com> wrote:
>>> I have also noticed that key bindings are not always shown in the
>>> menus, but I wondered if that was because I was using
>>> overriding-terminal-local-map there. I havn't had time to check,
>>> however key bindings in global map and normal minor mode maps seems to
>>> show up fine in the menus.
>>
>> I am not using `overriding-terminal-local-map'.
>
>
> OK. Then it is at least partly another bug.
>
> In the C code it looks up key bindings like this:
>
>  (where-is-internal 'ido-kill-buffer nil t nil nil)
>
> However it seems like key remapping is forgotten there.
> where-is-internal does not handle remapping.
>
> Could someone who knows remapping please look at that?


Maybe I should say that this happens in parse_menu_item in keyboard.c.

bug#6305: 24.0.50; Regression: most, but not all, keyboard shortcuts are missing

[Stefan Monnier]

>> This is a regression from 23.1 to 23.2.
>> I don't have a recipe from emacs -Q.  I'm sure you could create one.

That would be *very* helpful.

>> In Emacs 22 and 23.1, keyboard shortcuts ("equivalents", if you
>> prefer) are indicated correctly in menus.  In both Emacs 23.2 and
>> the recent build noted below, this is horribly broken.

That's bad.

> I have also noticed that key bindings are not always shown in the
> menus, but I wondered if that was because I was using
> overriding-terminal-local-map there. I havn't had time to check,
> however key bindings in global map and normal minor mode maps seems to
> show up fine in the menus.

I don't know where to start looking without a reproducible recipe.
The fix for 6171 only touched tmm.el so it can't affect GUI menus.

> In the C code it looks up key bindings like this:
>   (where-is-internal 'ido-kill-buffer nil t nil nil)
> However it seems like key remapping is forgotten there.

In which sense?

> where-is-internal does not handle remapping.

There are many different ways to "handle" remapping.  where-is-internal
most definitely has code to do something specific with remapping.

> Could someone who knows remapping please look at that?

Look at what?


        Stefan

bug#6305: 24.0.50; Regression: most, but not all, keyboard shortcuts are missing

[Lennart Borgman]

On Mon, May 31, 2010 at 5:31 PM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
>
>> I have also noticed that key bindings are not always shown in the
>> menus, but I wondered if that was because I was using
>> overriding-terminal-local-map there. I havn't had time to check,
>> however key bindings in global map and normal minor mode maps seems to
>> show up fine in the menus.
>
> I don't know where to start looking without a reproducible recipe.
> The fix for 6171 only touched tmm.el so it can't affect GUI menus.


I think the problem is this:

- Fwhere_is_internal calls Fcurrent_active_maps, but the latter does
not include overriding-terminal-local-map if the first arg is Qnil. It
is Qnil in this case so overriding-terminal-local-map is not
considered.

However using Qt for the first arg is no good either since then only
the overriding map(s) are considered. Obviously Fcurrent_active_maps
behaves this way for the key lookup in the command loop.

Perhaps there should be something like Freally_active_maps ()?


>> In the C code it looks up key bindings like this:
>>   (where-is-internal 'ido-kill-buffer nil t nil nil)
>> However it seems like key remapping is forgotten there.
>
> In which sense?


What was I thinking, I really should learn to be more explicit ... ;-)

Yes, of course it looks at remapping, but for menus it would be good
if this situation was handled too:

- Let us say `kill-buffer' is in the menus.
- It got remapped to `ido-kill-buffer'.

The code in parse_menu_item around line 8008 calls

	  keys = Fwhere_is_internal (def, Qnil, Qt, Qnil, Qnil);

Since the last arg is nil it does not find any binding for
`kill-buffer' AFAICS. Or do I miss something?

bug#6305: 24.0.50; Regression: most, but not all, keyboard shortcuts are missing

[Drew Adams]

This thread seems to be getting diverted.
This is not particularly about REMAPPED keys.

The missing shortcuts in the screenshots I sent are NOT for REMAPPED
commands/keys. If you need to access the source code that produces those menus
it is here: http://www.emacswiki.org/emacs/bookmark%2b.el.

Lennart seems to be referring to only a PART of the problem this bug report is
about - or perhaps even to a separate problem altogether.

The problem I reported is more general. If you want to file a separate bug
report for a remapping problem, please do so. If you think there is a relation
between the two problems, you can cross-reference this bug report.

But this thread is not about REMAPPED keys in particular.

bug#6305: 24.0.50; Regression: most, but not all, keyboard shortcuts are missing

[Lennart Borgman]

On Mon, May 31, 2010 at 7:33 PM, Drew Adams <drew.adams@oracle.com> wrote:
> This thread seems to be getting diverted.
> This is not particularly about REMAPPED keys.
>
> The missing shortcuts in the screenshots I sent are NOT for REMAPPED
> commands/keys. If you need to access the source code that produces those menus
> it is here: http://www.emacswiki.org/emacs/bookmark%2b.el.
>
> Lennart seems to be referring to only a PART of the problem this bug report is
> about - or perhaps even to a separate problem altogether.
>
> The problem I reported is more general. If you want to file a separate bug
> report for a remapping problem, please do so. If you think there is a relation
> between the two problems, you can cross-reference this bug report.
>
> But this thread is not about REMAPPED keys in particular.


I see, but I think I have said all that is needed so it seems a bit
overkill to open a new bug report.

bug#6305: 24.0.50; Regression: most, but not all, keyboard shortcuts are missing

[Stefan Monnier]

> The missing shortcuts in the screenshots I sent are NOT for REMAPPED
> commands/keys. If you need to access the source code that produces
> those menus it is here: http://www.emacswiki.org/emacs/bookmark%2b.el.

What do I do with this file?  Again: Please provide a recipe.

> Lennart seems to be referring to only a PART of the problem this bug
> report is about - or perhaps even to a separate problem altogether.

I think he's talking about two problems, both of which are unrelated
to yours.


        Stefan

bug#6305: 24.0.50; Regression: most, but not all, keyboard shortcuts are missing

[Drew Adams]

> What do I do with this file?  Again: Please provide a recipe.

My bad altogether.  I will close this bug.

I was seeing this in my setup because of a minor-mode keymap override.
Turning off the minor mode brings back the shortcuts.

Very sorry for the noise.

bug#6305: 24.0.50; Regression: most, but not all, keyboard shortcuts are missing

[Lennart Borgman]

On Mon, May 31, 2010 at 8:23 PM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
>> The missing shortcuts in the screenshots I sent are NOT for REMAPPED
>> commands/keys. If you need to access the source code that produces
>> those menus it is here: http://www.emacswiki.org/emacs/bookmark%2b.el.
>
> What do I do with this file?  Again: Please provide a recipe.
>
>> Lennart seems to be referring to only a PART of the problem this bug
>> report is about - or perhaps even to a separate problem altogether.
>
> I think he's talking about two problems, both of which are unrelated
> to yours.

Yes, it seems so. Can we then continue here (since Drew says he has no
problem with the original bug here)?

bug#6305: 24.0.50; Regression: most, but not all, keyboard shortcuts are missing

[Drew Adams]

> > I think he's talking about two problems, both of which are unrelated
> > to yours.
> 
> Yes, it seems so. Can we then continue here (since Drew says he has no
> problem with the original bug here)?

I'm sorry, I closed the bug.  But you can reopen it I imagine.

BTW, the missing bindings I reported were in fact due to remappings (second mea
culpa).  In the minor-mode map, I remap those commands, so in the minor-mode map
they should show up correctly but they are missing altogether.

bug#6305: 24.0.50; Regression: most, but not all, keyboard shortcuts are missing

[Drew Adams]

> I'm sorry, I closed the bug.  But you can reopen it I imagine.

You're in luck.  The close failed because I forgot to include the bug number.
So this is still open.

bug#6305: 24.0.50; Regression: most, but not all, keyboard shortcuts are missing

[Drew Adams]

Maybe see also bug #788. Dunno whether it's related, since this worked in Emacs
23.1 (i.e. after the temporary disabling done to "fix" #788). Probably
unrelated, but who knows?

bug#6305: 24.0.50; Regression: most, but not all, keyboard shortcuts are missing

[Stefan Monnier]

> Maybe see also bug #788. Dunno whether it's related, since this worked
> in Emacs 23.1 (i.e. after the temporary disabling done to "fix" #788).
> Probably unrelated, but who knows?

AFAIK 788 is now fixed "right" with my latest change.


        Stefan