fix(vscode-apollo): Comments not being highlighted correctly

[kevin-lindsay-1]

Given the following:

launches(
  """
  The number of results to show. Must be >= 1.
  """
  pageSize: Int = 20
[Completely blank line; no whitespace characters aside from \n]
  """
  If you add a cursor here, it will only return results _after_ this cursor
  """
  after: String = "hello nurse"
): LaunchConnection!

The second comment would be detected and highlighted as a string, not a comment. My formatter removes any additional spaces at the end of lines, so this was a visual nuisance.

Changed from (?=.) to (?=[\n.] because . doesn't take line breaks into consideration.

Hopefully that's the correct place to change this; never created a textmate syntax before.

[apollo-cla]

@kevin-lindsay-1: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

[trevor-scheer]

Thanks for submitting this @kevin-lindsay-1! Would it be reasonable to say that someone may want any number of whitespace characters instead of just a single line break?

My initial reaction is to expand your change to : \\s*(?=.) - which should greedily consume all whitespace until it runs into the previous expected behavior of running into any character. I'm also unfamiliar with the syntax itself, but this pattern of "eat all the whitespace" seems to be pretty prevalent everywhere else as well.

[kevin-lindsay-1]

I'm using graphql-code-generator, which does parse the default argument regardless of the whitespace; I simply submitted this PR in as minimal a fashion as possible.

I could theoretically comb through the syntax highlighting rules and try to fixup anything that looks like it wouldn't parse correctly.

I recall running into issues where trying to greedily eat whitespace wasn't working for me, likely due to rule conflicts/a general lack of knowledge of textmate 1.x, but I don't recall if I tried using that exact expression.

As far as I'm concerned, a brake-fix solution would at least prevent the highlighting from failing in my current code, and in future I would probably try to create a robust mock schema to try to break things, throwing it into a tool like regex101/regexr in order to judge the performance; I've never tried to look for a testing/performance analysis library for syntax highlighting before, but perhaps I'll dig into it further.

[kevin-lindsay-1]

The pattern of "eat all whitespace" can be completely valid, however the exact way you write the expression can sometimes double the evaluation time (or more). I don't currently know how VS Code runs its syntax highlighting in order to improve performance (if any), or if it's on the textmate end (or both).

[kevin-lindsay-1]

\\s*(?=.) doesn't work (just tested), as it's trying to eat all the whitespace before the end character, but doesn't recognize a newline as a valid character to end the variable definition itself. I tried a few other combinations out, as well, but I think the detection issues might be going slightly deeper than this. I used [\n.], because a newline is the only character not covered by ., other than something more complicated like (\r\n|\n|.)

[trevor-scheer]

@kevin-lindsay-1 I appreciate the thorough commentary, as well as exploring my suggestion. I'll go ahead and merge this 😄 thanks again for opening the PR! Just an FYI, the fix will be available in master immediately but we may not re-publish the extension for a bit.