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

Removed dependencies and synchronous IO operations #3

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

jacamera
Copy link

Thanks so much for taking the time to port this library and open source it!

Unfortunately I wasn't able to use it as-is for two reason:

  1. A namespace/type conflict between the types in MimeKitLite referenced by QuoteParser and the same types referenced by MailKit which I am using for other email related tasks, a scenario which I would guess might be somewhat common for developers interested in email quote parsing.
  2. The call to MimeMessage.Load in Parse.cs at line 16 is a synchronous IO operation that is forbidden by default in newer versions of ASP.NET core.

In order to address both issues I removed the dependencies in my fork and updated all the tests (all passing) to process the mail text content only, leaving all the MIME parsing concerns to MimeKit or some other external MIME parser. No idea if you're interested in merging such changes but figured I'd make a request just in case. Thanks again!

@paulirwin paulirwin self-assigned this Oct 29, 2019
@paulirwin
Copy link
Member

Hey @jacamera, thanks for taking the time to submit this PR! I did not realize some of the problems you've brought up existed. However, unfortunately I can't accept the PR as-is. I'm trying to keep this as close of a port to the original as possible, and the email test cases came directly from the upstream project. I'd like to keep those in sync in order to ensure compatibility. What about keeping the eml file tests as-is and parsing them in the unit test project with MimeKitLite instead of in the main library? That would keep the source test cases identical, while removing the dependency and moving the parsing out of the library.

It does seem reasonable to remove MimeKitLite from the project, as you've done here, as all it really is used for is to get the text and look at headers. But if we go this direction, I'd like to include examples in the README of how to do this with various libraries, including reply to header detection. Would you mind adding some examples with a couple of the libraries you've mentioned?

Thanks!

@jacamera
Copy link
Author

Hi @paulirwin, great suggestions! I restored the original tests, added MimeKitLite to the Tests project to parse the test emails and updated the README with some usage examples. Let me know if you see any other issues with the changes.

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.

2 participants