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

Add support for gql template literal #604

Merged
merged 1 commit into from Nov 3, 2018

Conversation

Projects
None yet
3 participants
@Ingramz
Contributor

Ingramz commented Oct 14, 2018

Closes #429

cc @50Wliu

Add support for gql template literal
Closes #429

Co-Authored-By: Tony Xiao <tonyxiao@users.noreply.github.com>

@50Wliu 50Wliu merged commit fdd1aac into atom:master Nov 3, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@FluorescentHallucinogen

This comment has been minimized.

FluorescentHallucinogen commented Dec 2, 2018

@50Wliu @Ingramz It seems it doesn't work. Could you please check?

@Ingramz

This comment has been minimized.

Contributor

Ingramz commented Dec 2, 2018

@FluorescentHallucinogen for me it works as intended. You'll need to have a third party graphql package installed. And make sure that you don't use the new Tree-Sitter parser as it is not supported there (yet) (uncheck Settings -> Core -> Use Tree Sitter Parsers). Then restart editor/reopen file and it should work.

@FluorescentHallucinogen

This comment has been minimized.

FluorescentHallucinogen commented Dec 2, 2018

@Ingramz

FYI, syntax highlighting of GraphQL inside <script type="application/graphql"> tags in *.html files works like a charm out of the box without any third-party packages thanks to https://github.com/atom/language-html/pull/191/files. E.g.:

<html>
  <apollo-query data="{{data}}" variables="[[variables]]">
    <script type="application/graphql">
      query($search: String, $currentPosition: Int) {
        posts(
          where: { OR: [{ title_contains: $search }, { content_contains: $search }] }
          orderBy: updatedAt_DESC
          skip: $currentPosition
          first: 50
        ) {
          id
          updatedAt
          title
          content
          author(where: { isAdmin: true }) {
            id
            name
            isAdmin
          }
        }
      }
    </script>
  </apollo-query>

  <!-- Other HTML elements -->
</html>

Is it possible to do the same (without third-party packages), but for tagged template literals?

@Ingramz

This comment has been minimized.

Contributor

Ingramz commented Dec 2, 2018

@FluorescentHallucinogen it looks like inside HTML grammar, someone has repurposed the CoffeeScript grammar for GraphQL which, while it 'works', is not technically correct.

How it looks like when CoffeeScript is used (notice that type declarations are styled differently):

query($search: String, $currentPosition: Int) {
  posts(
    where: { OR: [{ title_contains: $search }, { content_contains: $search }] }
    orderBy: updatedAt_DESC
    skip: $currentPosition
    first: 50
  ) {
    id
    updatedAt
    title
    content
    author(where: { isAdmin: true }) {
      id
      name
      isAdmin
    }
  }
}

Compare this to GraphQL grammar (Github uses the same community package):

query($search: String, $currentPosition: Int) {
  posts(
    where: { OR: [{ title_contains: $search }, { content_contains: $search }] }
    orderBy: updatedAt_DESC
    skip: $currentPosition
    first: 50
  ) {
    id
    updatedAt
    title
    content
    author(where: { isAdmin: true }) {
      id
      name
      isAdmin
    }
  }
}

It is certainly possible to swap it out to CofeeScript for the template literals too, but it would be more appropriate to migrate the script tags to leverage the community package as well in order to introduce better uniformity.

The fact that Github uses this grammar, means that this package already has the community's blessing and one should not be afraid having to use that package if they wish to use GraphQL in Atom.

@FluorescentHallucinogen

This comment has been minimized.

FluorescentHallucinogen commented Dec 3, 2018

@Ingramz

Thank you for the detailed explanation!

It wouldn't be better to change CofeeScript grammar to GraphQL grammar in https://github.com/atom/language-html/pull/191/files? Could you please make PR?

@FluorescentHallucinogen

This comment has been minimized.

FluorescentHallucinogen commented Dec 3, 2018

@Ingramz Thanks for the PR. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment