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

Rework mail move logic #188

Open
flokli opened this issue Mar 21, 2018 · 5 comments
Open

Rework mail move logic #188

flokli opened this issue Mar 21, 2018 · 5 comments

Comments

@flokli
Copy link
Member

flokli commented Mar 21, 2018

Currently, when moving files, we move the file in the filesystem and shell out to notmuch new, and pick up new file locations in doing that.

This causes problems, as users often call afew via notmuch's hook system, and in general, it's a bit invasive operation.

A better way would be to do the following on mail move:

  • Copy the file to the new location
  • Call notmuch.database.add_message() with the new location
  • Remove the file on the old location
  • Call notmuch.database.remove_message() with the old location

This would supersede #187, and fix #139.

@GuillaumeSeren
Copy link
Collaborator

Hey @flokli ,
I think the logic is better than calling a big 'notmuch new',
but it is a quite different from the #187 approach,
so the PR has to be changed a lot,
so I am checking if I can work a small patch to do it.

@GuillaumeSeren
Copy link
Collaborator

I have started to work on the patch last night,
it went well but 'add_message()' is deprecated so I have to use index_file(),
and actually the hard part is the filename param is

        :param filename: should be a path relative to the path of the
            open database (see :meth:`get_path`), or else should be an
            absolute filename with initial components that match the
            path of the database.

Which will probably a bit more work to get done, but I am on it.

see: https://git.notmuchmail.org/git?p=notmuch;a=blob;f=bindings/python/notmuch/database.py;h=342d665a224761a2557ea00233f7d99c7dbae321;hb=HEAD#l426

GuillaumeSeren added a commit to GuillaumeSeren/afew that referenced this issue Mar 25, 2018
Following the afewmail#188 issue, we needed to change move() function,
to be able to move, and upgrade the database a bit more lightly:
* Copy the original file to the destination
* Add the new file to the database
* Remove the original file
* Remove the original mail from db
@GuillaumeSeren
Copy link
Collaborator

Hey !
So I've finished a working patch, I tested it on my setup it work well,
if you can test and/or send feedback it would be great !

GuillaumeSeren added a commit to GuillaumeSeren/afew that referenced this issue Apr 2, 2018
Following the afewmail#188 issue, we needed to change move() function,
to be able to move, and upgrade the database a bit more lightly:
* Copy the original file to the destination
* Add the new file to the database
* Remove the original file
* Remove the original mail from db
GuillaumeSeren added a commit to GuillaumeSeren/afew that referenced this issue Apr 2, 2018
Following the afewmail#188 issue, we needed to change move() function,
to be able to move, and upgrade the database a bit more lightly:
* Copy the original file to the destination
* Add the new file to the database
* Remove the original file
* Remove the original mail from db
@mjg
Copy link
Contributor

mjg commented Jun 14, 2018

Caveat: This patch ignores the "rename" setting! Had to clean up quite a few mbsync problems because of that.

Also, maybe it could be rebased and cut into smaller chunks that are easier to review?

@GuillaumeSeren
Copy link
Collaborator

Hey,
@mjg thank you for testing this patch it need more reviews, can you develop a bit more your problem with the rename setting ?

I've just rebased the branch please respond on #189 to help improve this patch.

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

No branches or pull requests

3 participants