allow to specify args for notmuch new #200

mjg · 2018-06-12T13:16:13Z

In move mode, afew calls notmuch new after moving mails around. This
prevents afew -m from being used in a pre-new hook in notmuch.

Allow to specify args, so that afew -m --notmuch-args=--no-hooks can
live happily in a pre-new hook.

flokli · 2018-06-12T14:53:02Z

Can you use #187 for that?

mjg · 2018-06-12T15:01:26Z

Can you use #187 for that?

First of all, sorry for not searching the issues before... #187 is not quite it but #188 looks promising since it updates the notmuch db (even more effectively) but avoids hooks. It implies a change in behaviour for those users which use hooks (in my case, welcome change). I'll try #188 and report back.

mjg · 2018-06-14T08:56:08Z

So, that approach (use notmuch bindings, not the binary) works and looks better all together, but the implementation is lacking (ignores rename setting). Cleaning up that patch over there seems to be worthwhile if you decide that the change in behaviour is OK (no more hooks being fired - fine for me :)).

GuillaumeSeren · 2018-08-10T13:13:35Z

Hey,
first thank you for the patch 🍺

I agree on the feature as a heavy user of the MailMover myself,
it is close / same as the #187 and could trigger the same db non atomic operation,
but if we assume that a user can manage that issue on its own,
we could merge that before the rework of the MailMover.

I will check the patch, but i think if it can solve both your issue (#200) and (#187),
it could be a good thing.

@vanicat Can you have a look and check if it solve your issue ?

rpuntaie · 2019-08-21T09:28:57Z

#200 is for notmuch pre-new hook. I call afew in post-new hook. There it does not lead to a second afew run. It seems like notmuch sees its DB in use and so does not call post-new.

I consider using it in the post-new hook is the right way,

because notmuch needs to have its DB up to date before afew can work on it, else catch NullPointerError in case mail file doesn't exist? #43.
and after afew moved mails, it cares to call notmuch new again, since again its DB must be updated to reflect afew's changes.

mjg · 2019-08-21T10:36:25Z

#200 is for notmuch pre-new hook. I call afew in post-new hook. There it does not lead to a second afew run. It seems like notmuch sees its DB in use and so does not call post-new.

I consider using it in the post-new hook is the right way,

because notmuch needs to have its DB up to date before afew can work on it, else catch NullPointerError in case mail file doesn't exist? #43.

and after afew moved mails, it cares to call notmuch new again, since again its DB must be updated to reflect afew's changes.

This depends on your use of MUA and synchronisers. For me, the MUA only changes tags in the notmuch db or adds messages (sending), i.e. it operates through the nm library and the db is in sync locally.

afew/notmuch does the remote sync, which encompasses fetching/pushing of e-mail including necessary moves by tag and tagging by folder. This has to be done in a specific order to make sense. My ordering ist:

nm.pre-new: afew -m --notmuch-args=--no-hooks makes sure any new tags lead to apropriate foldering
nm.pre-new: mbsync pushes the local state to the remote, fetches remote updates
notmuch syncs the local file system state (changed by mbsync) to the db
nm.post-new: afew -t -n tags new messages in the db

I could not come up with any other ordering that makes sure that db and local state and remote state are in sync and that requires only one run of notmuch, mbsync and the two afew modules. But I'd be happy to learn alternatives.

rpuntaie · 2019-08-21T11:34:16Z

I'm currently moving from Thunderbird to a more open mail store.
So I'm just figuring things out step-by-step and happy to learn from others.

I have set up a mail sync script, that basically does

mbsync

notmuch new

in $MAILDIR/.notmuch/post-new I have

# create missing folders based on afew config, then
afew -mn
afew -tn

My [MailMover] does not use tags, but rather something like:

gmail/INBOX = 'from:someone AND subject:something':somefolder

-tn then tags based on the new folders using FolderNameFilter.

remote is synched only the next time my sync script is called.
It will then

update the local storage with new mails from remote
update remote with the mails I have removed locally (i.e. moved to non-synced local mailboxes)

I mean, remote and local will get out of sync anyway.
There is no need to leave the script with both at sync.

In move mode, afew calls `notmuch new` after moving mails around. This prevents `afew -m` from being used in a pre-new hook in `notmuch`. Allow to specify args, so that `afew -m --notmuch-args=--no-hooks` can live happily in a pre-new hook.

`: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 · 2019-09-18T12:36:09Z

Ok let's merge this thank you for the patch @mjg ☕

flokli · 2019-09-23T05:50:54Z

afew/MailMover.py

        to_delete_fnames = []
        moved = False
        for query in rules.keys():
-            destination = '{}/{}/cur/'.format(self.db_path, rules[query])


Where did that cur here go?

Apparantly, #200 and #201 got merged together when they were both merged (or I rebased one PR branch too early, or github computes the diff relative to the original merge base).
In any case, this is reflected in the change to new_name(): In short, afew used to move mail from new to cur but failed to update flags (everyone moving to cur should update flags), so #201 made afew leave mail in the respective subdir rather than chopping off new/cur/.. and appending cur..

Sorry, but this still doesn't feel quite readable to me.

I could guess submaildir is cur or new, but just taking [1] out of os.path.split()'s tail sounds scary - what happens to the other path components?

Could this maybe be made slightly more readable way, maybe by adding some comments?

We could also probably use pathlib paths around instead of strings.

mjg added 5 commits August 26, 2019 11:20

allow to specify args for notmuch new

9a23ae2

In move mode, afew calls `notmuch new` after moving mails around. This prevents `afew -m` from being used in a pre-new hook in `notmuch`. Allow to specify args, so that `afew -m --notmuch-args=--no-hooks` can live happily in a pre-new hook.

MailMover: fix renames

d9bc00c

`: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.

MailMover: refactor get_new_name()

b89d86a

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.

Merge branches 'MailMover-fix-renames' and 'notmuch-args' into HEAD

ab7bfc5

mjg force-pushed the master branch from 2b24c76 to ab7bfc5 Compare August 26, 2019 09:34

GuillaumeSeren approved these changes Sep 18, 2019

View reviewed changes

GuillaumeSeren merged commit 6b321b8 into afewmail:master Sep 18, 2019

GuillaumeSeren mentioned this pull request Sep 18, 2019

Rework move logic #189

Open

flokli reviewed Sep 23, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

allow to specify args for notmuch new #200

allow to specify args for notmuch new #200

Uh oh!

mjg commented Jun 12, 2018

Uh oh!

flokli commented Jun 12, 2018

Uh oh!

mjg commented Jun 12, 2018

Uh oh!

mjg commented Jun 14, 2018

Uh oh!

GuillaumeSeren commented Aug 10, 2018

Uh oh!

rpuntaie commented Aug 21, 2019

Uh oh!

mjg commented Aug 21, 2019

Uh oh!

rpuntaie commented Aug 21, 2019

Uh oh!

GuillaumeSeren commented Sep 18, 2019

Uh oh!

flokli Sep 23, 2019

Uh oh!

mjg Sep 23, 2019

Uh oh!

flokli Oct 12, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

allow to specify args for notmuch new #200

allow to specify args for notmuch new #200

Uh oh!

Conversation

mjg commented Jun 12, 2018

Uh oh!

flokli commented Jun 12, 2018

Uh oh!

mjg commented Jun 12, 2018

Uh oh!

mjg commented Jun 14, 2018

Uh oh!

GuillaumeSeren commented Aug 10, 2018

Uh oh!

rpuntaie commented Aug 21, 2019

Uh oh!

mjg commented Aug 21, 2019

Uh oh!

rpuntaie commented Aug 21, 2019

Uh oh!

GuillaumeSeren commented Sep 18, 2019

Uh oh!

flokli Sep 23, 2019

Choose a reason for hiding this comment

Uh oh!

mjg Sep 23, 2019

Choose a reason for hiding this comment

Uh oh!

flokli Oct 12, 2019

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants