Update README example: Expunge is implicit in MOVE#623
Conversation
| else | ||
| imap.copy(message_id, "Mail/sent-apr03") | ||
| imap.store(message_id, "+FLAGS", [:Deleted]) | ||
| imap.expunge |
There was a problem hiding this comment.
Good catch. Strictly speaking UID EXPUNGE is explicit in MOVE. I wonder if that complication is worth documenting as well? Currently though, the example is written for message sequence numbers... and #uid_expunge would need a larger edit to update it to use UIDs instead.
There was a problem hiding this comment.
Will there be many IMAP instances without MOVE but with UID EXPUNGE?
If so, it could be worth a rewrite as a separate change; I think it's important not to encourage an unnecessary expunge
There was a problem hiding this comment.
I agree. It's important to not encourage an unnecessary expunge in this example. I'll merge it as-is.
But it makes me want to update the example even more. 😉
Not only is it using sequence numbers (which rules out UID EXPUNGE), it's moving a single message at a time, which is generally much slower than moving in bulk. But, in my experience many popular servers have an (unknown) upper limit to how many messages they'll move in a single command before they start returning errors (or simply drop the connection!). So for best efficiency, you need to move in batches but tune the batch size to the server... But servers are less likely to raise an error when expunging a bunch of messages at once. So in my own code, I tend to batch the MOVE vs COPY/STORE/UID EXPUNGE, and then send a single EXPUNGE (when MOVE and UIDPLUS are not supported) after all of the batches... but that can be problematic when moving thousands of messages, etc, etc.
And all of that makes the example much more complicated! 🫤
batch_size = 50
dest = "Mail/sent-apr03"
search_result = imap.uid_search(["SINCE", "1-Apr-2003", "BEFORE", "30-Apr-2003"])
uidset = search_result.to_sequence_set
uidset.each_number.each_slice(batch_size) do |uid_batch|
if imap.capable?(:move) || imap.capable?(:IMAP4rev2)
imap.move(uid_batch, dest)
else
imap.copy(uid_batch, dest)
imap.store(uid_batch, "+FLAGS", [:Deleted])
if imap.capable?(:uidplus)
imap.uid_expunge uid_batch
else
imap.expunge # NOTE: this also expunges messages deleted by other sessions
end
end
endI don't know whether or not this more complicated example should be in the README, but I think it would be nice to put these more complex examples into the rdoc somewhere. (It also makes me think want to add a #capable_any? method.)
No description provided.