jparise / vim-graphql

A Vim plugin that provides GraphQL file detection, syntax highlighting, and indentation.
MIT License
490 stars 25 forks source link

Fix reason syntax #60

Closed jfrolich closed 4 years ago

jfrolich commented 4 years ago

The change was incorrect and due to the change, the highlighting didn't work anymore. We cannot match within a multiline string because it needs to match an "extension point" everything that happens inside that extension point is basically taken care of by the language extension. I also removed the extra items in contains, I am not sure why that needs to be there because it contains no reason at all, just GraphQL syntax. Also, the current change is independent of the reason-syntax you use.

jparise commented 4 years ago

@jfrolich could you also update the ReasonML tests for this change?

I don't know much ReasonML, but I thought that the string containing GraphQL syntax within the extension point still allowed interpolation and other string template features. Do I have that wrong?

jfrolich commented 4 years ago

I don't know much ReasonML, but I thought that the string containing GraphQL syntax within the extension point still allowed interpolation and other string template features. Do I have that wrong?

Nope the raw contents are run through a pre-compilation step.

I tried to fix it. But this line is still failing:

(2/2) [EXECUTE] (X) 'reasonMultilineString' should be equal to 'graphqlName'

I don't know where it got reasonMultilineString from (it's not mentioned anywhere in the source).

jparise commented 4 years ago

I don't know much ReasonML, but I thought that the string containing GraphQL syntax within the extension point still allowed interpolation and other string template features. Do I have that wrong?

Nope the raw contents are run through a pre-compilation step.

Got it.

I tried to fix it. But this line is still failing:

(2/2) [EXECUTE] (X) 'reasonMultilineString' should be equal to 'graphqlName'

I don't know where it got reasonMultilineString from (it's not mentioned anywhere in the source).

That assertion can be removed. It's from the vim-reasoml plugin.

I have a better handle on what's required to make this work as you intended now and can take it from here. I'll try to get this fixed up later today.

jfrolich commented 4 years ago

👍 Thanks so much!