bug#9898: 24.0.90; doc of :key-sequence

bug#9898: 24.0.90; doc of :key-sequence

[Drew Adams]

(elisp) Extended Menu Items
 
"`:key-sequence nil'
 This property indicates that there is normally no key binding
 which is equivalent to this menu item.  Using this property saves
 time in preparing the menu for display, because Emacs does not
 need to search the keymaps for a keyboard equivalent for this menu
 item.
 
 However, if the user has rebound this item's definition to a key
 sequence, Emacs ignores the `:keys' property and finds the keyboard
 equivalent anyway."
 
Should that `:keys' be `:key-sequence'?  Since when do we override :keys
with whatever the command is bound to?
 

In GNU Emacs 24.0.90.1 (i386-mingw-nt5.1.2600) of 2011-10-24 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#9898: 24.0.90; doc of :key-sequence

[Lars Ingebrigtsen]

"Drew Adams" <drew.adams@oracle.com> writes:

> (elisp) Extended Menu Items
>
> "`:key-sequence nil'
>  This property indicates that there is normally no key binding
>  which is equivalent to this menu item.  Using this property saves
>  time in preparing the menu for display, because Emacs does not
>  need to search the keymaps for a keyboard equivalent for this menu
>  item.
>
>  However, if the user has rebound this item's definition to a key
>  sequence, Emacs ignores the `:keys' property and finds the keyboard
>  equivalent anyway."
>
> Should that `:keys' be `:key-sequence'?  Since when do we override :keys
> with whatever the command is bound to?

Looking at the code, it looks to me that the documentation is wrong
here.  But I might be wrong.  The function in question that uses this
stuff is `easy-menu-convert-item-1'.  Somebody should take a look and
decide.

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

bug#9898: 24.0.90; doc of :key-sequence

[npostavs]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> "Drew Adams" <drew.adams@oracle.com> writes:
>
>> (elisp) Extended Menu Items
>>
>> "`:key-sequence nil'
>>  This property indicates that there is normally no key binding
>>  which is equivalent to this menu item.  Using this property saves
>>  time in preparing the menu for display, because Emacs does not
>>  need to search the keymaps for a keyboard equivalent for this menu
>>  item.
>>
>>  However, if the user has rebound this item's definition to a key
>>  sequence, Emacs ignores the `:keys' property and finds the keyboard
>>  equivalent anyway."
>>
>> Should that `:keys' be `:key-sequence'?  Since when do we override :keys
>> with whatever the command is bound to?
>
> Looking at the code, it looks to me that the documentation is wrong
> here.  But I might be wrong.  The function in question that uses this
> stuff is `easy-menu-convert-item-1'.  Somebody should take a look and
> decide.

I think the relevant code is in `parse_menu_item' of keyboard.c,
`easy-menu-convert-item-1' is only constructing menu items, not
interpreting them.

As far as I can tell, `:key-sequence nil' doesn't actually do anything
different from just not specifying `:key-sequence' at all (with the
minor exception that passing both `:keys STRING' and `:key-sequence nil'
causes the `:keys STRING' to be ignored).  So I propose just removing
both `:key-sequence nil' paragraphs.

parse_menu_item (Lisp_Object item, int inmenubar)
{
  Lisp_Object def, tem, item_string, start;
  Lisp_Object filter;
  Lisp_Object keyhint;
  int i;

  filter = Qnil;
  keyhint = Qnil;
  [...]
          /* Parse properties.  */
          while (CONSP (item) && CONSP (XCDR (item)))
            {
              tem = XCAR (item);
              item = XCDR (item);
        [...]
              else if (EQ (tem, QCkey_sequence))
                {
                  tem = XCAR (item);
                  if (SYMBOLP (tem) || STRINGP (tem) || VECTORP (tem))
                    /* Be GC protected. Set keyhint to item instead of tem.  */
                    keyhint = item;
                }
        [...]
    /* The previous code preferred :key-sequence to :keys, so we
       preserve this behavior.  */
    if (STRINGP (keyeq) && !CONSP (keyhint))
      keyeq = concat2 (space_space, Fsubstitute_command_keys (keyeq));
    else
      {
      [...]
        if (CONSP (keyhint) && !NILP (XCAR (keyhint)))
          {
            keys = XCAR (keyhint);
            [...]
          }

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

bug#9898: 24.0.90; doc of :key-sequence

[Lars Ingebrigtsen]

npostavs@users.sourceforge.net writes:

> As far as I can tell, `:key-sequence nil' doesn't actually do anything
> different from just not specifying `:key-sequence' at all (with the
> minor exception that passing both `:keys STRING' and `:key-sequence nil'
> causes the `:keys STRING' to be ignored).  So I propose just removing
> both `:key-sequence nil' paragraphs.

Looks like Stefan did something along those lines last year:

commit 5734339f4017836a2b171071056a320899a7c8e5
Author:     Stefan Monnier <monnier@iro.umontreal.ca>
AuthorDate: Sun Apr 19 11:19:03 2020 -0400

    * doc/lispref/keymaps.texi (Extended Menu Items, Easy Menu) <:key-sequence>:
    
    Clarify the documentation further

So I'm closing this bug report.

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