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

bpo-27240 Rewrite the email header folding algorithm. #3488

Merged
merged 3 commits into from Dec 3, 2017

Conversation

@bitdancer
Copy link
Member

bitdancer commented Sep 11, 2017

This will also fix bpo-30788.

The original algorithm tried to delegate the folding to the tokens so
that those tokens whose folding rules differed could specify the
differences. However, this resulted in a lot of duplicated code because
most of the rules were the same.

The new algorithm moves all folding logic into a set of functions
external to the token classes, but puts the information about which
tokens can be folded in which ways on the tokens...with the exception of
mime-parameters, which are a special case (which was not even
implemented in the old folder).

This algorithm can still probably be improved and hopefully simplified
somewhat.

Note that some of the test expectations are changed. I believe the
changes are toward more desirable and consistent behavior: in general
when (re) folding a line the canonical version of the tokens is
generated, rather than preserving errors or extra whitespace.

https://bugs.python.org/issue27240

@bitdancer bitdancer requested a review from python/email-team as a code owner Sep 11, 2017
@@ -245,13 +245,16 @@ def fold(self, *, policy):
the header name and the ': ' separator.
"""
# At some point we need to only put fws here if it was in the source.
# At some point we need to put fws here iif it was in the source.

This comment has been minimized.

Copy link
@cryvate

cryvate Sep 11, 2017

Contributor

iif stands for "if and only if" in this statement? I've only ever seen iff used as an acronym for that.

This comment has been minimized.

Copy link
@bitdancer

bitdancer Sep 11, 2017

Author Member

Correct.

bitdancer added 2 commits Sep 7, 2017
The original algorithm tried to delegate the folding to the tokens so
that those tokens whose folding rules differed could specify the
differences.  However, this resulted in a lot of duplicated code because
most of the rules were the same.

The new algorithm moves all folding logic into a set of functions
external to the token classes, but puts the information about which
tokens can be folded in which ways on the tokens...with the exception of
mime-parameters, which are a special case (which was not even
implemented in the old folder).

This algorithm can still probably be improved and hopefully simplified
somewhat.

Note that some of the test expectations are changed.  I believe the
changes are toward more desirable and consistent behavior: in general
when (re) folding a line the canonical version of the tokens is
generated, rather than preserving errors or extra whitespace.
Blurb doesn't support multiple issue numbers, so just list them.
@bitdancer bitdancer force-pushed the bitdancer:rewrite_folder branch from e46f7fa to e36dc10 Dec 3, 2017
@bitdancer bitdancer merged commit 85d5c18 into python:master Dec 3, 2017
3 checks passed
3 checks passed
bedevere/issue-number Issue number 27240 found
Details
bedevere/news News entry found in Misc/NEWS.d
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@miss-islington

This comment has been minimized.

Copy link

miss-islington commented Dec 3, 2017

Thanks @bitdancer for the PR 🌮🎉.. I'm working now to backport this PR to: 3.6.
🐍🍒🤖

miss-islington added a commit to miss-islington/cpython that referenced this pull request Dec 3, 2017
The original algorithm tried to delegate the folding to the tokens so
that those tokens whose folding rules differed could specify the
differences.  However, this resulted in a lot of duplicated code because
most of the rules were the same.

The new algorithm moves all folding logic into a set of functions
external to the token classes, but puts the information about which
tokens can be folded in which ways on the tokens...with the exception of
mime-parameters, which are a special case (which was not even
implemented in the old folder).

This algorithm can still probably be improved and hopefully simplified
somewhat.

Note that some of the test expectations are changed.  I believe the
changes are toward more desirable and consistent behavior: in general
when (re) folding a line the canonical version of the tokens is
generated, rather than preserving errors or extra whitespace.
(cherry picked from commit 85d5c18)
@bedevere-bot

This comment has been minimized.

Copy link

bedevere-bot commented Dec 3, 2017

GH-4693 is a backport of this pull request to the 3.6 branch.

@bitdancer bitdancer deleted the bitdancer:rewrite_folder branch Dec 4, 2017
bitdancer added a commit that referenced this pull request Dec 4, 2017
The original algorithm tried to delegate the folding to the tokens so
that those tokens whose folding rules differed could specify the
differences.  However, this resulted in a lot of duplicated code because
most of the rules were the same.

The new algorithm moves all folding logic into a set of functions
external to the token classes, but puts the information about which
tokens can be folded in which ways on the tokens...with the exception of
mime-parameters, which are a special case (which was not even
implemented in the old folder).

This algorithm can still probably be improved and hopefully simplified
somewhat.

Note that some of the test expectations are changed.  I believe the
changes are toward more desirable and consistent behavior: in general
when (re) folding a line the canonical version of the tokens is
generated, rather than preserving errors or extra whitespace.
(cherry picked from commit 85d5c18)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.