Changes to activate package only on Ruby file load #99
Conversation
|
Any ideas why all the tests are failing? Seems to be related to this change. |
|
Not right now, I'll check it out :) |
|
When you switch to an See here for an example. |
|
@ricardograca Now it should work :) |
|
Great! Looks good to me, but I'm not that familiar with this activation stuff yet, so I'll leave the final word to @Arcanemagus. |
There was a problem hiding this comment.
Sorry this took so long to get to! It got lost in my notifications 😞.
The original example that linter-jscs was based on used the same model it did where the activation promise was stored before activating the relevant language and opening a related file, then waiting on the package activation promise to complete, but if directly waiting on it works that's a nice simplification.
spec/linter-erb-spec.js
Outdated
| }); | ||
| ); | ||
|
|
||
| atom.packages.triggerActivationHook('language-ruby:grammar-used'); |
There was a problem hiding this comment.
The method I linked should work without this line, and as we already are reaching into the non-public API with the next line I'd rather this was removed.
(The opening of the good.erb file should have triggered this for us.)
There was a problem hiding this comment.
@Arcanemagus I tested it, but it does not activate for some reason
There was a problem hiding this comment.
Without this the tests time out when waiting for linter-erb since there is no language-ruby activation
There was a problem hiding this comment.
If that's the case then the file you are opening isn't being recognized as Ruby.
There was a problem hiding this comment.
The problem is that changing the code to be equal to linter-jscs example doesn't work, since the linter never activates
There was a problem hiding this comment.
After some debugging it seems like the fixture file doesn't correctly instantiate the editor, as the scope for the file is text.plain.null-grammar
|
@ricardograca Fair enough, but with the correct file type it recognizes a grammar and with just |
|
The problem here is, that I don't know how to debug Atom from this point onward. So I have no idea where the issue with plain |
|
I opened an upstream issue about this problem atom/language-ruby#203 @ricardograca I can't fix this by adding a new scope as the The problem here is that ERB files actualyl have no grammar at all in Atom :) They used to all be I could of course change the test to just activate |
This speeds up Atom startup
Why: * The correct grammar was not identified for `*.erb` files This change addresses the need by: * Renamed the file extensions * Fixed test setup to work without too much black API magic What side effects does this change have?: * None, AFAIK
Why: * Plain `.erb` files don't activate `language-ruby` package atm This change addresses the need by: * Adds new activation hook
|
@ricardograca I did a rebase from master, but still Travis fails with a missing glibc |
|
It seems a fix for that is on its way: atom/atom#14827 (comment) |
|
@ricardograca Could you schedule a rebuild? I'd hate to push an empty commit just to restart travis :) |
|
@deiga Done. It passed. |
|
@Arcanemagus Is this now up to par, or should some changes still be applied? |
|
Sorry it took so long and thanks for the contribution. We'll probably release a new version with these changes soon. |
|
🎉 This PR is included in version 1.1.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This speeds up Atom startup