JoshCheek / atom-seeing-is-believing

Seeing is Believing integration for the Atom text editor.
Do What The F*ck You Want To Public License
62 stars 4 forks source link

Fix issue with `String#contains` #15

Closed octosteve closed 9 years ago

octosteve commented 9 years ago

Atom seems to like String#includes over String#contains all of a sudden on my install. If you can verify this is a problem for others, here's the change. Closes #13

JoshCheek commented 9 years ago

The fix

Yeah, I just updated from 0.186.0 to 0.189.0 and experienced this.

Given that this would break 0.186.0, do you think its reasonable to do this? Which I assume will work on both:

(capturedError.contains || capturedError.includes)('LoadError')

Or, alternatively, there is apparently a "polyfill" (taken from the MDN docs)

if (!String.prototype.includes) {
  String.prototype.includes = function() {'use strict';
    return String.prototype.indexOf.apply(this, arguments) !== -1;
  };
}

Actually, what about using indexOf directly?

capturedError.indexOf('LoadError') !== -1

Investigation

What the fuck, Javascript?

I find JavaScript to be completely opaque :(

My copy of node has neither (modified for readability):

$ node --version
v0.10.26

$ node -pe '"abc".includes("b")'
TypeError: Object abc has no method 'includes'
"abc".includes("b")
      ^

$ node -pe '"abc".contains("b")'
TypeError: Object abc has no method 'contains'
"abc".contains("b")
      ^

MDN docs say it should be includes, but that it's an "experimental technology, part of the ECMAScript 6 (Harmony) proposal".

And I can't figure out how to get a list of methods for strings, yet somehow Chrome and Node can provide an autocomplete list:

screenshot 2015-04-04 22 47 31

It makes no sense to me. I'll switch it to includes, but I have no confidence it won't just magically break again at some point. I swear this language is fkn capricious.

JoshCheek commented 9 years ago
JoshCheek commented 9 years ago

I'm going to merge this and switch it to indexOf so that I don't break anyone on an older version. Happy to hear any input you have, Steve. And thanks for the fix ^_^ I didn't even know it was broken >.<

You going to DCamp this year? https://twitter.com/ruby_dcamp/status/582963878327353344 Would love to see y'all again! (edit: Oh, nvm, you are :P)

octosteve commented 9 years ago

I think indexOf is the way to go. Don't get me started on JS...

octosteve commented 9 years ago

@JoshCheek See you there!