Re: [PATCH] test: Adding non-maildir tags does not move message from new to cur

Subject: Re: [PATCH] test: Adding non-maildir tags does not move message from new to cur

Date: Sun, 09 Dec 2012 00:14:12 +0200

To: david@tethera.net, notmuch@notmuchmail.org

Cc: Michal Sojka

From: Jani Nikula


On Sat, 08 Dec 2012, david@tethera.net wrote:
> From: Michal Sojka <sojka@os.inf.tu-dresden.de>
>
> Some MUA's like mutt show the difference between "new" emails living in maildir
> directory new/, and "old" emails living in maildir directory cur/. However
> notmuch tag unconditionally moves selected messages from new/ to cur/, even if
> no maildir synchronized tag is changed.
>
> While maildir specification forbids messages with tags living in new/, there is
> no need to move messages to cur/ when no maildir synchronized tag is changed.
> Thus notmuch can remain transparent with respect to other MUA's.
>
> [ Edited commit log to better describe the intended changes, and tag the
>   test as broken until the actual changes are implemented -- Louis Rilling ]
>
> Signed-off-by: Louis Rilling <l.rilling@av7.net>
>
> [ Converted to use test_subtest_known_broken, David Bremner ]
> ---
>
> Do we agree that the behaviour of moving messages to ./cur on tagging
> is broken? If so, maybe it's worth tidying up and applying this.  The
> use of cd and ls strikes me as slightly suspect, but I welcome other
> opinions.

I think I would narrow down the special case a bit: I think messages in
./new that have no maildir flags, and have no ":2," in the end of the
filename, and and the tag change(s) will not affect maildir flags,
should stay in ./new. Files in ./new should not have ":2," or maildir
flags, and I see no reason to support having them there.

Thus any messages in ./new that do have maildir flags, or have ":2," in
the end of the filename should probably be moved to ./cur, even if the
tag change(s) do not affect maildir flags. The patch in this thread
fails here. It also changes the behaviour for messages in ./cur by not
appending ":2," to them.

As to the test, I think it should do something along the lines of (based
on search-output test):

notmuch search --output=files subject:"Message to stay in new" | sed -e "s,$MAIL_DIR,MAIL_DIR," >OUTPUT
cat <<EOF >EXPECTED
MAIL_DIR/new/message-to-stay-in-new
EOF
test_expect_equal_file OUTPUT EXPECTED

And would be nice to have similar tests for the other things I mentioned
above. If people agree with narrowing down the special case as I
suggest, that is.


BR,
Jani.

>
>  test/maildir-sync |    9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/test/maildir-sync b/test/maildir-sync
> index 0fc742a..6165782 100755
> --- a/test/maildir-sync
> +++ b/test/maildir-sync
> @@ -83,6 +83,15 @@ test_expect_equal "$output" "No new mail."
>  # creating new directories in the mail store, then it should be
>  # creating all necessary database state for those directories.
>  
> +test_begin_subtest "Adding non-maildir tags does not move message from new to cur"
> +test_subtest_known_broken
> +add_message [subject]='"Message to stay in new"' \
> +    [date]='"Sat, 01 Jan 2000 12:00:00 -0000"' \
> +    [filename]='message-to-stay-in-new' [dir]=new
> +notmuch tag +donotmove subject:"Message to stay in new"
> +output=$(cd "$MAIL_DIR"; ls */message-to-stay-in-new*)
> +test_expect_equal "$output" "new/message-to-stay-in-new"
> +
>  test_begin_subtest "Removing 'S' flag from existing filename adds 'unread' tag"
>  add_message [subject]='"Removing S flag"' [filename]='removing-s-flag:2,S' [dir]=cur
>  output=$(notmuch search subject:"Removing S flag" | notmuch_search_sanitize)
> -- 
> 1.7.10.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Thread: