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

Refactored to use @jordan-wright email package #4

Merged
merged 3 commits into from
May 5, 2014

Conversation

jordan-wright
Copy link
Collaborator

Hey there!

I appreciate you checking out my email package before, since it turned out there was a sneaky bug that I didn't catch! The issue was that, when sending the To field, I was just putting in the whole string, instead of just the address, which is required.

All fixed now! If you'd like, feel free to give this pull request a shot and see if it works a little better for you.

@jordan-wright
Copy link
Collaborator Author

This pull request also adds the ability to attach files as follows:

-attach file1.txt,file2.txt,etc.

@zachlatta
Copy link
Owner

Thanks! The primary reason I reverted 3f0665e was because, for whatever reason, my mail client has trouble parsing emails sent from your package.

I use sup. The entire message is loaded as an attachment.

selection_028

Here's a gist with two examples messages. The first is with your package and doesn't parse correctly in sup. The second is gophermail and parses correctly in sup.

Here's what a properly parsed email looks like:

selection_029

@jordan-wright
Copy link
Collaborator Author

Thanks for the update, Zach. You are correct, it does look like sup is having trouble parsing the message. Although, the only difference I can see in message structure is my (IMO preferred) use of quoted printable encoding instead of base64. I'm almost positive my encoding is according to the RFC, so I'm not sure where the discrepancy is.

I'll install sup, play around with it, and see what I can come up with. Until then, I'll open an issue on both my email and the sup repos to track it down together.

If you don't mind, can we keep this open for now, and I'll keep you posted with updates?

@jordan-wright
Copy link
Collaborator Author

Would you mind if I shared the gist you provided in both an issue in my email package as well as the issue in the sup repo?

@jordan-wright
Copy link
Collaborator Author

I believe I have this all fixed. Tl;dr - the problem was that I wasn't adding a Mime-Version header, which I have yet to see in the RFC's. Regardless, sup requires it, so it is all added now. Feel free to give it a shot with the updated package and see if that works for you! 😄

@jordan-wright jordan-wright reopened this May 5, 2014
@zachlatta zachlatta merged commit d2a94dd into zachlatta:master May 5, 2014
@zachlatta
Copy link
Owner

Merged! Thanks!

@zachlatta zachlatta mentioned this pull request May 5, 2014
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.

None yet

2 participants