Improving the representation of RFC8621 groups#112
Open
sftse wants to merge 3 commits intostalwartlabs:mainfrom
Open
Improving the representation of RFC8621 groups#112sftse wants to merge 3 commits intostalwartlabs:mainfrom
sftse wants to merge 3 commits intostalwartlabs:mainfrom
Conversation
The enum separating the Vec<Addr> from the Vec<Group> case carries a large API burden with it. The Address::List variant introduces code duplication to any consumer which has to write code to handle both Address::List and Address::Group, even though much of this code will be the same due to the way RFC8621 4.1.2.4 GroupedAddresses deals with single mailboxes. By reducing the enum variants, this duplication is no longer necessary.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The way
mail-parsercurrently represents mailboxes and groups is a bit verbose to deal with, and makes subsequent handling more complex and error prone. Because of the arguably awkward way RFC8621 decides to represent single mailboxes #98 as a group by section4.1.2.4 GroupedAddresses, anybody that does need the fidelity of the addresses parsed as in RFC5322 has to decompose them again.This PR does not advocate changing the representation to RFC5322, instead suggesting more modestly that the enum be abandoned. Because of the awkwardness of RFC8621
GroupedAddressesusers that do match on the enum already must be aware that single mailboxes may have different representations, and have to make sure to handle mailboxes the same whether they are inAddress::ListorAddress::Group. As an example, we needed the RFC5322 representation for which we used this codeThis exemplifies how the enum is more of a hinderance than a benefit as an API, but the common
Address::Listcase does save one allocation overAddress::Group. This PR removes the enum, focusing on the way this simplifies the API, but we can salvage the allocation if necessary.For reference, this is what the above code would look like after this PR.