Repository navigation
Implement RFC 6855 (IMAP Support for UTF-8) in imaplib. #65999
Description
| BPO | 21800 |
|---|---|
| Nosy | @warsaw, @pitrou, @ericvsmith, @bitdancer, @soltysh, @zvyn |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = <Date 2015-05-16.18:06:45.846>
created_at = <Date 2014-06-17.23:28:22.752>
labels = ['type-feature', 'expert-email']
title = 'Implement RFC 6855 (IMAP Support for UTF-8) in imaplib.'
updated_at = <Date 2015-05-16.18:06:45.845>
user = 'https://github.com/zvyn'bugs.python.org fields:
activity = <Date 2015-05-16.18:06:45.845>
actor = 'r.david.murray'
assignee = 'none'
closed = True
closed_date = <Date 2015-05-16.18:06:45.846>
closer = 'r.david.murray'
components = ['email']
creation = <Date 2014-06-17.23:28:22.752>
creator = 'zvyn'
dependencies = []
files = ['35808', '39284', '39324']
hgrepos = []
issue_num = 21800
keywords = ['patch']
message_count = 10.0
messages = ['222000', '236713', '242538', '242620', '242622', '242648', '242662', '242778', '242873', '242876']
nosy_count = 10.0
nosy_names = ['barry', 'pitrou', 'eric.smith', 'r.david.murray', 'jesstess', 'BreamoreBoy', 'python-dev', 'maciej.szulik', 'zvyn', 'H\xc3\xa5kan L\xc3\xb6vdahl']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue21800'
versions = ['Python 3.5']
Activity
zvyn commentedon Jul 1, 2014
I made a patch implementing the following changes to the IMAP4 class:
Does this look like a good idea to you? (I'll make a patch including docs when we agree on the API.)
BreamoreBoy commentedon Feb 26, 2015
The patch contains changes to code and tests have been added so can we have a formal review please.
bitdancer commentedon May 4, 2015
Here is an updated patch based on Milan's work, including docs. I've tweaked the API slightly: no dedicated method for doing the enable (instead it is inlined in authenticate), I added 'enable' to the exposed API (with a doc caveat about not using it for UTF8=ACCEPT), added None as a valid value for enable_UTF8 with the meaning "enable if possible" (True is now "must enable"), and exposed a utf8_enabled attribute so the program can easily tell what mode to use when specifying enable_UTF8=None. Someday, None should become the default. "None" is not the best choice for value, especially when it is not the default, so perhaps someone could suggest better values for that keyword.
It would be great if Milan or Maciej could give me a review (or anyone else who feels like it). I want to get this in before the beta deadline.
soltysh commentedon May 5, 2015
David I did the review and there's one thing that worries me the most, actually two:
bitdancer commentedon May 5, 2015
Well, the problem with that is that we then have to parse the capability to see if it is utf8 that is being enabled. I don't like that as an API, it feels fragile. Since capabilities cannot later be disabled, there's no functional reason to keep it separate. However, it would solve the problem of values to use in the init flag, and would remove the caveat that you shouldn't use UTF8=ENABLE in an explicit enable call, so perhaps it is best after all.
Do you have any interest in updating the patch? I won't be able to get back to it until this weekend.
soltysh commentedon May 6, 2015
Yes, I can update that (that IMAP testing bug - http://bugs.python.org/issue22137, is taking me longer than I expected it ;)).
I just want to make sure if I understand you correctly what's needs to be done is removing the utf8_enable code from init, we will enable ascii by default and only explicit call to enable method will enable it, am I missing something?
bitdancer commentedon May 6, 2015
An explicit call to enable with an argument string that contains 'UTF8=ACCEPT'. I did not go far enough through the BNF to determine if enable can be passed more than one capability at a time, but I suspect it can. In which case we should factor out the capability parsing such that we can reuse it for parsing the enable argument.
soltysh commentedon May 8, 2015
David, I've changed according to your suggestion, appreciate review.
python-dev commentedon May 10, 2015
New changeset 195343b5e64f by R David Murray in branch 'default':
bpo-21800: Add RFC 6855 support to imaplib.
https://hg.python.org/cpython/rev/195343b5e64f
bitdancer commentedon May 11, 2015
Thanks, Maciek (and Milan).
I tweaked your patch slightly (mostly doc changes...I moved the discussion of the utf8 RFC into the enable method only, and added back the docs for utf8_enabled). I made some review comments about the changes other than that doc reorg that I made before commit, just FYI.