From 285baf9a3491a59702ce55b4568e11568e3ce0fe Mon Sep 17 00:00:00 2001 From: Kevin McCarthy Date: Wed, 27 Sep 2017 13:45:36 -0700 Subject: [PATCH] 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.) --- imap/command.c | 14 +++++----- imap/imap_private.h | 2 +- imap/message.c | 64 ++++++++++++++++++++++++++++++++++++++------- 3 files changed, 63 insertions(+), 17 deletions(-) diff --git a/imap/command.c b/imap/command.c index 8529d9de..3c104681 100644 --- a/imap/command.c +++ b/imap/command.c @@ -652,6 +652,7 @@ static void cmd_parse_fetch (IMAP_DATA* idata, char* s) { unsigned int msn, uid; HEADER *h; + int server_changes = 0; dprint (3, (debugfile, "Handling FETCH\n")); @@ -687,13 +688,14 @@ static void cmd_parse_fetch (IMAP_DATA* idata, char* s) 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 --git a/imap/imap_private.h b/imap/imap_private.h index 0c28dfbb..8842add0 100644 --- a/imap/imap_private.h +++ b/imap/imap_private.h @@ -269,7 +269,7 @@ int imap_cmd_idle (IMAP_DATA* idata); 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 --git a/imap/message.c b/imap/message.c index 4c882165..28ff62b8 100644 --- a/imap/message.c +++ b/imap/message.c @@ -652,7 +652,7 @@ int imap_fetch_message (CONTEXT *ctx, MESSAGE *msg, int msgno) * 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 @@ void imap_free_header_data (IMAP_HEADER_DATA** 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 (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 @@ char* imap_set_flags (IMAP_DATA* idata, HEADER* h, char* s) 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; -- 2.40.0