Re: [andrew.maguire@ps.ge.com: RE: menu entries missing display of equivalent keyboard sequence]

Re: [andrew.maguire@ps.ge.com: RE: menu entries missing display of equivalent keyboard sequence]

[Stefan Monnier]

> The problem below is caused by the where_is_cache, which I believe you
> added.

Actually, the problem is not caused by this at all.  It's caused by another
problem: the way `nomenus' was implemented in Emacs-20 was that any binding
that had a "menu-item" shape (either old-style or new-style) was considered
as a menu binding and was thus not listed in the shortcuts.
I wanted to change that so that "Edit => Text Properties => Face => Bold"
shows the "M-g b" shortcut (even though the M-g prefix uses the HierarKey
thingy to display a "menu in the minibuffer", which relies on menu-items
as well, which is why the M-g b shortcut is missing in Emacs-20).

So now, the problem is "what is a menu binding" ?
In Emacs-21, I defined it as "a key sequence whose first key is
a symbol".  Clearly, it's pretty arbitrary as well, since as is seen
in Andrew's example, it rules out "f8 d", but it was chosen because
it correctly rules out the menu-bar bindings, the tool-bar bindings,
and the FOO-down-mouse-N popup menus bindings.

The offending lines are in keymap.c:

      /* if (nomenus && !ascii_sequence_p (this)) */
      if (nomenus && XINT (last) >= 0
	  && !INTEGERP (Faref (this, make_number (0))))
	/* If no menu entries should be returned, skip over the
	   keymaps bound to `menu-bar' and `tool-bar' and other
	   non-ascii prefixes like `C-down-mouse-2'.  */
	continue;


-- Stefan


> I think the simplest fix is to get rid of that cache.  As far as I can
> see, it is active only in the case of lookup for menu items equivalent
> key bindings, and it's unnecessary in that case because the menu
> itself caches the equivalent key bindings.
> 
> Is there some case where where_is_cache really provides service?
> If so, would you like to fix this bug?
> 
> Please respond.
> 
> ------- Start of forwarded message -------
> From: andrew.maguire@ps.ge.com
> To: bug-gnu-emacs@gnu.org
> Date: Fri, 20 Dec 2002 12:29:40 -0500
> cc: andrew.maguire@ps.ge.com
> Subject: RE: menu entries missing display of equivalent keyboard sequence 
> Sender: bug-gnu-emacs-bounces+rms=gnu.org@gnu.org
> 
> Here is some example lisp that works in 20.7 but not in 21.2
> 
> (defun andrew-fn ()
> "Test"
> (interactive)
> (message "Hello"))
> 
> 
> (progn
>   (setq andrew-map (make-sparse-keymap "Andrew"))
>   (put 'andrew-fn 'menu-enable t)
>   ;(define-key andrew-map "F" nil)
>   (define-key andrew-map [andrew-function] '("My function" . andrew-fn))
>   (define-key global-map [menu-bar andrew] andrew-map)
>   (define-key-after (lookup-key global-map [menu-bar]) [andrew]
>     (cons "Andrew" andrew-map) 'dev)
>   (setq f8-map (make-sparse-keymap))
>   (fset 'f8-map f8-map)
>   (define-key f8-map "d" 'andrew-fn)
>   (define-key global-map [f8] 'f8-map)
> )
> 
> > -----Original Message-----
> > From: Andrew Maguire [mailto:Andrew.Maguire@Smallworld.co.uk]
> > Sent: 20 December 2002 15:42
> > To: bug-gnu-emacs@gnu.org; Maguire, Andrew (PS, GENS)
> > Subject: menu entries missing display of equivalent keyboard sequence 
> > 
> > 
> > This bug report will be sent to the Free Software Foundation,
> > not to your local site managers!
> > Please write in English, because the Emacs maintainers do not have
> > translators to read other languages for them.
> > 
> > Your bug report will be posted to the bug-gnu-emacs@gnu.org 
> > mailing list,
> > and to the gnu.emacs.bug news group.
> > 
> > In GNU Emacs 21.2.1 (sparc-sun-solaris2.6, X toolkit)
> >  of 2002-07-11 on albany
> > configured using `configure  --libexecdir=/usr/local/lib'
> > Important settings:
> >   value of $LC_ALL: nil
> >   value of $LC_COLLATE: en_GB.ISO8859-15
> >   value of $LC_CTYPE: en_GB.ISO8859-15
> >   value of $LC_MESSAGES: C
> >   value of $LC_MONETARY: en_GB.ISO8859-15
> >   value of $LC_NUMERIC: en_GB.ISO8859-15
> >   value of $LC_TIME: en_GB.ISO8859-15
> >   value of $LANG: en_GB.ISO8859-15
> >   locale-coding-system: iso-latin-9
> >   default-enable-multibyte-characters: t
> > 
> > Please describe exactly what actions triggered the bug
> > and the precise symptoms of the bug:
> > 
> > The code I have defines various global keys, creates a couple of
> > menu-bar general entries and some major mode specific ones
> > but the menu entries do not show their equivalent keyboard shortcuts.
> > (Apart from one that is defined on f1.) I am just using the simple
> > method for defining menu entries i.e. I am not using (menu-item ...).
> > 
> > The problem alsoo occurs on the Windows build 21.2.1 on buffy.
> > The menu entries for the standard menus is there, Fil, Edit, Help etc.
> > It just seems that the is something up with the mechanism that
> > calculates the shortcut keys and the caching in the keymap definition.
> > 
> > My code is fine on Emacs 20.7.
> > 
> > Can I force a recalculation of the caching of keysequences?
> > 
> > Thanks in advance,
> > 
> > Andrew Maguire
> > 
> > Recent input:
> > <down> <return> <help-echo> <down-mouse-2> <help-echo> 
> > <mouse-2> <down-mouse-1> <mouse-1> <next> <next> <next> 
> > <prior> <prior> <C-home> <next> <next> <next> <next> 
> > <next> <next> <next> C-s c o n s C-s C-s C-s C-s C-s 
> > C-s C-s C-s C-s C-s C-s C-s C-s C-s C-s C-s <down> 
> > <next> C-s C-s C-s C-s C-s C-s C-s C-s C-s C-s C-s 
> > C-s C-s C-s C-s C-s C-s C-s C-s C-s C-s C-s <down> 
> > <down> <down> <down> <down> <down> <down> <down> <down> 
> > <down> <help-echo> <down-mouse-1> <mouse-1> C-h C-g 
> > C-x b * i n <tab> <return> <prior> <help-echo> <help-echo> 
> > <help-echo> <help-echo> <help-echo> <help-echo> <help-echo> 
> > <help-echo> <menu-bar> <help-menu> <report-emacs-b
> > ug>
> > 
> > Recent messages:
> > setq: Quit [2 times]
> > Starting gis version selection...
> > 
> > Type C-x 1 to remove help window.  C-M-v to scroll the help.
> > call-interactively: Quit
> > list: Quit
> > sw-half-scroll-down: Beginning of buffer
> > Mark set
> > Mark saved where search started [2 times]
> > Loading emacsbug...done
> > 
> 
> 
> _______________________________________________
> Bug-gnu-emacs mailing list
> Bug-gnu-emacs@gnu.org
> http://mail.gnu.org/mailman/listinfo/bug-gnu-emacs
> ------- End of forwarded message -------
> 

Re: [andrew.maguire@ps.ge.com: RE: menu entries missing display of equivalent keyboard sequence]

[Richard Stallman]

    So now, the problem is "what is a menu binding" ?

Perhaps "any binding that starts with menu-bar or tool-bar" is the
right criterion.  If you want to catch pop-up menu bindings, you could
add "any binding that starts with a mouse-event symbol and uses it as
a prefix."  That is not guaranteed to be correct, but practically
speaking nobody uses mouse-events as prefixes except for pop-up menus.

Another possibility is "menu-item shape and attached to a symbol".
That way, M-g b would not be considered a menu-binding, but everything
else would still work as in version 20.

Re: [andrew.maguire@ps.ge.com: RE: menu entries missing display of equivalent keyboard sequence]

[Stefan Monnier]

>     So now, the problem is "what is a menu binding" ?
> 
> Perhaps "any binding that starts with menu-bar or tool-bar" is the
> right criterion.  If you want to catch pop-up menu bindings, you could
> add "any binding that starts with a mouse-event symbol and uses it as
> a prefix."  That is not guaranteed to be correct, but practically
> speaking nobody uses mouse-events as prefixes except for pop-up menus.

How about the patch below ?


	Stefan


Index: keymap.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/keymap.c,v
retrieving revision 1.270
diff -u -u -b -r1.270 keymap.c
--- keymap.c	6 Jan 2003 00:53:47 -0000	1.270
+++ keymap.c	28 Jan 2003 22:44:43 -0000
@@ -2236,6 +2368,8 @@
   return Qnil;
 }
 
+static Lisp_Object Vmenu_events;
+
 /* This function can GC if Flookup_key autoloads any keymaps.  */
 
 static Lisp_Object
@@ -2274,7 +2408,7 @@
   for (; !NILP (maps); maps = Fcdr (maps))
     {
       /* Key sequence to reach map, and the map that it reaches */
-      register Lisp_Object this, map;
+      register Lisp_Object this, map, tem;
 
       /* In order to fold [META-PREFIX-CHAR CHAR] sequences into
 	 [M-CHAR] sequences, check if last character of the sequence
@@ -2290,7 +2424,8 @@
 
       /* if (nomenus && !ascii_sequence_p (this)) */
       if (nomenus && XINT (last) >= 0
-	  && !INTEGERP (Faref (this, make_number (0))))
+	  && SYMBOLP (tem = Faref (this, make_number (0)))
+	  && !NILP (Fmemq (XCAR (parse_modifiers (tem)), Vmenu_events)))
 	/* If no menu entries should be returned, skip over the
 	   keymaps bound to `menu-bar' and `tool-bar' and other
 	   non-ascii prefixes like `C-down-mouse-2'.  */
@@ -3623,6 +3759,14 @@
 and applies even for keys that have ordinary bindings.  */);
   Vkey_translation_map = Qnil;
 
+  staticpro (&Vmenu_events);
+  Vmenu_events = Fcons (intern ("menu-bar"),
+			Fcons (intern ("tool-bar"),
+			       Fcons (intern ("mouse-1"),
+				      Fcons (intern ("mouse-2"),
+					     Fcons (intern ("mouse-3"),
+						    Qnil)))));
+
   Qsingle_key_description = intern ("single-key-description");
   staticpro (&Qsingle_key_description);

Re: [andrew.maguire@ps.ge.com: RE: menu entries missing display of equivalent keyboard sequence]

[Richard Stallman]

    -	  && !INTEGERP (Faref (this, make_number (0))))
    +	  && SYMBOLP (tem = Faref (this, make_number (0)))
    +	  && !NILP (Fmemq (XCAR (parse_modifiers (tem)), Vmenu_events)))

parse_modifiers is extremely expensive--it allocates space in caches.
Doing this for a lot of event types unnecessarily is a bad idea.

How many different non-integer prefix keys does parse_modifiers get
called for here?  Maybe it is few enough that this is not a real
issue, but could you check?

If it is indeed few enough, please add a comment saying how many it was
and explaining this issue.

If it is many, then I think the code should operate at a lower level.

Re: [andrew.maguire@ps.ge.com: RE: menu entries missing display of equivalent keyboard sequence]

[Stefan Monnier]

>     -	  && !INTEGERP (Faref (this, make_number (0))))
>     +	  && SYMBOLP (tem = Faref (this, make_number (0)))
>     +	  && !NILP (Fmemq (XCAR (parse_modifiers (tem)), Vmenu_events)))
> 
> parse_modifiers is extremely expensive--it allocates space in caches.
> Doing this for a lot of event types unnecessarily is a bad idea.

My quick experiment indicates that the number of symbols whose cache
is filled is increased by a bit more than 10% (from 618 to 689
in my test case, which was rather conservative).

The experiment was to start Emacs, open an elisp buffer, a TeX buffer,
a PCL-CVS buffer and a *vc-diff* buffer, open up a menu in each one of
them (to cause the `where-is' thingy to happen) and then count the
number of symbols that have the event-symbol-element-mask property set.

> How many different non-integer prefix keys does parse_modifiers get
> called for here?  Maybe it is few enough that this is not a real
> issue, but could you check?

I don't know if 70 is few enough.


	Stefan

Re: [andrew.maguire@ps.ge.com: RE: menu entries missing display of equivalent keyboard sequence]

[Richard Stallman]

    > parse_modifiers is extremely expensive--it allocates space in caches.
    > Doing this for a lot of event types unnecessarily is a bad idea.

    My quick experiment indicates that the number of symbols whose cache
    is filled is increased by a bit more than 10% (from 618 to 689
    in my test case, which was rather conservative).

I am surprised--could you see what symbols these are?