Re: PR for UID EXPUNGE to achieve MOVE semantics

Menno Finlay-Smits
 

I've just realised there's already a UID MOVE PR up: https://github.com/mjs/imapclient/pull/293

Having UID expunge support would still be awesome though.


On Tue, 10 Oct 2017, at 00:07, Menno Finlay-Smits wrote:
Hi Dave,

Thanks very much for your contribution and apologies for taking so long to get back to you.
 
I am using IMAPClient for a project, and have a need for single-message MOVE semantics. I saw the comment on this issue "...implement the MOVE extension to the UID command." https://github.com/mjs/imapclient/issues/142

I'd love to see support for MOVE.

Unfortunately, we need a patch to imaplib.py to get it to support 'UID MOVE' - it won't issue 'UID <command>' unless it supports <command>. https://github.com/python/cpython/blob/2.7/Lib/imaplib.py#L767-L768

That's a solvable issue. IMAPClient already patches out imaplib's Commands dict to add support for commands it doesn't already support. See https://github.com/mjs/imapclient/blob/1.x/imapclient/imapclient.py#L41-L65

It's ugly but it works. Future versions of IMAPClient probably won't depend on imaplib. When that happens the nasty patching can go.
 
I can achieve the semantics I want with:
 
COPY m other_folder
DELETE m
UID EXPUNGE m
 
(UID EXPUNGE is desirable to avoid expunging every deleted message in a folder).
 
So I put together this change to allow for UID EXPUNGE, based on the 1.x branch:
 
https://github.com/davebrown/imapclient/commit/fc8a0eeb2fc7c35f8355734fb7cd6b758ff721e2
 
The idea is to overload arguments to expunge(): if messages are specified, do a 'UID EXPUNGE <messages>', else it is a folder-wide expunge(). The former won't work if use_uid=False. I don't have insight how many IMAPClient users turn off uid's. 

I don't think many people use `use_uid=False`. What you've done looks sensible to me.

 
This also implements item on the list for https://github.com/mjs/imapclient/issues/36
 
That change also adds a livetest, which asserts that only the specified message is expunged, and other Deleted messages remain.

Thank you very much for taking the time to do the live test too.

 
If that change is within the ethos of the project, I'll submit a PR (to 1.x? master?). Otherwise, I'd like to figure out a way to achieve MOVE without a global expunge.

Your approach looks good. Please submit a PR. Against 1.x is fine - I can take care of forward porting it if you like.

If you have the time to do a PR for MOVE as well, that would be amazing. It's worth having both MOVE and UID EXPUNGE support.

 
Thanks,
Dave Brown
 
PS - I'm unsure what should be the return type from UID expunge. It's currently None, but should probably be whatever is the response payload, but I haven't sorted out the helper methods needed to achieve that. If the change is on the right track, I'll try to figure that out.

_command_and_check() should be giving you the response string so I'm not sure why you're not seeing anything. Perhaps set `debug=True` and see what the server is returning?

--
Menno Finlay-Smits
inbox@... :: http://menno.io/





--
Menno Finlay-Smits
inbox@... :: http://menno.io/

Join imapclient@groups.io to automatically receive all group messages.