Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Nested Syntax Support #1878

Closed
CrossR opened this issue Jun 6, 2020 · 2 comments
Closed

Nested Syntax Support #1878

CrossR opened this issue Jun 6, 2020 · 2 comments
Labels
A-syntax-highlighting Area: Syntax Highlighting bug Something isn't working R-duplicate Resolved as a duplicate of an existing issue

Comments

@CrossR
Copy link
Member

CrossR commented Jun 6, 2020

The HTML syntax (seems) to include other syntaxes, which would make sense. I assume we don't support that since we get this:

image

Right now, where styling drops off when we hit the style tags. Removing that section returns it to normal (except the script tag, which I'm assuming is the same issue again).
@CrossR CrossR added bug Something isn't working A-syntax-highlighting Area: Syntax Highlighting labels Jun 6, 2020
@bryphe bryphe mentioned this issue Aug 14, 2020
Merged
bryphe added a commit that referenced this issue Aug 17, 2020
@bryphe
fix(theme/#1933): Scope matching precedence (#2301)
cbc1123 
__Issue:__ Our scope-selection strategy was not correct in some cases - particularly, the precedence in which we would apply the scopes.

For example, in #1933 - with the dracula theme - there'd be a case where a token would have the following textmate scopes:
- `support.function.console.js`
- `meta.function-call.js`
- `source.js`

For the dracula theme, there is a `source` selector that is intended to be a fallback - however, it was taking precedence over some of the other theme selectors that should've been applied, like `meta.function-call`.

This would cause the entire source file to be highlighted with the `source` scope selector, instead of the more specific ones.

__Defect:__ We apply selectors in reverse order (first, we apply any selectors matching `source.js`, and then `meta.function-call.js source.js`, etc...) - this gives us right precedence. However, if we had a match, we were failing to 'fall-back' to a more specific selector.

__Fix:__ Add a test case to reproduce, and fall back to override with more specific scopes.

This looks to fix a couple of related issues:
- #1933 - no syntax highlighting with OneDark / Dracula for some files
- #2006  - incorrect syntax highlighting for tsx files
- #1879  - html syntax highlighting not displayed
- #1878 - nested syntax highlights not shown

Thanks @CrossR for the helpful tool improvements in #2255 and @thismat for the investigation to narrow down the issue in #1933 !

`dracula` theme - __before__:

![image](https://user-images.githubusercontent.com/13532591/90297301-e2a6a000-de42-11ea-9380-178dc6345bf8.png)

`dracula` theme - __after__:

![image](https://user-images.githubusercontent.com/13532591/90297344-feaa4180-de42-11ea-999c-95936b4e3369.png)
@bryphe
Copy link
Member

bryphe commented Aug 27, 2020

We actually have nested syntax support:

image

but it happened that this same bug impacting #1933 (fixed in #2301): #2301 happened to affect nested syntaxes also (they were more likely to hit the issue in #1933 )

The main thing we're missing, though, is injection grammar support - tracked in #2118

@bryphe bryphe added the R-duplicate Resolved as a duplicate of an existing issue label Aug 27, 2020
@bryphe
Copy link
Member

bryphe commented Aug 27, 2020

I'll close this out as a duplicate of #1933, and use #2118 to track the injection grammar work

@bryphe bryphe closed this as completed Aug 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-syntax-highlighting Area: Syntax Highlighting bug Something isn't working R-duplicate Resolved as a duplicate of an existing issue
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@CrossR @bryphe