David Champion wrote:
> # HG changeset patch
> # User David Champion <[email protected]>
> # Date 1385592820 21600
> #      Wed Nov 27 16:53:40 2013 -0600
> # Node ID 44c56715ef8bb953bfda7a271a2c0544b55941cc
> # Parent  3306cb186f49e83edf15aac91c51f4c6131ef8fe
> add `unbind' and `unmacro' commands
> 
> This is a refresh and update of a very old (~2001) patch providing
> unbind and unmacro commands.

Overall this patch looks good.  I have a few small questions and
suggestions, but nothing that should hold up the patch.  +1.

It looks like, with this patch, the usages are now:
   bind    { map | * }  key  { function | noop }
   bind    { map | * }  *
   unbind  { map | * } { key | * }
   unmacro { map | * } { key | * }
with the latter three added by the patch.

I think it would be helpful if the "Usage" section for bind listed both
modes, and the unbind and unmacro "Usage" showed the * option for the
map.

Do we still need the "noop" binding with the patch?

A couple more comments below:

> diff -r 3306cb186f49 -r 44c56715ef8b doc/manual.xml.head
> --- a/doc/manual.xml.head     Tue Oct 29 00:11:16 2013 -0700
> +++ b/doc/manual.xml.head     Wed Nov 27 16:53:40 2013 -0600

> +<literal>enter-command</literal> prompt is actually usable, and an
> +<literal>exit</literal> bindind is added to the <literal>pager</literal>

typo: bindind -> binding

> diff -r 3306cb186f49 -r 44c56715ef8b keymap.c
> --- a/keymap.c        Tue Oct 29 00:11:16 2013 -0700
> +++ b/keymap.c        Wed Nov 27 16:53:40 2013 -0600

> +/* unbind menu-name '<key_sequence>' */
> +/* Never unbind *everything*.  Leave an escape hatch. */
> +int mutt_parse_unbind (BUFFER *buf, BUFFER *s, unsigned long data, BUFFER 
> *err)
> +{
> +  char *key;
> +  int menu[sizeof(Menus)/sizeof(struct mapping_t)-1], r = 0, nummenus, i;
> +
> +  if ((key = parse_keymap (menu, s, sizeof (menu)/sizeof (menu[0]),
> +                        &nummenus, err, TRUE)) == NULL)
> +    return (-1);
> +
> +  if (MoreArgs (s))
> +  {
> +    strfcpy (err->data, _("unbind: too many arguments"), err->dsize);
> +    FREE (&key);
> +    return (-1);
> +  }
> +
> +  if (mutt_strcmp(key, "*") == 0)
> +  {
> +    int neededitorbindings = 0;
> +
> +    /* Unbind all in selected menus */
> +    for (i = 0; i < nummenus; ++i)
> +    {
> +      if (menu[i] == -1)
> +     continue;
> +      dprint(1, (debugfile, "unbind %s %s\n", mutt_getnamebyvalue(menu[i], 
> Menus), key));
> +      km_unbind (&Keymaps[menu[i]], NULL, data);
> +
> +      /* If we unbound * in generic, then rebind <enter-command> */
> +      if ((menu[i] == MENU_GENERIC) && ((data & M_UNBIND) == M_UNBIND))
> +      {
> +     neededitorbindings++;
> +     km_bindkey (":", MENU_GENERIC, OP_ENTER_COMMAND);
> +      }
> +
> +      /* <enter-command> will need basic editor mappings.
> +       * If editor bindings were removed, remember that. */
> +      if (menu[i] == MENU_EDITOR)
> +     neededitorbindings++;
> +
> +      /* pager needs at least an exit */
> +      if (menu[i] == MENU_PAGER)
> +     km_bindkey ("q", MENU_PAGER, OP_EXIT);
> +    }
> +    if (neededitorbindings == 2)
> +      create_bindings (OpEditor, MENU_EDITOR);

Why do we only restore the bindings when needitorbindings == 2?
Should we restore them if we do "unbind editor *"?

> +  }
> +  else
> +  {
> +    /* Unbind designated key in each selected menu. */
> +    for (i = 0; i < nummenus; ++i)
> +    {
> +      if (menu[i] != -1)
> +      {
> +     dprint(1, (debugfile, "unbind %s %s\n", mutt_getnamebyvalue(menu[i], 
> Menus), key));
> +     km_unbind (&Keymaps[menu[i]], key, data);
> +      }
> +    }
> +  }
> +
> +  FREE (&key);
> +  return (r);
> +}

-Kevin

Attachment: signature.asc
Description: PGP signature

Reply via email to