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

Mail mover fix renames #201

Merged
merged 3 commits into from Sep 18, 2019
Merged

Conversation

mjg
Copy link
Contributor

@mjg mjg commented Jun 14, 2018

So here is a suggestion to fix #196.
A fix to #190 could go on top easily (although it is unrelated). In fact, there is currently a mix of "treating paths right" (os.path...) and working with / by hand which could use some sanitizing.

@GuillaumeSeren
Copy link
Collaborator

Hey @mjg
Thank you for the PR, as the MailMover is actually in refactor and the PR is open #189,
we have to state the order to pass the PR but we are working on it.

`:flags` need to be preserved when renaming files, but UIDs of the form
`U=[0-9]+` should be removed. The current code fails to do that when a mail
file has a UID but no flags.

Change the code to nuke the non-flag part in any case.
Refactor so that the name generation becomes clearer, and before
changing the dir generation.

Change in behavour: returns a full path instead of a dir now in the
rename==False case.
Currently, MailMover moves all mail to `/cur`. But everyone moving mail
from `/new` to `/cur` is required to set up the maildir flags field in
the name.

Rather than setting up that field, preserve the maildir subdir part and
let other clients change subdirs as necessary. After all, "moving" does
not constitute "reading".
@GuillaumeSeren
Copy link
Collaborator

GuillaumeSeren commented Sep 18, 2019

As the mail mover as to be refactor and discussed again,
I think we can merge this.

Thank you for the patch @mjg 🍺

@GuillaumeSeren GuillaumeSeren merged commit ea6221d into afewmail:master Sep 18, 2019
@mjg mjg deleted the MailMover-fix-renames branch September 18, 2019 14:33
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 this pull request may close these issues.

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