Skip to content
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

Use GraphQL grammar to highlight its script block #221

Merged
merged 1 commit into from Jan 10, 2019

Conversation

Projects
None yet
5 participants
@Ingramz
Copy link
Contributor

Ingramz commented Dec 3, 2018

Description of the Change

As discovered in atom/language-javascript#604 (comment), the script block for GraphQL in HTML grammar uses CoffeeScript as its grammar. While it has been seemingly working 'fine' for the time being, we should encourage using the appropriate GraphQL grammar for more accurate highlighting instead.

Various grammars in Atom already make use of the community package (JavaScript, Ruby, GFM), with currently this one being the odd-one-out.

Alternate Designs

None considered.

Benefits

More accurate highlighting for GraphQL in script tags in both Atom and GitHub.

Possible Drawbacks

We lose out-of-the-box 'support' for GraphQL in this case. This should be noted in release notes that users will need to install the appropriate language-graphql community package instead.

Applicable Issues

atom/language-javascript#604

cc @FluorescentHallucinogen @50Wliu

@rsese

This comment has been minimized.

Copy link
Member

rsese commented Dec 5, 2018

Thanks @Ingramz! Someone from the team will take a look as soon as they can.

@FluorescentHallucinogen

This comment has been minimized.

Copy link

FluorescentHallucinogen commented Dec 15, 2018

Any progress?

@rsese

This comment has been minimized.

Copy link
Member

rsese commented Dec 17, 2018

@FluorescentHallucinogen - with the current workload of the team, we can't promise any particular timeframe at the moment for reviews. But once someone can take a look, there will be comments here in the pull request.

@50Wliu

50Wliu approved these changes Dec 18, 2018

Copy link
Member

50Wliu left a comment

👍 from me, as long as the change is included in the release notes.

@daviwil
Copy link
Member

daviwil left a comment

Thanks @Ingramz!

@daviwil daviwil merged commit ef8f62b into atom:master Jan 10, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.