Not sure why the Travis build stalled, all tests passed for me...

yay ... I really want that iconv dependency nuked

On Sat, Feb 16, 2013 at 9:45 AM, Alexander notifications@github.com wrote:

Not sure why the Travis build stalled, all tests passed for me...

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/170#issuecomment-13632232.

Passing now! (I somehow managed to start a new build on Travis.)

I triggered a new build :)

On Friday, February 15, 2013, Alexander wrote:

Passing now! (I somehow managed to start a new build on Travis.)

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/170#issuecomment-13634695.

Ha, I refreshed the build page and it looked like it had just restarted. Good timing!

I almost made this same pull request when I made issue #100, but after pondering it I wanted to either:

• Prove that this code is necessary, thus write a spec.
• OR prove that this code is extraneous, thus removing it.

@alxndr can you substantiate either?

TextSentinel is only used to analyze user input sent through Rails, not the arbitrarily invalid strings I can send to it via console.

I haven't been able to pack an invalid string that would actually ever hit TextSentinel through a form in Rails. Or one curled to the app. Other machinery like URI decoding and Rails' internal utf-8 coercion would fix the string long before it landed in TextSentinel.

And when I do manually pack an invalid string and drop it into the String#encode proposal, it throws invalid byte sequence in UTF-8 which is precisely what the Iconv version is designed to handle. Fails for the same reason ActiveSupport::Mulitbyte::Unicode#normalize fails.

So, is it necessary?

@danneu I am pretty certain it is unnecessary. I was the one who added it originally and it was only to try and stop some kinds of unicode griefing that we've fixed in other ways (entropy check + minimum spaces mostly.)

I'm delighted to accept this patch @alxndr! 🐟

Got a sorta related question on meta about writing tests for a JS bug: http://meta.discourse.org/t/how-should-discourse-handle-github-pull-requests/3053/7