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);