<<< Date Index >>>     <<< Thread Index >>>

Patch for "tag-prefix and macros" bug

Hi,

I'm trying to send the herewith attached mail to the bug database, but
apparently, the SMTP server for bugs.guug.de is down. So I'm sending
it directly here.

What do you think of it?

-- 
Lionel
--- Begin Message --- 
tags 1385 +patch
thanks

I have at last found the time and energy to correct the bug I reported
ages ago. As bug #1562 (duplicate) says more precisely, the problem is
that the tag-prefix acts only on the first command of a macro, which
in my case was a no-op. The problem comes from the fact that mutt
macros are like C macros: textual replacement. So "<tag-prefix><F2>",
if F2 is bound to ab, expands to "<tag-prefix>ab". The "<tag-prefix>"
applies only to the *next* function, and here you are.

To solve this, I have introduced "block tag prefix", i.e. a
<tag-prefix> that stays into effect until an end-of-block marker is
met. These blocks nest. I called the new commands <tag-prefix-begin>
and <tag-prefix-end>. For example, in
"<tag-prefix-begin>ab<tag-prefix-end>", both a and b will act on
tagged messages.

Then, if a macro-bound-key is pressed under scope of a <tag-prefix>,
its expansion is automatically put between a <tag-prefix-begin> and a
<tag-prefix-end>, thereby achieving the expected effect.

My patch also "corrects" another thing I judged was a bug: When the
auto_tag option is set ";t" will not do what is expected, namely untag
all tagged messages, like with auto_tag unset. "tttt" still does the
right thing: tag four messages. The code "simply" keeps track of the
reason why a tag prefix is in effect: Explicit user request or
automatic through auto_tag.

By reading the code, I found out about <tag-prefix-cond> and
<end-cond>. These don't seem to be documented anywhere (except the
ChangeLog). The behaviour wasn't consistent: In
"<tag-prefix-cond>ab<end-cond>c", if there are messages tagged, only a
operates on them and b and c operate on the current message, but this
sequence is equivalent to "c" when there are no messages
tagged. Additionally, I feel that it will be very hard, if possible,
to do this kind of thing properly (with nesting, macro expansion, etc)
in the current Mutt framework: Macros are not parseable without
knowledge of arity of the commands. E.g., on
"<tag-prefix-cond><pipe-entry>foo '<end-cond>'\n", the "<end-cond>"
has to be skipped, because it is part of the argument of
<pipe-entry>. To do this properly, one would probably need a global
flag "do_actions" and have _every_ operation check this flag before
acting, but after having consumed its eventual argument(s). Hence, I
have completely removed the tag-prefix-cond and end-cond. As it never
was documented, and it was broken anyway it isn't like we are dropping
documented compatibility or features.


Documentation: My patch adds none. Will write some if you decide to
integrate patch, or variant thereof. The same for the ChangeLog.

I use "tag-" in the status line to indicate a one-time tag prefix
(<tag-prefix>) and "Tag-" to indicate we are in scope of block tag
prefix.

-- 
Lionel
diff --recursive -u mutt-1.5.5.1/OPS mutt-1.5.5.1.lio/OPS
--- mutt-1.5.5.1/OPS    Wed Nov  5 10:41:31 2003
+++ mutt-1.5.5.1.lio/OPS        Sun Feb  1 22:59:48 2004
@@ -1,5 +1,4 @@
 OP_NULL "null operation"
-OP_END_COND "end of conditional execution (noop)"
 OP_ATTACH_VIEW_MAILCAP "force viewing of attachment using mailcap"
 OP_ATTACH_VIEW_TEXT "view attachment as text"
 OP_ATTACH_COLLAPSE "Toggle display of subparts"
@@ -157,7 +156,8 @@
 OP_SORT_REVERSE "sort messages in reverse order"
 OP_TAG "tag the current entry"
 OP_TAG_PREFIX "apply next function to tagged messages"
-OP_TAG_PREFIX_COND "apply next function ONLY to tagged messages"
+OP_TAG_PREFIX_BEGIN "apply a sequence of functions to tagged messages"
+OP_TAG_PREFIX_END "end of tag-prefix-begin scope"
 OP_TAG_SUBTHREAD "tag the current subthread"
 OP_TAG_THREAD "tag the current thread"
 OP_TOGGLE_NEW "toggle a message's 'new' flag"
diff --recursive -u mutt-1.5.5.1/browser.c mutt-1.5.5.1.lio/browser.c
--- mutt-1.5.5.1/browser.c      Wed Nov  5 10:41:31 2003
+++ mutt-1.5.5.1.lio/browser.c  Sun Feb  1 12:12:33 2004
@@ -492,6 +492,7 @@
     menu->top = 0;
 
   menu->tagged = 0;
+  menu->tagprefix_depth = 0;
   
   if (buffy)
     snprintf (title, titlelen, _("Mailboxes [%d]"), mutt_buffy_check (0));
diff --recursive -u mutt-1.5.5.1/commands.c mutt-1.5.5.1.lio/commands.c
--- mutt-1.5.5.1/commands.c     Wed Nov  5 10:41:31 2003
+++ mutt-1.5.5.1.lio/commands.c Sun Feb  1 16:53:01 2004
@@ -214,7 +214,7 @@
     if (r != -1 && option (OPTPROMPTAFTER))
     {
       mutt_ungetch (mutt_any_key_to_continue _("Command: "), 0);
-      rc = km_dokey (MENU_PAGER);
+      rc = km_dokey (MENU_PAGER, 0);
     }
     else
       rc = 0;
diff --recursive -u mutt-1.5.5.1/curs_main.c mutt-1.5.5.1.lio/curs_main.c
--- mutt-1.5.5.1/curs_main.c    Wed Nov  5 10:41:31 2003
+++ mutt-1.5.5.1.lio/curs_main.c        Sun Feb  1 22:56:17 2004
@@ -43,6 +43,7 @@
 #include <string.h>
 #include <sys/stat.h>
 #include <errno.h>
+#include <assert.h>
 
 static const char *No_mailbox_is_open = N_("No mailbox is open.");
 static const char *There_are_no_messages = N_("There are no messages.");
@@ -414,7 +415,8 @@
   int op = OP_NULL;
   int done = 0;                /* controls when to exit the "event" loop */
   int i = 0, j;
-  int tag = 0;                 /* has the tag-prefix command been pressed? */
+  int tag = TAG_PREFIX_NONE;   /* Is a tag prefix in action? */
+  int tag_depth = 0;           /* Number of block tag prefixes in action */
   int newcount = -1;
   int oldcount = -1;
   int rc = -1;
@@ -439,8 +441,6 @@
 
   FOREVER
   {
-    tag = 0; /* clear the tag-prefix */
-
     menu->max = Context ? Context->vcount : 0;
     oldcount = Context ? Context->msgcount : 0;
 
@@ -576,7 +576,22 @@
        move (menu->current - menu->top + menu->offset, COLS - 1);
       mutt_refresh ();
 
-      op = km_dokey (MENU_MAIN);
+      assert(tag_depth >= 0);
+
+      /* clear the tag prefix if not in a tag prefix block */
+      if (tag_depth > 0)
+      {
+       tag = TAG_PREFIX_BLOCK;
+       /* give visual indication that the next command is a tag- command */
+       mvaddstr (LINES - 1, 0, "Tag-");
+       clrtoeol ();
+      }
+      else if (option (OPTAUTOTAG) && Context && Context->tagged)
+       tag = TAG_PREFIX_AUTO;
+      else
+       tag = TAG_PREFIX_NONE;
+
+      op = km_dokey (MENU_MAIN, tag);
 
       dprint(4, (debugfile, "mutt_index_menu[%d]: Got op %d\n", __LINE__, op));
 
@@ -617,24 +632,22 @@
          mutt_error _("No tagged messages.");
          continue;
        }
-       tag = 1;
+       tag = TAG_PREFIX_SINGLE;
 
        /* give visual indication that the next command is a tag- command */
        mvaddstr (LINES - 1, 0, "tag-");
        clrtoeol ();
 
        /* get the real command */
-       if ((op = km_dokey (MENU_MAIN)) == OP_TAG_PREFIX)
+       if ((op = km_dokey (MENU_MAIN, tag)) == OP_TAG_PREFIX)
        {
          /* abort tag sequence */
          CLEARLINE (LINES-1);
          continue;
        }
       }
-      else if (option (OPTAUTOTAG) && Context && Context->tagged)
-       tag = 1;
 
-      if (op == OP_TAG_PREFIX_COND)
+      if (op == OP_TAG_PREFIX_BEGIN)
       {
        if (!Context)
        {
@@ -644,30 +657,14 @@
 
        if (!Context->tagged)
        {
-         event_t tmp;
-         while(UngetCount>0)
-         {
-           tmp=mutt_getch();
-           if(tmp.op==OP_END_COND)break;
-         }
-         mutt_message  _("Nothing to do.");
+         mutt_error _("No tagged messages.");
          continue;
        }
-       tag = 1;
 
-       /* give visual indication that the next command is a tag- command */
-       mvaddstr (LINES - 1, 0, "tag-");
-       clrtoeol ();
-
-       /* get the real command */
-       if ((op = km_dokey (MENU_MAIN)) == OP_TAG_PREFIX)
-       {
-         /* abort tag sequence */
-         CLEARLINE (LINES-1);
-         continue;
-       }
+       assert(tag_depth >= 0);
+       ++tag_depth;
+       continue;
       }
-
       mutt_clear_error ();
     }
     else
@@ -921,7 +918,7 @@
 
        CHECK_MSGCOUNT;
         CHECK_VISIBLE;
-       if (tag && !option (OPTAUTOTAG))
+       if (tag >= TAG_PREFIX_EXPLICIT)
        {
          for (j = 0; j < Context->vcount; j++)
            mutt_set_flag (Context, Context->hdrs[Context->v2r[j]], M_TAG, 0);
@@ -2064,7 +2061,10 @@
        menu->redraw = REDRAW_FULL;
        break;
 
-      case OP_END_COND:
+      case OP_TAG_PREFIX_END:
+       assert(tag_depth >= 0);
+       if (tag_depth > 0)
+         --tag_depth;
        break;
 
       case OP_WHAT_KEY:
diff --recursive -u mutt-1.5.5.1/enter.c mutt-1.5.5.1.lio/enter.c
--- mutt-1.5.5.1/enter.c        Wed Nov  5 10:41:31 2003
+++ mutt-1.5.5.1.lio/enter.c    Sun Feb  1 16:52:45 2004
@@ -256,7 +256,7 @@
     }
     mutt_refresh ();
 
-    if ((ch = km_dokey (MENU_EDITOR)) == -1)
+    if ((ch = km_dokey (MENU_EDITOR, 0)) == -1)
     {
       rv = -1; 
       goto bye;
diff --recursive -u mutt-1.5.5.1/functions.h mutt-1.5.5.1.lio/functions.h
--- mutt-1.5.5.1/functions.h    Wed Nov  5 10:41:31 2003
+++ mutt-1.5.5.1.lio/functions.h        Sun Feb  1 22:59:05 2004
@@ -51,8 +51,8 @@
   { "half-down",       OP_HALF_DOWN,           "]" },
   { "help",            OP_HELP,                "?" },
   { "tag-prefix",      OP_TAG_PREFIX,          ";" },
-  { "tag-prefix-cond", OP_TAG_PREFIX_COND,     NULL },
-  { "end-cond",                OP_END_COND,            NULL },
+  { "tag-prefix-begin",        OP_TAG_PREFIX_BEGIN,    NULL },
+  { "tag-prefix-end",  OP_TAG_PREFIX_END,      NULL },
   { "shell-escape",    OP_SHELL_ESCAPE,        "!" },
   { "select-entry",    OP_GENERIC_SELECT_ENTRY,M_ENTER_S },
   { "search",          OP_SEARCH,              "/" },
diff --recursive -u mutt-1.5.5.1/keymap.c mutt-1.5.5.1.lio/keymap.c
--- mutt-1.5.5.1/keymap.c       Wed Nov  5 10:41:32 2003
+++ mutt-1.5.5.1.lio/keymap.c   Sun Feb  1 21:11:38 2004
@@ -337,7 +337,7 @@
   }
 }
 
-static int retry_generic (int menu, keycode_t *keys, int keyslen, int lastkey)
+static int retry_generic (int menu, int tag, keycode_t *keys, int keyslen, int 
lastkey)
 {
   if (menu != MENU_EDITOR && menu != MENU_GENERIC && menu != MENU_PAGER)
   {
@@ -345,7 +345,7 @@
       mutt_ungetch (lastkey, 0);
     for (; keyslen; keyslen--)
       mutt_ungetch (keys[keyslen - 1], 0);
-    return (km_dokey (MENU_GENERIC));
+    return (km_dokey (MENU_GENERIC, tag));
   }
   if (menu != MENU_EDITOR)
   {
@@ -360,7 +360,7 @@
  *     OP_NULL         no function bound to key sequence
  *     -1              error occured while reading input
  */
-int km_dokey (int menu)
+int km_dokey (int menu, int tag)
 {
   event_t tmp;
   struct keymap_t *map = Keymaps[menu];
@@ -369,7 +369,7 @@
   int i;
 
   if (!map)
-    return (retry_generic (menu, NULL, 0, 0));
+    return (retry_generic (menu, tag, NULL, 0, 0));
 
   FOREVER
   {
@@ -436,12 +436,12 @@
     while (LastKey > map->keys[pos])
     {
       if (pos > map->eq || !map->next)
-       return (retry_generic (menu, map->keys, pos, LastKey));
+       return (retry_generic (menu, tag, map->keys, pos, LastKey));
       map = map->next;
     }
 
     if (LastKey != map->keys[pos])
-      return (retry_generic (menu, map->keys, pos, LastKey));
+      return (retry_generic (menu, tag, map->keys, pos, LastKey));
 
     if (++pos == map->len)
     {
@@ -456,7 +456,11 @@
        return -1;
       }
 
+      if (tag >= TAG_PREFIX_SINGLE)
+       push_string("<tag-prefix-end>");
       push_string (map->macro);
+      if (tag >= TAG_PREFIX_SINGLE)
+       push_string("<tag-prefix-begin>");
       map = Keymaps[menu];
       pos = 0;
     }
@@ -663,7 +667,7 @@
 
   /* make sure the key is really the help key in this menu */
   push_string (buf);
-  if (km_dokey (menu) != OP_HELP)
+  if (km_dokey (menu, 0) != OP_HELP)
   {
     mutt_error _("Key is not bound.");
     return;
diff --recursive -u mutt-1.5.5.1/keymap.h mutt-1.5.5.1.lio/keymap.h
--- mutt-1.5.5.1/keymap.h       Wed Nov  5 10:41:32 2003
+++ mutt-1.5.5.1.lio/keymap.h   Sun Feb  1 16:56:53 2004
@@ -29,7 +29,7 @@
 
 void km_bind (char *, int, int, char *, char *);
 void km_bindkey (char *, int, int);
-int km_dokey (int);
+int km_dokey (int, int);
 
 /* entry in the keymap tree */
 struct keymap_t
diff --recursive -u mutt-1.5.5.1/menu.c mutt-1.5.5.1.lio/menu.c
--- mutt-1.5.5.1/menu.c Wed Nov  5 10:41:32 2003
+++ mutt-1.5.5.1.lio/menu.c     Sun Feb  1 22:57:56 2004
@@ -27,6 +27,7 @@
 
 #include <string.h>
 #include <stdlib.h>
+#include <assert.h>
 
 extern int Charset_is_utf8; /* FIXME: bad modularisation */
 
@@ -840,7 +841,6 @@
       return OP_NULL;
     }
     
-    
     mutt_curs_set (0);
 
 #ifdef USE_IMAP
@@ -862,39 +862,49 @@
     /* try to catch dialog keys before ops */
     if (menu->dialog && menu_dialog_dokey (menu, &i) == 0)
       return i;
-                   
-    i = km_dokey (menu->menu);
-    if (i == OP_TAG_PREFIX || i == OP_TAG_PREFIX_COND)
+
+    /* clear the tag prefix if not in a tag prefix block */
+    if (menu->tagprefix_depth > 0)
+    {
+      menu->tagprefix = TAG_PREFIX_BLOCK;
+      /* give visual indication that the next command is a tag- command */
+      mvaddstr (LINES - 1, 0, "Tag-");
+      clrtoeol ();
+    }
+    else if (menu->tagged && option (OPTAUTOTAG))
+      menu->tagprefix = TAG_PREFIX_AUTO;
+    else
+      menu->tagprefix = TAG_PREFIX_NONE;
+
+    i = km_dokey (menu->menu, menu->tagprefix);
+
+    if (i == OP_TAG_PREFIX || i == OP_TAG_PREFIX_BEGIN)
     {
       if (menu->tagged)
       {
-       mvaddstr (LINES - 1, 0, "Tag-");
-       clrtoeol ();
-       i = km_dokey (menu->menu);
-       menu->tagprefix = 1;
-       CLEARLINE (LINES - 1);
+       if (i == OP_TAG_PREFIX_BEGIN)
+       {
+         assert(menu->tagprefix_depth >= 0);
+         ++(menu->tagprefix_depth);
+         /* Loop on reading next command after eventual redraw */
+         i = -1;
+       }
+       else /* (i == OP_TAG_PREFIX) */
+       {
+         assert(i == OP_TAG_PREFIX);
+         menu->tagprefix = TAG_PREFIX_SINGLE;
+         mvaddstr (LINES - 1, 0, "tag-");
+         clrtoeol ();
+         i = km_dokey (menu->menu, menu->tagprefix);
+         CLEARLINE (LINES - 1);
+       }
       }
-      else if (i == OP_TAG_PREFIX)
+      else
       {
        mutt_error _("No tagged entries.");
        i = -1;
       }
-      else /* None tagged, OP_TAG_PREFIX_COND */
-      {
-       event_t tmp;
-       while(UngetCount>0)
-       {
-         tmp=mutt_getch();
-         if(tmp.op==OP_END_COND)break;
-       }
-       mutt_message _("Nothing to do.");
-       i = -1;
-      }
     }
-    else if (menu->tagged && option (OPTAUTOTAG))
-      menu->tagprefix = 1;
-    else
-      menu->tagprefix = 0;
 
     mutt_curs_set (1);
 
@@ -1005,7 +1015,7 @@
       case OP_TAG:
        if (menu->tag && !menu->dialog)
        {
-         if (menu->tagprefix && !option (OPTAUTOTAG))
+         if (menu->tagprefix >= TAG_PREFIX_EXPLICIT)
          {
            for (i = 0; i < menu->max; i++)
              menu->tagged += menu->tag (menu, i, 0);
@@ -1053,7 +1063,10 @@
        km_error_key (menu->menu);
        break;
 
-      case OP_END_COND:
+      case OP_TAG_PREFIX_END:
+       assert(menu->tagprefix_depth >= 0);
+       if (menu->tagprefix_depth > 0)
+         --(menu->tagprefix_depth);
        break;
 
       default:
diff --recursive -u mutt-1.5.5.1/mutt_menu.h mutt-1.5.5.1.lio/mutt_menu.h
--- mutt-1.5.5.1/mutt_menu.h    Wed Nov  5 10:41:32 2003
+++ mutt-1.5.5.1.lio/mutt_menu.h        Sun Feb  1 20:49:49 2004
@@ -34,6 +34,18 @@
 
 #define M_MODEFMT "-- Mutt: %s"
 
+/* The rest of the code relies on the order relation
+ * between these values. Respect it.
+ * It also relies on the fact that TAG_PREFIX_NONE
+ * is false and all other values are true.
+ */
+
+#define TAG_PREFIX_SINGLE 4
+#define TAG_PREFIX_BLOCK 3
+#define TAG_PREFIX_EXPLICIT 2
+#define TAG_PREFIX_AUTO 1
+#define TAG_PREFIX_NONE 0
+
 typedef struct menu_t
 {
   char *title;   /* the title of this menu */
@@ -45,7 +57,14 @@
   int menu;    /* menu definition for keymap entries. */
   int offset;  /* which screen row to start the index */
   int pagelen; /* number of entries per screen */
-  int tagprefix;
+  int tagprefix; /* Is a tag prefix in action.?
+                  * TAG_PREFIX_NONE: No
+                  * TAG_PREFIX_AUTO: Automatic tag prefix
+                  * TAG_PREFIX_EXPLICIT or higher: Explicit tag prefix
+                  * TAG_PREFIX_BLOCK: In scope of block tag prefix
+                  * TAG_PREFIX_SINGLE: tag-prefix is in action
+                  */
+  int tagprefix_depth; /* Number of block tag prefixes in scope */
 
   /* Setting dialog != NULL overrides normal menu behaviour. 
    * In dialog mode menubar is hidden and prompt keys are checked before
diff --recursive -u mutt-1.5.5.1/pager.c mutt-1.5.5.1.lio/pager.c
--- mutt-1.5.5.1/pager.c        Wed Nov  5 10:41:32 2003
+++ mutt-1.5.5.1.lio/pager.c    Sun Feb  1 16:52:52 2004
@@ -1746,7 +1746,7 @@
 
     move (statusoffset, COLS-1);
     mutt_refresh ();
-    ch = km_dokey (MENU_PAGER);
+    ch = km_dokey (MENU_PAGER, 0);
     if (ch != -1)
       mutt_clear_error ();
     mutt_curs_set (1);

--- End Message ---

Attachment: signature.asc
Description: Digital signature