Skip to content

Conversation

@jfrolich
Copy link
Contributor

@jfrolich jfrolich commented Sep 4, 2020

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.

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
Copy link
Owner

jparise commented Sep 6, 2020

@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
Copy link
Contributor Author

jfrolich commented Sep 7, 2020

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
Copy link
Owner

jparise commented Sep 7, 2020

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.

- Be more lenient with whitespace matches (more still to do)
- Remove the vim-reasonml-specific syntax check
- Remove the unnecessary 'filetype' setting
@jparise jparise merged commit 59ea49f into jparise:master Sep 7, 2020
@jfrolich
Copy link
Contributor Author

jfrolich commented Sep 8, 2020

👍 Thanks so much!

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants