-
Notifications
You must be signed in to change notification settings - Fork 22
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
How do you feel about converting this to ES6? #54
Comments
Certainly not opposed! Might be a pretty big undertaking, but if you submit a PR, i'll definitely take a look :) |
The toughest part of this is the heavy use of the I ran Coffee destroy: ->
@activeItemSub.dispose()
@changeSub?.dispose()
@tokenizeSub?.dispose() JS destroy() {
this.activeItemSub.dispose();
__guard__(this.changeSub, x => x.dispose());
return __guard__(this.tokenizeSub, x1 => x1.dispose());
}
function __guard__(value, transform) {
return (typeof value !== 'undefined' && value !== null) ? transform(value) : undefined;
} I'm not super-familiar with atom plugin development, so I don't know how many of these existential operators are important to keep. However, if we could remove some of those, it'd be really easy to convert over! |
I imagine with a solid block of time I could knock this out pretty quickly. Might be good to rewrite some of the logic anyways. Just gotta find that solid block of time ;) I've also been thinking about rewriting the plugin to use a custom text editor view, which would take a lot more work but support a lot of the customization folks have been asking for. I'd really love to explore that further too! |
A lot of Atom is moving to ES6 and I have a few ideas on stuff to contribute, but I vastly prefer working in ES6 to CoffeeScript because the linter catches so much more stuff. I'll be glad to do the boring conversion and submit a PR if you're interested.
The text was updated successfully, but these errors were encountered: