gh-122953: add support for IMAP4rev2 in imaplib#150282
Conversation
Documentation build overview
|
There was a problem hiding this comment.
There are too many changes at the same time. Isn't there a simpler design while retaining a minimum diff? in addition, we need a What's New entry.
There are also too many small helpers that are only used once. They are not needed. Just inline them. I also don't know whether we really need to support rev2 now or if we can just leave it to third-party libraries. How often is it used?
According to the issue, just adding the rev2 shouold be enough. Why supporting more rev2 commands in this PR?
|
|
||
| This module defines three classes, :class:`IMAP4`, :class:`IMAP4_SSL` and | ||
| :class:`IMAP4_stream`, which encapsulate a connection to an IMAP4 server and | ||
| implement a large subset of the IMAP4rev1 client protocol as defined in |
There was a problem hiding this comment.
Retain the mention to RFC 2060 and add mention to RFC 9051 for rev2.
| self._send_textline(' '.join((tag, code, message))) | ||
|
|
||
| def handle(self): | ||
| def handle(self): |
There was a problem hiding this comment.
sorry, will fix this. I was lot on this function after creating tests to understand how handle of the server calls the commands in test cases and was doing find on it so accidentally added space.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
Initially I thought it was a quick change but after reading RFC 9051 and the previous ones since 1996 and the related ones, the changes didn't seemed simple because rev2 has quite a few capabiliites builtin including ENABLE, IDLE, and UTF-8 so they wouldn't be advertized in capability and also if a imap server advertises both rev1 and rev2, rfc 9051 requires to first switch to rev1 and then enable rev2 using enable command - I saw this case in docker of imap stalwart image. At few places in imaplib, there are exceptions raised if the capability is not there so I had to make check for the above mentioned case. uf8-accept is extension is rev1 but builtin in rev2 and is used for folder names and internatinal characters in header. I didn't like helper methods at first as they grew but decided with helpers at last after thinking of dataclass for rev2 policy or single big helper method which get called in _command helper because all commands funnel through there but then _command wouldn't have single responsibility. |
add support for IMAP4rev2 as outlined in RFC 9051
A number of rev1 extensions are folded into rev2 including IDLE and ENABLE and utf-8 support out of the box (see RFC 9051 Appendix E.2). For backward compatibility if server advertises both rev2 and rev1, connect as rev1 and switch to rev2 using enable IMAP4rev2. Helper methods have been added to do the check. Checked in colab that the major consumer imap Gmail, Yahoo, Outlook, icloud and AOL have rev1 capability. Ran docker container of imap stalwart advertising capability rev1 and rev2.