From: Michal Sojka <sojkam1@fel.cvut.cz>
To: Stewart Smith <stewart@flamingspork.com>, Tim Stoakes
	<notmuch@stoakes.net>, notmuch@notmuchmail.org
In-Reply-To: <20100216022128.GH22402@flamingspork.com>
References: <87d43b2oif.fsf@yoom.home.cworth.org>
	<9bfdedddeab9c58cd45d8d448323d0fc@localhost>
	<87skc23327.fsf@yoom.home.cworth.org>
	<4b0eef22.JwxdgTGElffx149F%michiel@michielbuddingh.net>
	<87ws1bjpmm.fsf@yoom.home.cworth.org> <878wdfkhcl.fsf@aegir.org.uk>
	<20100210031339.GH16686@mail.rocksoft.com>
	<20100215081331.GD22402@flamingspork.com>
	<20100216015856.GG22402@flamingspork.com>
	<20100216021250.GB21780@lapse.rw.madduck.net>
	<20100216022128.GH22402@flamingspork.com>
Date: Tue, 16 Feb 2010 10:35:27 +0100
Message-ID: <87vddxikv4.fsf@steelpick.localdomain>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Subject: Re: [notmuch] [PATCH] notmuch: Respect maildir message flags
Precedence: list

On Tue, 16 Feb 2010 13:21:28 +1100, Stewart Smith <stewart@flamingspork.com> wrote:
> What about this patch (just with those few things fixed)?
> 
> diff --git a/notmuch-new.c b/notmuch-new.c
> index f25c71f..8303047 100644
> --- a/notmuch-new.c
> +++ b/notmuch-new.c
> @@ -39,6 +39,7 @@ typedef struct {
>      int total_files;
>      int processed_files;
>      int added_messages;
> +    int tag_maildir;

I think notmuch_bool_t will be better than int here.

BTW what is the reason for using notmuch_bool_t instead of bool from
stdbool.h?

> @@ -222,6 +277,7 @@ add_files_recursive (notmuch_database_t *notmuch,
>      notmuch_filenames_t *db_subdirs = NULL;
>      struct stat st;
>      notmuch_bool_t is_maildir, new_directory;
> +    int maildir_detected = -1;

Again, notmuch_bool_t is IMHO better. You seem only to use values -1 and
1 which is quite confusing.

>  
>      if (stat (path, &st)) {
>  	fprintf (stderr, "Error reading directory %s: %s\n",
> @@ -301,6 +357,28 @@ add_files_recursive (notmuch_database_t *notmuch,
>  	    continue;
>  	}
>  
> +	/* If this directory is a Maildir folder, we need to
> +	 * ignore any subdirectories marked tmp/, and scan for
> +	 * Maildir attributes on messages contained in the sub-
> +	 * directories 'new' and 'cur'. */
> +	if (maildir_detected != 0 &&
> +	    (entry->d_type == DT_DIR || entry->d_type == DT_UNKNOWN) &&
> +	    ((strcmp (entry->d_name, "tmp") == 0) ||
> +	     (strcmp (entry->d_name, "new") == 0) ||
> +	     (strcmp (entry->d_name, "cur") == 0))) {
> +
> +    if (maildir_detected == -1) {
> +      maildir_detected = _entries_resemble_maildir(fs_entries, num_fs_entries);
> +    }
> +    if (maildir_detected == 1) {
> +      if (strcmp (entry->d_name, "tmp") == 0) {
> +        continue;
> +      } else {
> +        state->tag_maildir = TRUE;

You might also want to set this to FALSE somewhere. It is very unlikely,
but somebody can create non-maildir under maildir.

> @@ -412,7 +490,12 @@ add_files_recursive (notmuch_database_t *notmuch,
>  	/* success */
>  	case NOTMUCH_STATUS_SUCCESS:
>  	    state->added_messages++;
> -	    tag_inbox_and_unread (message);
> +			if (state->tag_maildir) {
> +			    derive_tags_from_maildir_flags (message,
> +							    entry->d_name);
> +			} else {
> +			    tag_inbox_and_unread (message);
> +			}
>  	    break;
>  	/* Non-fatal issues (go on to next file) */
>  	case NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID:

You add the tags only to newly discovered mails. If a file is renamed
(e.g. because another mail reader removed the S flag), the tags will not
be updated. It is a question, what is the proper behavior. I personally
use something like what is in your patch and then use notmuchsync to
handle renamed files. It has only one problem - notmuchsync is quite
slow, so if this is solved in notmuch, I'd be happy.

Cheers,
 Michal