Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should afew's MailMover move emails out of Maildir's /new or /tmp subfolders ? #196

Closed
dannyob opened this issue May 9, 2018 · 5 comments · Fixed by #201
Closed

Should afew's MailMover move emails out of Maildir's /new or /tmp subfolders ? #196

dannyob opened this issue May 9, 2018 · 5 comments · Fixed by #201

Comments

@dannyob
Copy link

dannyob commented May 9, 2018

I use afew as a post-new notmuch hook, after I've synced my mail using mbsync but before my new mail has been read by my MUA. As a result, new mail is still in the /new sub-directory (and indexed as such by notmuch).

This also means that each of those mail's filenames has not yet had any metadata appended, which means that filename doesn't have a colon in it, which means that MailMover's file renaming strategy breaks. The end result is that the UID element of the mail is not stripped out, leading to conflicting UIDs in the destination Maildir.

I have a small patch for this for afew, which limits moves to files in the "/cur" subfolder.

Does that seem the right approach? Or is this something better handled by calling afew at a different point?

@flokli
Copy link
Member

flokli commented May 10, 2018

Right, good catch!

However, I hesitate in just silently skipping mails. It might still be a valid usecase to move mails from inside the /new subfolder with MailMover (as an alternative to sieve rules on the server side).

So I think extending MailMovers renaming strategy to also work with files without any metadata appended should be the right way to fix it.
I fear MailMover currently also doesn't properly take into account preserving mails in the new subdir of the destination.

I still need to push some changes on top of #189 (will hopefully get to that this weekend), after that, we should work on a patch to fix the behaviour.

@mjg
Copy link
Contributor

mjg commented Jun 14, 2018

On moving: I would think we have either "no metadata no UID" or "metadata and UID", but I may be wrong. In any case /new should be moved to /new, not /cur, shouldn't it?

On the hook ( ;) ): I think that afew -m belongs in the pre-new hook, afew -t in the post-new hook. The reasoning is the following: afew (folderfilter+mailmover) is about keeping a mapping between folders and tags. In your notmuch-based MUA, you operate on tags and want that action to be reflected in a folder change. So, once you change tags in your MUA, the tags are "out of snyc" with the locations. In that state, afew -m is what makes the folders follow the tags; then, mbsync --push is what pushes these changes to the IMAP server; mbsync --pull brings in new mail into specific folders; notmuch new makes them known to the notmuch db; afew -t makes the locations of the new mails be reflected in the tags. So:

pre-new: afew -m; mbsync
post-new: afew -t -n

@mjg
Copy link
Contributor

mjg commented Jun 14, 2018

So, I've checked the maildir and mbsync docs, and the upshot is:

Whoever moves maildir mail from /new to /cur is required to add :metadata. So, I suggest that afew's MailMover would move from /new to /new and from /cur to cur.

UIDs in a maildir using the native mbsync format need to be unique. So, if the message has a UID and is being moved somewhere else then the UID has to be removed (so that it will be reassigned one by mbsync). A UID without :metadata should happen within /new only, but it's best not to depend on this. So I suggest afew's MailMover to match on ,U=[0-9]+ and remove only that.

@mjg
Copy link
Contributor

mjg commented Jun 14, 2018

Addition, sorry: To be extra safe, regenerating the name (as MM currently does) is best. Patch coming.

@dannyob
Copy link
Author

dannyob commented Jun 15, 2018

This looks great, thankyou!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants