* [PATCH 0/4] Synchronize maildir flags in new/
@ 2012-06-03 16:57 Austin Clements
2012-06-03 16:57 ` [PATCH 1/4] test: Add broken test for tag synchronization on files delivered to new/ Austin Clements
` (4 more replies)
0 siblings, 5 replies; 18+ messages in thread
From: Austin Clements @ 2012-06-03 16:57 UTC (permalink / raw)
To: notmuch
This problem came up again on IRC a few days ago, so I'm finally
posting this series (which I've been running locally for months).
The idea---which is explained in detail in the first patch---is that
messages in new/ are maildir messages with an empty set of maildir
flags. Currently we treat such messages as non-maildir messages,
which, in particular, means we don't automatically give them the
unread tag.
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 1/4] test: Add broken test for tag synchronization on files delivered to new/ 2012-06-03 16:57 [PATCH 0/4] Synchronize maildir flags in new/ Austin Clements @ 2012-06-03 16:57 ` Austin Clements 2012-06-04 7:10 ` Jani Nikula 2012-06-03 16:57 ` [PATCH 2/4] lib: Move _filename_is_in_maildir Austin Clements ` (3 subsequent siblings) 4 siblings, 1 reply; 18+ messages in thread From: Austin Clements @ 2012-06-03 16:57 UTC (permalink / raw) To: notmuch Currently, notmuch new only synchronizs maildir flags to tags for files that have an "info" part. However, in maildir, new mail doesn't gain the info part until it moves from new/ to cur/. Hence, even though mail in new/ doesn't have an info part, it is still a maildir message and thus has maildir flags (though none of them set). The most visible effect of not synchronizing maildir flags for messages in new/ is that newly delivered messages don't get the unread tag (unless it is assigned by some other mechanism, like new.tags). This patch does *not* modify the test for messages in cur/ that do not have an "info" part. Unlike a message in new/, a message in cur/ without an info part is no longer a maildir message, and thus shouldn't be subject to maildir flag synchronization. --- test/maildir-sync | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/maildir-sync b/test/maildir-sync index d72ec07..6360fd2 100755 --- a/test/maildir-sync +++ b/test/maildir-sync @@ -166,4 +166,14 @@ add_message [subject]='"Non-compliant maildir info"' [dir]=cur [filename]='non-c notmuch tag +unread +draft -flagged subject:"Non-compliant maildir info" test_expect_equal "$(cd $MAIL_DIR/cur/; ls non-compliant*)" "non-compliant-maildir-info:2,These-are-not-flags-in-ASCII-order-donottouch" +test_begin_subtest "Files in new/ get default synchronized tags" +test_subtest_known_broken +OLDCONFIG=$(notmuch config get new.tags) +notmuch config set new.tags test +add_message [subject]='"File in new/"' [dir]=new [filename]='file-in-new' +notmuch config set new.tags $OLDCONFIG +notmuch search 'subject:"File in new"' | notmuch_search_sanitize > output +test_expect_equal "$(< output)" \ +"thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; File in new/ (test unread)" + test_done -- 1.7.10 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] test: Add broken test for tag synchronization on files delivered to new/ 2012-06-03 16:57 ` [PATCH 1/4] test: Add broken test for tag synchronization on files delivered to new/ Austin Clements @ 2012-06-04 7:10 ` Jani Nikula 2012-06-09 19:02 ` Austin Clements 0 siblings, 1 reply; 18+ messages in thread From: Jani Nikula @ 2012-06-04 7:10 UTC (permalink / raw) To: Austin Clements, notmuch On Sun, 03 Jun 2012, Austin Clements <amdragon@MIT.EDU> wrote: > Currently, notmuch new only synchronizs maildir flags to tags for +e to synchronizs. > files that have an "info" part. However, in maildir, new mail doesn't > gain the info part until it moves from new/ to cur/. Hence, even > though mail in new/ doesn't have an info part, it is still a maildir > message and thus has maildir flags (though none of them set). > > The most visible effect of not synchronizing maildir flags for > messages in new/ is that newly delivered messages don't get the unread > tag (unless it is assigned by some other mechanism, like new.tags). > > This patch does *not* modify the test for messages in cur/ that do not > have an "info" part. Unlike a message in new/, a message in cur/ > without an info part is no longer a maildir message, and thus > shouldn't be subject to maildir flag synchronization. > --- > test/maildir-sync | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/test/maildir-sync b/test/maildir-sync > index d72ec07..6360fd2 100755 > --- a/test/maildir-sync > +++ b/test/maildir-sync > @@ -166,4 +166,14 @@ add_message [subject]='"Non-compliant maildir info"' [dir]=cur [filename]='non-c > notmuch tag +unread +draft -flagged subject:"Non-compliant maildir info" > test_expect_equal "$(cd $MAIL_DIR/cur/; ls non-compliant*)" "non-compliant-maildir-info:2,These-are-not-flags-in-ASCII-order-donottouch" > > +test_begin_subtest "Files in new/ get default synchronized tags" > +test_subtest_known_broken > +OLDCONFIG=$(notmuch config get new.tags) > +notmuch config set new.tags test > +add_message [subject]='"File in new/"' [dir]=new [filename]='file-in-new' > +notmuch config set new.tags $OLDCONFIG > +notmuch search 'subject:"File in new"' | notmuch_search_sanitize > output > +test_expect_equal "$(< output)" \ > +"thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; File in new/ (test unread)" > + > test_done > -- > 1.7.10 > > _______________________________________________ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] test: Add broken test for tag synchronization on files delivered to new/ 2012-06-04 7:10 ` Jani Nikula @ 2012-06-09 19:02 ` Austin Clements 0 siblings, 0 replies; 18+ messages in thread From: Austin Clements @ 2012-06-09 19:02 UTC (permalink / raw) To: Jani Nikula; +Cc: notmuch Quoth Jani Nikula on Jun 04 at 7:10 am: > On Sun, 03 Jun 2012, Austin Clements <amdragon@MIT.EDU> wrote: > > Currently, notmuch new only synchronizs maildir flags to tags for > > +e to synchronizs. Vowels are so web 1.0. Updated series on its way. ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/4] lib: Move _filename_is_in_maildir 2012-06-03 16:57 [PATCH 0/4] Synchronize maildir flags in new/ Austin Clements 2012-06-03 16:57 ` [PATCH 1/4] test: Add broken test for tag synchronization on files delivered to new/ Austin Clements @ 2012-06-03 16:57 ` Austin Clements 2012-06-03 16:57 ` [PATCH 3/4] lib: Only synchronize maildir flags for messages in maildirs Austin Clements ` (2 subsequent siblings) 4 siblings, 0 replies; 18+ messages in thread From: Austin Clements @ 2012-06-03 16:57 UTC (permalink / raw) To: notmuch This way notmuch_message_maildir_flags_to_tags can call it. It makes more sense for this to be just above all of the maildir synchronization code rather than mixed in the middle. --- lib/message.cc | 82 ++++++++++++++++++++++++++++---------------------------- 1 file changed, 41 insertions(+), 41 deletions(-) diff --git a/lib/message.cc b/lib/message.cc index 6787506..ed96477 100644 --- a/lib/message.cc +++ b/lib/message.cc @@ -1027,6 +1027,47 @@ notmuch_message_remove_tag (notmuch_message_t *message, const char *tag) return NOTMUCH_STATUS_SUCCESS; } +/* Is the given filename within a maildir directory? + * + * Specifically, is the final directory component of 'filename' either + * "cur" or "new". If so, return a pointer to that final directory + * component within 'filename'. If not, return NULL. + * + * A non-NULL return value is guaranteed to be a valid string pointer + * pointing to the characters "new/" or "cur/", (but not + * NUL-terminated). + */ +static const char * +_filename_is_in_maildir (const char *filename) +{ + const char *slash, *dir = NULL; + + /* Find the last '/' separating directory from filename. */ + slash = strrchr (filename, '/'); + if (slash == NULL) + return NULL; + + /* Jump back 4 characters to where the previous '/' will be if the + * directory is named "cur" or "new". */ + if (slash - filename < 4) + return NULL; + + slash -= 4; + + if (*slash != '/') + return NULL; + + dir = slash + 1; + + if (STRNCMP_LITERAL (dir, "cur/") == 0 || + STRNCMP_LITERAL (dir, "new/") == 0) + { + return dir; + } + + return NULL; +} + notmuch_status_t notmuch_message_maildir_flags_to_tags (notmuch_message_t *message) { @@ -1083,47 +1124,6 @@ notmuch_message_maildir_flags_to_tags (notmuch_message_t *message) return status; } -/* Is the given filename within a maildir directory? - * - * Specifically, is the final directory component of 'filename' either - * "cur" or "new". If so, return a pointer to that final directory - * component within 'filename'. If not, return NULL. - * - * A non-NULL return value is guaranteed to be a valid string pointer - * pointing to the characters "new/" or "cur/", (but not - * NUL-terminated). - */ -static const char * -_filename_is_in_maildir (const char *filename) -{ - const char *slash, *dir = NULL; - - /* Find the last '/' separating directory from filename. */ - slash = strrchr (filename, '/'); - if (slash == NULL) - return NULL; - - /* Jump back 4 characters to where the previous '/' will be if the - * directory is named "cur" or "new". */ - if (slash - filename < 4) - return NULL; - - slash -= 4; - - if (*slash != '/') - return NULL; - - dir = slash + 1; - - if (STRNCMP_LITERAL (dir, "cur/") == 0 || - STRNCMP_LITERAL (dir, "new/") == 0) - { - return dir; - } - - return NULL; -} - /* From the set of tags on 'message' and the flag2tag table, compute a * set of maildir-flag actions to be taken, (flags that should be * either set or cleared). -- 1.7.10 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/4] lib: Only synchronize maildir flags for messages in maildirs 2012-06-03 16:57 [PATCH 0/4] Synchronize maildir flags in new/ Austin Clements 2012-06-03 16:57 ` [PATCH 1/4] test: Add broken test for tag synchronization on files delivered to new/ Austin Clements 2012-06-03 16:57 ` [PATCH 2/4] lib: Move _filename_is_in_maildir Austin Clements @ 2012-06-03 16:57 ` Austin Clements 2012-06-04 7:01 ` Jani Nikula 2012-06-03 16:57 ` [PATCH 4/4] lib: Treat messages in new/ as maildir messages with no flags set Austin Clements 2012-06-09 19:14 ` [PATCH v2 0/5] Synchronize maildir flags in new/ Austin Clements 4 siblings, 1 reply; 18+ messages in thread From: Austin Clements @ 2012-06-03 16:57 UTC (permalink / raw) To: notmuch --- lib/message.cc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/message.cc b/lib/message.cc index ed96477..bbac2ff 100644 --- a/lib/message.cc +++ b/lib/message.cc @@ -1074,7 +1074,7 @@ notmuch_message_maildir_flags_to_tags (notmuch_message_t *message) const char *flags; notmuch_status_t status; notmuch_filenames_t *filenames; - const char *filename; + const char *filename, *dir; char *combined_flags = talloc_strdup (message, ""); unsigned i; int seen_maildir_info = 0; @@ -1084,6 +1084,10 @@ notmuch_message_maildir_flags_to_tags (notmuch_message_t *message) notmuch_filenames_move_to_next (filenames)) { filename = notmuch_filenames_get (filenames); + dir = _filename_is_in_maildir (filename); + + if (! dir) + continue; flags = strstr (filename, ":2,"); if (! flags) -- 1.7.10 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 3/4] lib: Only synchronize maildir flags for messages in maildirs 2012-06-03 16:57 ` [PATCH 3/4] lib: Only synchronize maildir flags for messages in maildirs Austin Clements @ 2012-06-04 7:01 ` Jani Nikula 0 siblings, 0 replies; 18+ messages in thread From: Jani Nikula @ 2012-06-04 7:01 UTC (permalink / raw) To: Austin Clements, notmuch The patch makes a lot of sense, and fixes the asymmetry introduced by commit 95dd5fe5. I think it deserves a more verbose commit message, along with that commit reference, and (eventually, not necessarily in this series) a NEWS item of its own. On Sun, 03 Jun 2012, Austin Clements <amdragon@MIT.EDU> wrote: > --- > lib/message.cc | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/lib/message.cc b/lib/message.cc > index ed96477..bbac2ff 100644 > --- a/lib/message.cc > +++ b/lib/message.cc > @@ -1074,7 +1074,7 @@ notmuch_message_maildir_flags_to_tags (notmuch_message_t *message) > const char *flags; > notmuch_status_t status; > notmuch_filenames_t *filenames; > - const char *filename; > + const char *filename, *dir; > char *combined_flags = talloc_strdup (message, ""); > unsigned i; > int seen_maildir_info = 0; > @@ -1084,6 +1084,10 @@ notmuch_message_maildir_flags_to_tags (notmuch_message_t *message) > notmuch_filenames_move_to_next (filenames)) > { > filename = notmuch_filenames_get (filenames); > + dir = _filename_is_in_maildir (filename); > + > + if (! dir) > + continue; > > flags = strstr (filename, ":2,"); > if (! flags) > -- > 1.7.10 > > _______________________________________________ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 4/4] lib: Treat messages in new/ as maildir messages with no flags set 2012-06-03 16:57 [PATCH 0/4] Synchronize maildir flags in new/ Austin Clements ` (2 preceding siblings ...) 2012-06-03 16:57 ` [PATCH 3/4] lib: Only synchronize maildir flags for messages in maildirs Austin Clements @ 2012-06-03 16:57 ` Austin Clements 2012-06-04 7:08 ` Jani Nikula 2012-06-09 19:14 ` [PATCH v2 0/5] Synchronize maildir flags in new/ Austin Clements 4 siblings, 1 reply; 18+ messages in thread From: Austin Clements @ 2012-06-03 16:57 UTC (permalink / raw) To: notmuch This fixes the broken test added a few patches ago by synchronizing the tags of messages in new/, even if they have no "info" part. See the test patch for rationale. --- lib/message.cc | 20 +++++++++++++------- test/maildir-sync | 1 - 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/lib/message.cc b/lib/message.cc index bbac2ff..978de06 100644 --- a/lib/message.cc +++ b/lib/message.cc @@ -1090,13 +1090,19 @@ notmuch_message_maildir_flags_to_tags (notmuch_message_t *message) continue; flags = strstr (filename, ":2,"); - if (! flags) - continue; - - seen_maildir_info = 1; - flags += 3; - - combined_flags = talloc_strdup_append (combined_flags, flags); + if (flags) { + seen_maildir_info = 1; + flags += 3; + combined_flags = talloc_strdup_append (combined_flags, flags); + } else if (STRNCMP_LITERAL (dir, "new/") == 0) { + /* Messages are delivered to new/ with no "info" part, but + * they effectively have default maildir flags. According + * to the spec, we should ignore the info part for + * messages in new/, but some MUAs (mutt) can set maildir + * flags on messages in new/, so we're liberal in what we + * accept. */ + seen_maildir_info = 1; + } } /* If none of the filenames have any maildir info field (not even diff --git a/test/maildir-sync b/test/maildir-sync index 6360fd2..01348d3 100755 --- a/test/maildir-sync +++ b/test/maildir-sync @@ -167,7 +167,6 @@ notmuch tag +unread +draft -flagged subject:"Non-compliant maildir info" test_expect_equal "$(cd $MAIL_DIR/cur/; ls non-compliant*)" "non-compliant-maildir-info:2,These-are-not-flags-in-ASCII-order-donottouch" test_begin_subtest "Files in new/ get default synchronized tags" -test_subtest_known_broken OLDCONFIG=$(notmuch config get new.tags) notmuch config set new.tags test add_message [subject]='"File in new/"' [dir]=new [filename]='file-in-new' -- 1.7.10 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 4/4] lib: Treat messages in new/ as maildir messages with no flags set 2012-06-03 16:57 ` [PATCH 4/4] lib: Treat messages in new/ as maildir messages with no flags set Austin Clements @ 2012-06-04 7:08 ` Jani Nikula 0 siblings, 0 replies; 18+ messages in thread From: Jani Nikula @ 2012-06-04 7:08 UTC (permalink / raw) To: Austin Clements, notmuch On Sun, 03 Jun 2012, Austin Clements <amdragon@MIT.EDU> wrote: > This fixes the broken test added a few patches ago by synchronizing > the tags of messages in new/, even if they have no "info" part. See > the test patch for rationale. There's no harm in copy-pasting some of the rationale here. It's easier to find later on. The series LGTM, modulo the commit message nitpicks. > --- > lib/message.cc | 20 +++++++++++++------- > test/maildir-sync | 1 - > 2 files changed, 13 insertions(+), 8 deletions(-) > > diff --git a/lib/message.cc b/lib/message.cc > index bbac2ff..978de06 100644 > --- a/lib/message.cc > +++ b/lib/message.cc > @@ -1090,13 +1090,19 @@ notmuch_message_maildir_flags_to_tags (notmuch_message_t *message) > continue; > > flags = strstr (filename, ":2,"); > - if (! flags) > - continue; > - > - seen_maildir_info = 1; > - flags += 3; > - > - combined_flags = talloc_strdup_append (combined_flags, flags); > + if (flags) { > + seen_maildir_info = 1; > + flags += 3; > + combined_flags = talloc_strdup_append (combined_flags, flags); > + } else if (STRNCMP_LITERAL (dir, "new/") == 0) { > + /* Messages are delivered to new/ with no "info" part, but > + * they effectively have default maildir flags. According > + * to the spec, we should ignore the info part for > + * messages in new/, but some MUAs (mutt) can set maildir > + * flags on messages in new/, so we're liberal in what we > + * accept. */ > + seen_maildir_info = 1; > + } > } > > /* If none of the filenames have any maildir info field (not even > diff --git a/test/maildir-sync b/test/maildir-sync > index 6360fd2..01348d3 100755 > --- a/test/maildir-sync > +++ b/test/maildir-sync > @@ -167,7 +167,6 @@ notmuch tag +unread +draft -flagged subject:"Non-compliant maildir info" > test_expect_equal "$(cd $MAIL_DIR/cur/; ls non-compliant*)" "non-compliant-maildir-info:2,These-are-not-flags-in-ASCII-order-donottouch" > > test_begin_subtest "Files in new/ get default synchronized tags" > -test_subtest_known_broken > OLDCONFIG=$(notmuch config get new.tags) > notmuch config set new.tags test > add_message [subject]='"File in new/"' [dir]=new [filename]='file-in-new' > -- > 1.7.10 > > _______________________________________________ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 0/5] Synchronize maildir flags in new/ 2012-06-03 16:57 [PATCH 0/4] Synchronize maildir flags in new/ Austin Clements ` (3 preceding siblings ...) 2012-06-03 16:57 ` [PATCH 4/4] lib: Treat messages in new/ as maildir messages with no flags set Austin Clements @ 2012-06-09 19:14 ` Austin Clements 2012-06-09 19:14 ` [PATCH v2 1/5] test: Add broken test for tag synchronization on files delivered to new/ Austin Clements ` (7 more replies) 4 siblings, 8 replies; 18+ messages in thread From: Austin Clements @ 2012-06-09 19:14 UTC (permalink / raw) To: notmuch v2 hopefully addresses Jani's comments about v1's commit messages. It also adds a news patch. ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 1/5] test: Add broken test for tag synchronization on files delivered to new/ 2012-06-09 19:14 ` [PATCH v2 0/5] Synchronize maildir flags in new/ Austin Clements @ 2012-06-09 19:14 ` Austin Clements 2012-06-09 19:14 ` [PATCH v2 2/5] lib: Move _filename_is_in_maildir Austin Clements ` (6 subsequent siblings) 7 siblings, 0 replies; 18+ messages in thread From: Austin Clements @ 2012-06-09 19:14 UTC (permalink / raw) To: notmuch Currently, notmuch new only synchronizes maildir flags to tags for files that have an "info" part. However, in maildir, new mail doesn't gain the info part until it moves from new/ to cur/. Hence, even though mail in new/ doesn't have an info part, it is still a maildir message and thus has maildir flags (though none of them set). The most visible effect of not synchronizing maildir flags for messages in new/ is that newly delivered messages don't get the unread tag (unless it is assigned by some other mechanism, like new.tags). This patch does *not* modify the test for messages in cur/ that do not have an "info" part. Unlike a message in new/, a message in cur/ without an info part is no longer a maildir message, and thus shouldn't be subject to maildir flag synchronization. --- test/maildir-sync | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/maildir-sync b/test/maildir-sync index d72ec07..6360fd2 100755 --- a/test/maildir-sync +++ b/test/maildir-sync @@ -166,4 +166,14 @@ add_message [subject]='"Non-compliant maildir info"' [dir]=cur [filename]='non-c notmuch tag +unread +draft -flagged subject:"Non-compliant maildir info" test_expect_equal "$(cd $MAIL_DIR/cur/; ls non-compliant*)" "non-compliant-maildir-info:2,These-are-not-flags-in-ASCII-order-donottouch" +test_begin_subtest "Files in new/ get default synchronized tags" +test_subtest_known_broken +OLDCONFIG=$(notmuch config get new.tags) +notmuch config set new.tags test +add_message [subject]='"File in new/"' [dir]=new [filename]='file-in-new' +notmuch config set new.tags $OLDCONFIG +notmuch search 'subject:"File in new"' | notmuch_search_sanitize > output +test_expect_equal "$(< output)" \ +"thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; File in new/ (test unread)" + test_done -- 1.7.10 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 2/5] lib: Move _filename_is_in_maildir 2012-06-09 19:14 ` [PATCH v2 0/5] Synchronize maildir flags in new/ Austin Clements 2012-06-09 19:14 ` [PATCH v2 1/5] test: Add broken test for tag synchronization on files delivered to new/ Austin Clements @ 2012-06-09 19:14 ` Austin Clements 2012-06-09 19:14 ` [PATCH v2 3/5] lib: Only synchronize maildir flags for messages in maildirs Austin Clements ` (5 subsequent siblings) 7 siblings, 0 replies; 18+ messages in thread From: Austin Clements @ 2012-06-09 19:14 UTC (permalink / raw) To: notmuch This way notmuch_message_maildir_flags_to_tags can call it. It makes more sense for this to be just above all of the maildir synchronization code rather than mixed in the middle. --- lib/message.cc | 82 ++++++++++++++++++++++++++++---------------------------- 1 file changed, 41 insertions(+), 41 deletions(-) diff --git a/lib/message.cc b/lib/message.cc index 6787506..ed96477 100644 --- a/lib/message.cc +++ b/lib/message.cc @@ -1027,6 +1027,47 @@ notmuch_message_remove_tag (notmuch_message_t *message, const char *tag) return NOTMUCH_STATUS_SUCCESS; } +/* Is the given filename within a maildir directory? + * + * Specifically, is the final directory component of 'filename' either + * "cur" or "new". If so, return a pointer to that final directory + * component within 'filename'. If not, return NULL. + * + * A non-NULL return value is guaranteed to be a valid string pointer + * pointing to the characters "new/" or "cur/", (but not + * NUL-terminated). + */ +static const char * +_filename_is_in_maildir (const char *filename) +{ + const char *slash, *dir = NULL; + + /* Find the last '/' separating directory from filename. */ + slash = strrchr (filename, '/'); + if (slash == NULL) + return NULL; + + /* Jump back 4 characters to where the previous '/' will be if the + * directory is named "cur" or "new". */ + if (slash - filename < 4) + return NULL; + + slash -= 4; + + if (*slash != '/') + return NULL; + + dir = slash + 1; + + if (STRNCMP_LITERAL (dir, "cur/") == 0 || + STRNCMP_LITERAL (dir, "new/") == 0) + { + return dir; + } + + return NULL; +} + notmuch_status_t notmuch_message_maildir_flags_to_tags (notmuch_message_t *message) { @@ -1083,47 +1124,6 @@ notmuch_message_maildir_flags_to_tags (notmuch_message_t *message) return status; } -/* Is the given filename within a maildir directory? - * - * Specifically, is the final directory component of 'filename' either - * "cur" or "new". If so, return a pointer to that final directory - * component within 'filename'. If not, return NULL. - * - * A non-NULL return value is guaranteed to be a valid string pointer - * pointing to the characters "new/" or "cur/", (but not - * NUL-terminated). - */ -static const char * -_filename_is_in_maildir (const char *filename) -{ - const char *slash, *dir = NULL; - - /* Find the last '/' separating directory from filename. */ - slash = strrchr (filename, '/'); - if (slash == NULL) - return NULL; - - /* Jump back 4 characters to where the previous '/' will be if the - * directory is named "cur" or "new". */ - if (slash - filename < 4) - return NULL; - - slash -= 4; - - if (*slash != '/') - return NULL; - - dir = slash + 1; - - if (STRNCMP_LITERAL (dir, "cur/") == 0 || - STRNCMP_LITERAL (dir, "new/") == 0) - { - return dir; - } - - return NULL; -} - /* From the set of tags on 'message' and the flag2tag table, compute a * set of maildir-flag actions to be taken, (flags that should be * either set or cleared). -- 1.7.10 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 3/5] lib: Only synchronize maildir flags for messages in maildirs 2012-06-09 19:14 ` [PATCH v2 0/5] Synchronize maildir flags in new/ Austin Clements 2012-06-09 19:14 ` [PATCH v2 1/5] test: Add broken test for tag synchronization on files delivered to new/ Austin Clements 2012-06-09 19:14 ` [PATCH v2 2/5] lib: Move _filename_is_in_maildir Austin Clements @ 2012-06-09 19:14 ` Austin Clements 2012-06-09 19:14 ` [PATCH v2 4/5] lib: Treat messages in new/ as maildir messages with no flags set Austin Clements ` (4 subsequent siblings) 7 siblings, 0 replies; 18+ messages in thread From: Austin Clements @ 2012-06-09 19:14 UTC (permalink / raw) To: notmuch Previously, we synchronized flags to tags for any message that looked like it had maildir flags in its file name, regardless of whether it was in a maildir-like directory structure. This was asymmetric with tag-to-flag synchronization, which only applied to messages in directories named new/ and cur/ (introduced by 95dd5fe5). This change makes our interpretation stricter and addresses this asymmetry by only synchronizing flags to tags for messages in directories named new/ or cur/. It also prepares us to treat messages in new/ as maildir messages, even though they lack maildir flags. --- lib/message.cc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/message.cc b/lib/message.cc index ed96477..bbac2ff 100644 --- a/lib/message.cc +++ b/lib/message.cc @@ -1074,7 +1074,7 @@ notmuch_message_maildir_flags_to_tags (notmuch_message_t *message) const char *flags; notmuch_status_t status; notmuch_filenames_t *filenames; - const char *filename; + const char *filename, *dir; char *combined_flags = talloc_strdup (message, ""); unsigned i; int seen_maildir_info = 0; @@ -1084,6 +1084,10 @@ notmuch_message_maildir_flags_to_tags (notmuch_message_t *message) notmuch_filenames_move_to_next (filenames)) { filename = notmuch_filenames_get (filenames); + dir = _filename_is_in_maildir (filename); + + if (! dir) + continue; flags = strstr (filename, ":2,"); if (! flags) -- 1.7.10 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 4/5] lib: Treat messages in new/ as maildir messages with no flags set 2012-06-09 19:14 ` [PATCH v2 0/5] Synchronize maildir flags in new/ Austin Clements ` (2 preceding siblings ...) 2012-06-09 19:14 ` [PATCH v2 3/5] lib: Only synchronize maildir flags for messages in maildirs Austin Clements @ 2012-06-09 19:14 ` Austin Clements 2012-06-09 19:14 ` [PATCH v2 5/5] News for updated maildir sync semantics Austin Clements ` (3 subsequent siblings) 7 siblings, 0 replies; 18+ messages in thread From: Austin Clements @ 2012-06-09 19:14 UTC (permalink / raw) To: notmuch Previously, notmuch new only synchronized maildir flags to tags for files with a maildir "info" part. Since messages in new/ don't have an info part, notmuch would ignore them for flag-to-tag synchronization. This patch makes notmuch consider messages in new/ to be legitimate maildir messages that simply have no maildir flags set. The most visible effect of this is that such messages now automatically get the unread tag. --- lib/message.cc | 20 +++++++++++++------- test/maildir-sync | 1 - 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/lib/message.cc b/lib/message.cc index bbac2ff..978de06 100644 --- a/lib/message.cc +++ b/lib/message.cc @@ -1090,13 +1090,19 @@ notmuch_message_maildir_flags_to_tags (notmuch_message_t *message) continue; flags = strstr (filename, ":2,"); - if (! flags) - continue; - - seen_maildir_info = 1; - flags += 3; - - combined_flags = talloc_strdup_append (combined_flags, flags); + if (flags) { + seen_maildir_info = 1; + flags += 3; + combined_flags = talloc_strdup_append (combined_flags, flags); + } else if (STRNCMP_LITERAL (dir, "new/") == 0) { + /* Messages are delivered to new/ with no "info" part, but + * they effectively have default maildir flags. According + * to the spec, we should ignore the info part for + * messages in new/, but some MUAs (mutt) can set maildir + * flags on messages in new/, so we're liberal in what we + * accept. */ + seen_maildir_info = 1; + } } /* If none of the filenames have any maildir info field (not even diff --git a/test/maildir-sync b/test/maildir-sync index 6360fd2..01348d3 100755 --- a/test/maildir-sync +++ b/test/maildir-sync @@ -167,7 +167,6 @@ notmuch tag +unread +draft -flagged subject:"Non-compliant maildir info" test_expect_equal "$(cd $MAIL_DIR/cur/; ls non-compliant*)" "non-compliant-maildir-info:2,These-are-not-flags-in-ASCII-order-donottouch" test_begin_subtest "Files in new/ get default synchronized tags" -test_subtest_known_broken OLDCONFIG=$(notmuch config get new.tags) notmuch config set new.tags test add_message [subject]='"File in new/"' [dir]=new [filename]='file-in-new' -- 1.7.10 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 5/5] News for updated maildir sync semantics 2012-06-09 19:14 ` [PATCH v2 0/5] Synchronize maildir flags in new/ Austin Clements ` (3 preceding siblings ...) 2012-06-09 19:14 ` [PATCH v2 4/5] lib: Treat messages in new/ as maildir messages with no flags set Austin Clements @ 2012-06-09 19:14 ` Austin Clements 2012-06-10 8:32 ` [PATCH v2 0/5] Synchronize maildir flags in new/ Tomi Ollila ` (2 subsequent siblings) 7 siblings, 0 replies; 18+ messages in thread From: Austin Clements @ 2012-06-09 19:14 UTC (permalink / raw) To: notmuch --- NEWS | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/NEWS b/NEWS index fb55efb..d29ec5b 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,19 @@ +Notmuch 0.14 (xxxx-xx-xx) +========================= + +General bug fixes +----------------- + +Maildir tag synchronization + + Maildir flag-to-tag synchronization now applies only to messages in + maildir-like directory structures. Previously, it applied to any + message that had a maildir "info" part, which meant it could + incorrectly synchronize tags for non-maildir messages, while at the + same time failing to synchronize tags for newly received maildir + messages (typically causing new messages to not receive the "unread" + tag). + Notmuch 0.13.2 (2012-06-02) =========================== -- 1.7.10 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2 0/5] Synchronize maildir flags in new/ 2012-06-09 19:14 ` [PATCH v2 0/5] Synchronize maildir flags in new/ Austin Clements ` (4 preceding siblings ...) 2012-06-09 19:14 ` [PATCH v2 5/5] News for updated maildir sync semantics Austin Clements @ 2012-06-10 8:32 ` Tomi Ollila 2012-06-10 18:06 ` Jani Nikula 2012-06-10 23:20 ` David Bremner 7 siblings, 0 replies; 18+ messages in thread From: Tomi Ollila @ 2012-06-10 8:32 UTC (permalink / raw) To: Austin Clements, notmuch On Sat, Jun 09 2012, Austin Clements <amdragon@MIT.EDU> wrote: > v2 hopefully addresses Jani's comments about v1's commit messages. It > also adds a news patch. LGTM. Tomi ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 0/5] Synchronize maildir flags in new/ 2012-06-09 19:14 ` [PATCH v2 0/5] Synchronize maildir flags in new/ Austin Clements ` (5 preceding siblings ...) 2012-06-10 8:32 ` [PATCH v2 0/5] Synchronize maildir flags in new/ Tomi Ollila @ 2012-06-10 18:06 ` Jani Nikula 2012-06-10 23:20 ` David Bremner 7 siblings, 0 replies; 18+ messages in thread From: Jani Nikula @ 2012-06-10 18:06 UTC (permalink / raw) To: Austin Clements, notmuch On Sat, 09 Jun 2012, Austin Clements <amdragon@MIT.EDU> wrote: > v2 hopefully addresses Jani's comments about v1's commit messages. It > also adds a news patch. Thanks, looks good, Jani. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 0/5] Synchronize maildir flags in new/ 2012-06-09 19:14 ` [PATCH v2 0/5] Synchronize maildir flags in new/ Austin Clements ` (6 preceding siblings ...) 2012-06-10 18:06 ` Jani Nikula @ 2012-06-10 23:20 ` David Bremner 7 siblings, 0 replies; 18+ messages in thread From: David Bremner @ 2012-06-10 23:20 UTC (permalink / raw) To: Austin Clements, notmuch Austin Clements <amdragon@MIT.EDU> writes: > v2 hopefully addresses Jani's comments about v1's commit messages. It > also adds a news patch. Pushed. d ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2012-06-10 23:20 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-06-03 16:57 [PATCH 0/4] Synchronize maildir flags in new/ Austin Clements 2012-06-03 16:57 ` [PATCH 1/4] test: Add broken test for tag synchronization on files delivered to new/ Austin Clements 2012-06-04 7:10 ` Jani Nikula 2012-06-09 19:02 ` Austin Clements 2012-06-03 16:57 ` [PATCH 2/4] lib: Move _filename_is_in_maildir Austin Clements 2012-06-03 16:57 ` [PATCH 3/4] lib: Only synchronize maildir flags for messages in maildirs Austin Clements 2012-06-04 7:01 ` Jani Nikula 2012-06-03 16:57 ` [PATCH 4/4] lib: Treat messages in new/ as maildir messages with no flags set Austin Clements 2012-06-04 7:08 ` Jani Nikula 2012-06-09 19:14 ` [PATCH v2 0/5] Synchronize maildir flags in new/ Austin Clements 2012-06-09 19:14 ` [PATCH v2 1/5] test: Add broken test for tag synchronization on files delivered to new/ Austin Clements 2012-06-09 19:14 ` [PATCH v2 2/5] lib: Move _filename_is_in_maildir Austin Clements 2012-06-09 19:14 ` [PATCH v2 3/5] lib: Only synchronize maildir flags for messages in maildirs Austin Clements 2012-06-09 19:14 ` [PATCH v2 4/5] lib: Treat messages in new/ as maildir messages with no flags set Austin Clements 2012-06-09 19:14 ` [PATCH v2 5/5] News for updated maildir sync semantics Austin Clements 2012-06-10 8:32 ` [PATCH v2 0/5] Synchronize maildir flags in new/ Tomi Ollila 2012-06-10 18:06 ` Jani Nikula 2012-06-10 23:20 ` David Bremner
Code repositories for project(s) associated with this public inbox https://yhetil.org/notmuch.git/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).