Skip to content

Conversation

@anaulin
Copy link
Contributor

@anaulin anaulin commented Apr 6, 2014

This fixes the problem described in issue #17

anaulin added 3 commits April 5, 2014 21:30
…dle of the line don't get interpreted as beginning of signature.
… the middle of the line don't get interpreted as beginning of signature."

This reverts commit 1e8a064.
…the middle of text don't get interpreted as beginning of signature.
@anaulin
Copy link
Contributor Author

anaulin commented Apr 8, 2014

@technoweenie could you have a look at this?

@anaulin
Copy link
Contributor Author

anaulin commented Apr 17, 2014

@defunkt could you or someone else on your team have a look at this PR?

@technoweenie
Copy link
Contributor

Hi, thanks for the patch. I've filed an internal issue for someone to look at. Do you want me cut a pre-release gem for you?

@anaulin
Copy link
Contributor Author

anaulin commented Apr 17, 2014

No need for a pre-release gem, thanks (I'm running a rudely monkey-patched version).

@anaulin
Copy link
Contributor Author

anaulin commented May 30, 2014

Any chance this will ever get reviewed? Or should I just close this PR?

@technoweenie
Copy link
Contributor

Nope, this slipped through the cracks. I'll just merge it.

@technoweenie
Copy link
Contributor

  2) Failure:
test_reads_email_with_correct_signature(EmailReplyParserTest) [/Users/rick/p/email_reply_parser/test/email_reply_parser_test.rb:87]:
Expected /^-- \nrick/ to match "--\nrick\n".

  3) Failure:
test_reads_simple_body(EmailReplyParserTest) [/Users/rick/p/email_reply_parser/test/email_reply_parser_test.rb:14]:
<3> expected but was
<2>.

  4) Failure:
test_reads_top_post(EmailReplyParserTest) [/Users/rick/p/email_reply_parser/test/email_reply_parser_test.rb:33]:
<5> expected but was
<4>.

Failures ^

@technoweenie technoweenie merged commit ca9c5a1 into github:master May 30, 2014
@technoweenie
Copy link
Contributor

Fix in 7837dac. The regexes are backwards because of the way it figures out which fragments to show on emails with inline replies. Take the following email for example:

> 1: Hi

2. Sup

> 3: Bye!

We want fragment 3 to be hidden. Fragment 1 should be shown, because it's necessary context for fragment 2. If we parse top to bottom, we have no idea if fragment 1 should be hidden or not. By parsing from bottom to top, we mark replied fragments hidden until an original fragment (like 2) is found.

@anaulin
Copy link
Contributor Author

anaulin commented May 30, 2014

Thanks for fixing that, and for merging in. Ok with you if I delete this branch? Or do you guys like to keep those around?

@anaulin
Copy link
Contributor Author

anaulin commented May 30, 2014

Also, any idea when you'll have a new release with this fix?

@technoweenie
Copy link
Contributor

Just pushed it to rubygems. https://github.com/github/email_reply_parser/releases/tag/v0.5.6

Feel free to delete the branch. GitHub PRs are Git refs, so we can easily restore them if we really need to.

Thanks for the patch!

@anaulin
Copy link
Contributor Author

anaulin commented May 30, 2014

Awesome, thanks for the review and the release!

@anaulin anaulin deleted the sig-delimiter-middle-of-line branch May 30, 2014 20:26
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