jparise / vim-graphql

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

Only set 'syn-sync' when it is the main syntax #73

Closed pjio closed 3 years ago

pjio commented 3 years ago

This PR tries another approach and sets a flag in the files which include the nested syntax.

I tested with nvim v0.4.3 and disabled my other plugins:

:syn sync shows for graphql files as intended: syncing starts 500 lines before top line

For php files :syn sync show the original: syncing starts 2147483647 lines before top line

I also tested that the nested syntax is still working. Very nice feature to have!

pjio commented 3 years ago

I changed the condition to if !get(b:, 'graphql_nested_syntax')

To write a unit test is more tricky than I first thought. The problem is to set minlines to a predictable value before the buffer is loaded.

I had some success with adding this to test/php/default.vader

Execute (Settings assertions):
  redir @a | syntax sync | redir END
  AssertEqual 'syncing starts 2147483647 lines before top line', @a

But on the CI there are different defaults: image

For now I think it's not easily possible.

pjio commented 3 years ago

I added a test which checks for the '500' in the syntax setting.

pjio commented 3 years ago

The test checks now for the exact value which would be set if there is a regression. The downside is, the test has to be changed whenever the value "500" is changed or the string format changes. The upside is, it shouldn't fail as a false positive. I had to store the result of match() in a local variable because Assert didn't handle the comparison as a boolean expression (I assume a limitation of Vader..)

jparise commented 3 years ago

Thanks for this, @pjio! I appreciate you digging in to the issue and producing a generalized solution to the problem.