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

Improve quoted-printable decoding #307

Merged
merged 1 commit into from
Feb 11, 2022

Conversation

Maria-12648430
Copy link
Contributor

This PR improves Quoted-Printable decoding by removing look-aheads in the remaining binary, and processing the entire body in one go instead of line by line.

This PR also includes a small (unrelated) nano-optimization/correction, replacing a list with a tuple in a case expression and the associated matches.

@mworrell mworrell mentioned this pull request Feb 11, 2022
@mworrell
Copy link
Collaborator

Hi @Maria-12648430, we merged a re-format of the code, which makes your PR conflict with master 😭

Could you rebase?

@Maria-12648430
Copy link
Contributor Author

Could you rebase?

It will probably have to wait until next week, but yes, sure 😊

@Maria-12648430
Copy link
Contributor Author

Well, I did it today ;) For reference, I rebased my branch on master, solved the merge conflicts, then ran rebar3 fmt. Hope it all turned out as you expect.

<<
"=?ISO-8859-1?Q?a?=\n"
" =?ISO-8859-1?Q?b?="
>>,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was not part of my original PR, it got reformatted with the rest when I ran rebar3 fmt.

Copy link
Collaborator

@seriyps seriyps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Can't say I 100% understand QP-encoding (especially soft-linebreaks and whitespace management).
But since code is now simpler and more optimized + all tests pass - I'm for it.

@mworrell mworrell merged commit 45ed7f9 into gen-smtp:master Feb 11, 2022
@mworrell
Copy link
Collaborator

Thanks @Maria-12648430 and sorry for the needed rebase.

@Maria-12648430
Copy link
Contributor Author

Thanks @Maria-12648430 and sorry for the needed rebase.

No worries 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants