[prev in list] [next in list] [prev in thread] [next in thread] 

List:       mutt-dev
Subject:    mutt: Make cmd_parse_fetch() more precise about setting reopen/c...
From:       Brendan Cully <brendan () kublai ! com>
Date:       2017-09-27 20:50:19
Message-ID: hg.e98ad5446640.1506545419.1928362535899276112 () dev ! cs ! ubc ! ca
[Download RAW message or body]

changeset: 7170:e98ad5446640
user:      Kevin McCarthy <kevin@8t8.us>
date:      Wed Sep 27 13:45:36 2017 -0700
link:      http://dev.mutt.org/hg/mutt/rev/e98ad5446640

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

diffs (146 lines):

diff -r cbe207b97a2a -r e98ad5446640 imap/command.c
--- a/imap/command.c	Tue Sep 26 19:45:23 2017 -0700
+++ b/imap/command.c	Wed Sep 27 13:45:36 2017 -0700
@@ -652,6 +652,7 @@
 {
   unsigned int msn, uid;
   HEADER *h;
+  int server_changes = 0;
 
   dprint (3, (debugfile, "Handling FETCH\n"));
 
@@ -687,13 +688,14 @@
 
     if (ascii_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 -r cbe207b97a2a -r e98ad5446640 imap/imap_private.h
--- a/imap/imap_private.h	Tue Sep 26 19:45:23 2017 -0700
+++ b/imap/imap_private.h	Wed Sep 27 13:45:36 2017 -0700
@@ -269,7 +269,7 @@
 void imap_add_keywords (char* s, HEADER* keywords, LIST* mailbox_flags, size_t slen);
 void imap_free_header_data (IMAP_HEADER_DATA** data);
 int imap_read_headers (IMAP_DATA* idata, unsigned int msn_begin, unsigned int msn_end);
-char* imap_set_flags (IMAP_DATA* idata, HEADER* h, char* s);
+char* imap_set_flags (IMAP_DATA* idata, HEADER* h, char* s, int *server_changes);
 int imap_cache_del (IMAP_DATA* idata, HEADER* h);
 int imap_cache_clean (IMAP_DATA* idata);
 
diff -r cbe207b97a2a -r e98ad5446640 imap/message.c
--- a/imap/message.c	Tue Sep 26 19:45:23 2017 -0700
+++ b/imap/message.c	Wed Sep 27 13:45:36 2017 -0700
@@ -652,7 +652,7 @@
 	 * incrementally update flags later, this won't stop us syncing */
 	else if ((ascii_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;
 	}
       }
@@ -1183,19 +1183,55 @@
   }
 }
 
+/* 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 (CONTEXT *ctx, 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 out the message header according to the flags from
- *   the server. Expects a flags line of the form "FLAGS (flag flag ...)" */
-char* imap_set_flags (IMAP_DATA* idata, HEADER* h, char* s)
+ * 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 (IMAP_DATA* idata, HEADER* h, char* s, int *server_changes)
 {
   CONTEXT* ctx = idata->ctx;
   IMAP_HEADER newh;
+  IMAP_HEADER_DATA old_hd;
   IMAP_HEADER_DATA* hd;
   unsigned char 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));
+
   dprint (2, (debugfile, "imap_set_flags: parsing FLAGS\n"));
   if ((s = msg_parse_flags (&newh, s)) == NULL)
     return NULL;
@@ -1207,16 +1243,24 @@
   readonly = ctx->readonly;
   ctx->readonly = 0;
 
-  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 = 0;
+  if (!local_changes)
+    h->changed = 0;
   ctx->changed &= ~readonly;
   ctx->readonly = readonly;
 
[prev in list] [next in list] [prev in thread] [next in thread]