graphql / graphiql

GraphiQL & the GraphQL LSP Reference Ecosystem for building browser & IDE tools.
MIT License
16.08k stars 1.72k forks source link

Field coloring varies based on newlines #2354

Open plaa opened 5 years ago

plaa commented 5 years ago

Actual Behavior

Field coloring varies depending whether the opening brace or directive is on the same line as the type definition. Especially in more complex cases (the Bar example below) it may be preferable to have directives on their own lines.

See field: String below:

Screenshot 2019-05-06 at 14 33 44

Expected Behavior

Colors don't change based on locations.

Steps to Reproduce the Problem Or Description

Example code from screenshot:

type Foo1 {
  field: String
}

type Foo2
{
  field: String
}

type Bar1 @auth(rules: [
  { allow: groups, groups: ["Admin"] }
  { allow: groups, groups: ["Everyone"], operations: [read] }
]) {
  field: String
}

type Bar2
@auth(rules: [
  { allow: groups, groups: ["Admin"] }
  { allow: groups, groups: ["Everyone"], operations: [read] }
]) {
  field: String
}

Specifications

d3dc commented 4 years ago

This is definitely still an issue a year later.

Screen Shot 2020-05-13 at 10 13 32 AM Screen Shot 2020-05-13 at 10 13 26 AM
acao commented 4 years ago

yikes still an issue. new maintainer here! let's get this one.

my guess is the fix can be introduced to this block of the grammar, if anyone wants to start on a PR. otherwise I will get to this one in the next few weeks https://github.com/prisma-labs/vscode-graphql/blob/6bdfa89e63f737134b2d9a6a4e8959092b0e2fbf/grammars/graphql.json#L58

hkumar0132 commented 3 years ago

I would like to work on this issue if it's still open @plaa I have been doing web development in javascript for a year now and would like to enter the open source world.

plaa commented 3 years ago

@hkumar0132 I am unaware of it being fixed. I have moved to another project and am no longer using GraphQL, so I cannot verify. Probably @acao can comment.

hkumar0132 commented 3 years ago

@plaa Thanks for the response. Just wanna know if it's a good issue to solve for a beginner?

acao commented 3 years ago

@hkumar0132 possibly? if you’re familiar with regular expressions then definately. all these issues labelled “grammar” will just involve modifying the expressions in the grammar json files. here’s where to get started:

https://github.com/graphql/vscode-graphql#development

abc-moviestarplanet commented 2 years ago

I just spent some hours to see if I could learn something about grammars and be able to fix it. With my very limited knowledge, what I found out was that as soon as the { is in a new line, it gets the meta.selectionset.graphql scope.

So if I modify the graphql-operation-def entry in the grammar file, to include #graphql-type-object before #graphql-selection-set then it all seemed to work fine.

Unfortunately I am unaware of side effects of this change and probably will need several more hours to learn grammars, so I'm hoping it at leasts helps someone else.

"graphql-operation-def": {
      "patterns": [
        { "include": "#graphql-query-mutation" },
        { "include": "#graphql-name" },
        { "include": "#graphql-variable-definitions" },
        { "include": "#graphql-directive" },
        { "include": "#graphql-type-object" },  // added this
        { "include": "#graphql-selection-set" }
      ]
    },
acao commented 2 years ago

I really appreciate this @abc-moviestarplanet! I will test this out some more but this looks like it's what we need to fix this issue. My few fixes to the grammar files since taking over have happened similarly.