Date: Mon, 11 Jul 2011 20:04:43 -0400
From: Austin Clements <amdragon@MIT.EDU>
To: Louis Rilling <l.rilling@av7.net>
Subject: Re: [PATCH v2] tags_to_maildir_flags: Don't rename if no flags change
Message-ID: <20110712000443.GE25558@mit.edu>
References: <20110711223805.GC3583@localdomain>
	<1310424115-18850-1-git-send-email-l.rilling@av7.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1310424115-18850-1-git-send-email-l.rilling@av7.net>
User-Agent: Mutt/1.5.20 (2009-06-14)
Cc: notmuch@notmuchmail.org
Precedence: list

LGTM.

Quoth Louis Rilling on Jul 12 at 12:41 am:
> notmuch_message_tags_to_maildir_flags() unconditionally moves messages from
> maildir directory "new/" to maildir directory "cur/", which makes messages lose
> their "new" status in the MUA. However some users want to keep this "new"
> status after, for instance, an auto-tagging of new messages.
> 
> However, as Austin mentioned and according to the maildir specification,
> messages living in "new/" are not allowed to have flags, even if mutt allows it
> to happen. For this reason, this patch prevents moving messages from "new/" to
> "cur/", only if no flags have to be changed. It's hopefully enough to satisfy
> mutt (and maybe other MUAs showing the "new" status) users checking the "new"
> status.
> 
> Changelog:
> * v2: Fix bool type as well as NULL returned despite having no errors (Austin
>       Clements)
> 
> Signed-off-by: Louis Rilling <l.rilling@av7.net>
> ---
>  lib/message.cc |   12 +++++++++++-
>  1 files changed, 11 insertions(+), 1 deletions(-)
> 
> diff --git a/lib/message.cc b/lib/message.cc
> index 64b6cf8..3f8c4ba 100644
> --- a/lib/message.cc
> +++ b/lib/message.cc
> @@ -1139,7 +1139,7 @@ _get_maildir_flag_actions (notmuch_message_t *message,
>   * compute the new maildir filename.
>   *
>   * If the existing filename is in the directory "new", the new
> - * filename will be in the directory "cur".
> + * filename will be in the directory "cur", unless no flags are changed.
>   *
>   * After a sequence of ":2," in the filename, any subsequent
>   * single-character flags will be added or removed according to the
> @@ -1162,6 +1162,7 @@ _new_maildir_filename (void *ctx,
>      char *filename_new, *dir;
>      char flag_map[128];
>      int flags_in_map = 0;
> +    notmuch_bool_t flags_changed = FALSE;
>      unsigned int i;
>      char *s;
>  
> @@ -1202,6 +1203,7 @@ _new_maildir_filename (void *ctx,
>  	if (flag_map[flag] == 0) {
>  	    flag_map[flag] = 1;
>  	    flags_in_map++;
> +	    flags_changed = TRUE;
>  	}
>      }
>  
> @@ -1210,9 +1212,17 @@ _new_maildir_filename (void *ctx,
>  	if (flag_map[flag]) {
>  	    flag_map[flag] = 0;
>  	    flags_in_map--;
> +	    flags_changed = TRUE;
>  	}
>      }
>  
> +    /* No need to rename. Messages in new/ can be kept in new/.
> +     * Note: We don't even try to fix buggy messages having flags and living in
> +     * new/. It's not our business.
> +     */
> +    if (!flags_changed)
> +	return talloc_strdup (ctx, filename);
> +
>      filename_new = (char *) talloc_size (ctx,
>  					 info - filename +
>  					 strlen (":2,") + flags_in_map + 1);