Make cmd_parse_fetch() more precise about setting reopen/check flags.

Previously any FETCH with FLAGS would result in either
  idata->reopen |= IMAP_EXPUNGE_PENDING;
  -or-
  idata->check_status = IMAP_FLAGS_PENDING;
being set.

This is unnecessary in the case of responses to FLAGS.SILENT updates
sent by mutt (which seem to commonly happen now-a-days).

Change imap_set_flags() to compare the old server flags against the
new ones, and report when _those_ updates would/did result in a local
header flag change.  Only set one of the reopen/check_status flags in
the event of an actual change (or potential change if a local change
has been made to the header.)

diff --git a/imap/command.c b/imap/command.c
index 7db301e36151374fd6c90a1f8579a6a97c89668e..f0d967654b12766cb545fdcc127039fa6bed44a5 100644 (file)
--- a/imap/command.c
+++ b/imap/command.c
@@ -260,6 +260,7 @@ static void cmd_parse_fetch(struct ImapData *idata, char *s)
 {
   unsigned int msn, uid;
   struct Header *h = NULL;
+  int server_changes = 0;
 
   mutt_debug(3, "Handling FETCH\n");
 
@@ -295,13 +296,14 @@ static void cmd_parse_fetch(struct ImapData *idata, char *s)
 
     if (mutt_strncasecmp("FLAGS", s, 5) == 0)
     {
-      /* If server flags could conflict with mutt's flags, reopen the mailbox. */
-      if (h->changed)
-        idata->reopen |= IMAP_EXPUNGE_PENDING;
-      else
+      imap_set_flags(idata, h, s, &server_changes);
+      if (server_changes)
       {
-        imap_set_flags(idata, h, s);
-        idata->check_status = IMAP_FLAGS_PENDING;
+        /* If server flags could conflict with mutt's flags, reopen the mailbox. */
+        if (h->changed)
+          idata->reopen |= IMAP_EXPUNGE_PENDING;
+        else
+          idata->check_status = IMAP_FLAGS_PENDING;
       }
       return;
     }

diff --git a/imap/imap_private.h b/imap/imap_private.h
index 32e4c00d5ecbae7d0c0dd8602550513553827575..cf942de50ee8dabd1f001d4c22afa019669598a9 100644 (file)
--- a/imap/imap_private.h
+++ b/imap/imap_private.h
@@ -298,7 +298,7 @@ int imap_cmd_idle(struct ImapData *idata);
 void imap_add_keywords(char *s, struct Header *keywords, struct ListHead *mailbox_flags, size_t slen);
 void imap_free_header_data(struct ImapHeaderData **data);
 int imap_read_headers(struct ImapData *idata, unsigned int msn_begin, unsigned int msn_end);
-char *imap_set_flags(struct ImapData *idata, struct Header *h, char *s);
+char *imap_set_flags(struct ImapData *idata, struct Header *h, char *s, int *server_changes);
 int imap_cache_del(struct ImapData *idata, struct Header *h);
 int imap_cache_clean(struct ImapData *idata);
 

diff --git a/imap/message.c b/imap/message.c
index ee1be60d83656f1040ab76326b7b8fc819a161e8..8813793650ec8d5d5c10e6002b77f9ad356d12b0 100644 (file)
--- a/imap/message.c
+++ b/imap/message.c
@@ -970,7 +970,7 @@ int imap_fetch_message(struct Context *ctx, struct Message *msg, int msgno)
          * incrementally update flags later, this won't stop us syncing */
         else if ((mutt_strncasecmp("FLAGS", pc, 5) == 0) && !h->changed)
         {
-          if ((pc = imap_set_flags(idata, h, pc)) == NULL)
+          if ((pc = imap_set_flags(idata, h, pc, NULL)) == NULL)
             goto bail;
         }
       }
@@ -1428,22 +1428,61 @@ void imap_free_header_data(struct ImapHeaderData **data)
   }
 }
 
+/* Sets server_changes to 1 if a change to a flag is made, or in the
+ * case of local_changes, if a change to a flag _would_ have been
+ * made. */
+static void imap_set_changed_flag(struct Context *ctx, struct Header *h,
+                                  int local_changes, int *server_changes, int flag_name,
+                                  int old_hd_flag, int new_hd_flag, int h_flag)
+{
+  /* If there are local_changes, we only want to note if the server
+   * flags have changed, so we can set a reopen flag in
+   * cmd_parse_fetch().  We don't want to count a local modification
+   * to the header flag as a "change".
+   */
+  if ((old_hd_flag != new_hd_flag) || (!local_changes))
+  {
+    if (new_hd_flag != h_flag)
+    {
+      if (server_changes)
+        *server_changes = 1;
+
+      /* Local changes have priority */
+      if (!local_changes)
+        mutt_set_flag(ctx, h, flag_name, new_hd_flag);
+    }
+  }
+}
+
 /**
  * imap_set_flags - fill the message header according to the server flags
  *
  * Expects a flags line of the form "FLAGS (flag flag ...)"
  */
-char *imap_set_flags(struct ImapData *idata, struct Header *h, char *s)
+
+/* imap_set_flags: fill out the message header according to the flags from
+ * the server. Expects a flags line of the form "FLAGS (flag flag ...)"
+ *
+ * Sets server_changes to 1 if a change to a flag is made, or in the
+ * case of h->changed, if a change to a flag _would_ have been
+ * made. */
+char *imap_set_flags(struct ImapData *idata, struct Header *h, char *s, int *server_changes)
 {
   struct Context *ctx = idata->ctx;
   struct ImapHeader newh;
+  struct ImapHeaderData old_hd;
   struct ImapHeaderData *hd = NULL;
   bool readonly;
+  int local_changes;
+
+  local_changes = h->changed;
 
   memset(&newh, 0, sizeof(newh));
   hd = h->data;
   newh.data = hd;
 
+  memcpy(&old_hd, hd, sizeof(old_hd));
+
   mutt_debug(2, "imap_set_flags: parsing FLAGS\n");
   if ((s = msg_parse_flags(&newh, s)) == NULL)
     return NULL;
@@ -1455,16 +1494,24 @@ char *imap_set_flags(struct ImapData *idata, struct Header *h, char *s)
   readonly = ctx->readonly;
   ctx->readonly = false;
 
-  mutt_set_flag(ctx, h, MUTT_NEW, !(hd->read || hd->old));
-  mutt_set_flag(ctx, h, MUTT_OLD, hd->old);
-  mutt_set_flag(ctx, h, MUTT_READ, hd->read);
-  mutt_set_flag(ctx, h, MUTT_DELETE, hd->deleted);
-  mutt_set_flag(ctx, h, MUTT_FLAG, hd->flagged);
-  mutt_set_flag(ctx, h, MUTT_REPLIED, hd->replied);
+  /* This is redundant with the following two checks. Removing:
+   * mutt_set_flag (ctx, h, MUTT_NEW, !(hd->read || hd->old));
+   */
+  imap_set_changed_flag(ctx, h, local_changes, server_changes, MUTT_OLD,
+                        old_hd.old, hd->old, h->old);
+  imap_set_changed_flag(ctx, h, local_changes, server_changes, MUTT_READ,
+                        old_hd.read, hd->read, h->read);
+  imap_set_changed_flag(ctx, h, local_changes, server_changes, MUTT_DELETE,
+                        old_hd.deleted, hd->deleted, h->deleted);
+  imap_set_changed_flag(ctx, h, local_changes, server_changes, MUTT_FLAG,
+                        old_hd.flagged, hd->flagged, h->flagged);
+  imap_set_changed_flag(ctx, h, local_changes, server_changes, MUTT_REPLIED,
+                        old_hd.replied, hd->replied, h->replied);
 
   /* this message is now definitively *not* changed (mutt_set_flag
    * marks things changed as a side-effect) */
-  h->changed = false;
+  if (!local_changes)
+    h->changed = false;
   ctx->changed &= !readonly;
   ctx->readonly = readonly;