Skip to content

Switch to bundled grammar #1382

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

Merged
merged 3 commits into from
Dec 8, 2017
Merged

Switch to bundled grammar #1382

merged 3 commits into from
Dec 8, 2017

Conversation

lierdakil
Copy link
Collaborator

  • All compiled assets are included (atom packages are git tags and hence the built files need to be a part of the source control)

This is in response to #1374

Long story short, we should probably use bundled grammar. It's very nearly the same as ours (also converted from Microsoft's TextMate grammar), and there's little point in being stubborn and trying to maintain our own.

Scopes are also mostly the same, so the only difference, really, is activation hooks.

@lierdakil lierdakil requested a review from guncha December 5, 2017 20:54
@guncha
Copy link
Contributor

guncha commented Dec 5, 2017

Hehe, this is what I see when I search for language-typescript:

image

Is this because I'm behind the latest Atom?

@guncha
Copy link
Contributor

guncha commented Dec 5, 2017

Yep, updated to 1.22.1 and it now shows as a bundled package (same repo, different version though). In principle, yes, it's a good idea to depend on it assuming Atom folks are going to keep it up to date.

Copy link
Contributor

@guncha guncha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's the grammar script that you can get rid of now as well as the npm script.

@lierdakil
Copy link
Collaborator Author

language-typescript's last release was less than 24 hours ago (although since it's bundled, it's only automatically updated together with Atom), besides they have (somewhat minimalist currently) typescript support for atom-ide, so I expect they intend to keep it up-to-date for the foreseeable future.

On another note, thanks for the tip, scripts/grammar.ts (and relevant line in package.json) removed.

@lierdakil lierdakil merged commit c12fea7 into master Dec 8, 2017
@lierdakil lierdakil deleted the use-builtin-grammar branch January 10, 2018 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants