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

Perform whitespace removal (like Mojo 6) in Sanitize #4

Merged
merged 1 commit into from Dec 1, 2016

Conversation

Projects
None yet
2 participants
@jmaslak
Contributor

jmaslak commented Dec 1, 2016

I received your module as part of the November CPAN Pull Request Challenge. I looked at the bug referenced in #3 and put together this pretty simple fix (it's very similar tot he code that was removed in Mojo 7).

Mojo 7 no longer trims or removes extra whitespace in the all_text() method call, which caused a test to fail.

There are two ways to fix it - either add the whitespace removal to the sanitize method, or change the test. Because of concerns about breaking compatibility, I'm thinking removing the whitespace is the better option, but if you disagree, let me know and I'll submit a PR with the test changed instead.

@sshaw

This comment has been minimized.

Show comment
Hide comment
@sshaw

sshaw Dec 1, 2016

Owner

Yay, CPAN PRC 🎉

Thanks for this. Fixing broken plugins every major Mojolicous release; I think I've reached my limit...

🏃

Owner

sshaw commented Dec 1, 2016

Yay, CPAN PRC 🎉

Thanks for this. Fixing broken plugins every major Mojolicous release; I think I've reached my limit...

🏃

@sshaw sshaw merged commit 14383d2 into sshaw:master Dec 1, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@sshaw sshaw referenced this pull request Dec 1, 2016

Closed

Tests fail with Mojolicious 7 #3

@sshaw

This comment has been minimized.

Show comment
Hide comment
@sshaw

sshaw Dec 1, 2016

Owner

Released as v0.06.

Owner

sshaw commented Dec 1, 2016

Released as v0.06.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment