Grammar generator mishandles name property with embedded scopes

[lierdakil]

Prerequisites

• [X] Put an X between the brackets on this line if you have done all of the following:
  • Reproduced the problem in Safe Mode: http://flight-manual.atom.io/hacking-atom/sections/debugging/#using-safe-mode
  • Followed all applicable steps in the debugging guide: http://flight-manual.atom.io/hacking-atom/sections/debugging/
  • Checked the FAQs on the message board for common solutions: https://discuss.atom.io/c/faq
  • Checked that your issue isn't already filed: https://github.com/issues?utf8=✓&q=is%3Aissue+user%3Aatom
  • Checked that there is not already an Atom package that provides the described functionality: https://atom.io/packages

Description

Grammar generator mis-handles name property with embedded scopes, for example:

https://github.com/atom/language-typescript/blob/1927633390e3c81eea4ed6309222dc863e4a0c1e/grammars/TypeScript.json#L799

FirstMate can't handle this syntax, and as a result, source span gets the class syntax--meta syntax--definition syntax--function syntax--ts entity syntax--name syntax--function syntax--ts. Notice the naked entity without synax-- prefix there in the middle.

This is brought over from https://github.com/TypeStrong/atom-typescript/issues/1379

Steps to Reproduce

1. Create TypeScript file with contents

   class SomeClass {
     someMethod() {}
   }
   

2. log-cursor-scope on someMethod

Expected behavior:

• source.ts
• meta.class.ts
• meta.method.declaration.ts
• meta.definition.method.ts
• entity.name.function.ts

Actual behavior:

• source.ts
• meta.class.ts
• meta.method.declaration.ts
• meta.definition.method.ts entity.name.function.ts

Reproduces how often: 100%

Versions

[:~] $ atom --version
Atom    : 1.23.1
Electron: 1.6.15
Chrome  : 56.0.2924.87
Node    : 7.4.0
[:~] $ apm --version
apm  1.18.11
npm  3.10.10
node 6.9.5 x64
atom 1.23.1
python 3.4.6
git 2.15.1

Additional Information

See also: https://github.com/Microsoft/TypeScript-TmLanguage/pull/545

[Nxt3]

This also happens with other Typescript scopes: https://discuss.atom.io/t/cant-seem-to-style-element-based-on-cursor-scope/51526

[lierdakil]

Bump. Is anyone actually responsible for this project at the moment?

[damieng]

@lierdakil The team was on a bug bash and retreat last month so we are somewhat behind on triage.

[lierdakil]

Any updates?

[damieng]

The grammar comes from upstream and is supposed to work in a variety of editors - from their README;

This repository contains TmLanguage files that are consumed by TypeScript editors and plugins such as Visual Studio Code, The TypeScript Sublime Plugin, Atom TypeScript, and possibly others.

I'm not aware that any other editor can handle spaces in the names correctly besides VSCode so either that upstream repository isn't the one we all want to use after all or they need to consider not closing the issue with "works in VSCode" :(

[lierdakil]

FWIW, Sublime seems to handle it correctly: while in Atom it's when it should be

(I'm not familiar enough with Sublime to make any informed claims, but it seems that it just uses whitespace as a scope delimiter, so if grammar yields a scope with whitespace, Sublime just treats that as two scopes)

[lierdakil]

Might it might be easier to fix on Atom side?.. At least I believe a quick fix should amount to a single-line change (didn't look at the sources yet though, so allow for a ± couple lines). A "correct" fix might prove to be a bit more involved, but probably not drammatically so.

[damieng]

The problem is if I hack the grammar files here then we're effectively detached from upstream unless we keep remembering to make those changes every time we pull...

Also not sure what the right fix would actually be. Sure they've put multiple grammar scope names in a name but there isn't an Atom-compatible syntax for that so we'd just have to choose one or come up with some kind of combo name.

Open to suggestions or PRs (should probably sync upstream again before anyone bothers with that)

[lierdakil]

I was more thinking along the lines of patching first-mate to handle scopes with spaces actually.

A technically "correct" fix on the grammar side would be a scope-within-a-scope. So essentially (and forgive me if it doesn't work, it was a while since I last touched grammar)

match: '...'
name: 'meta.definition.function.ts'
captures: 0: name: 'entity.name.function.ts'

... although it isn't nestable, or at least I can't say from the top of my head how it could be.

[lierdakil]

And by "nestable" I actually meant "can handle more than two scopes". Sorry, long day.