search

PowerShell / EditorSyntax

PowerShell syntax highlighting for editors (VS Code, Atom, SublimeText, TextMate, etc.) and GitHub!
MIT License
132 stars 44 forks source link

subexpression marker ($ in '$(') is in scope 'punctuation.definition.variable' #132

Closed msftrncs closed 3 years ago

msftrncs commented 6 years ago

Environment

Issue Description

The subexpression marker(the $ in '$(') is currently scoped in 'punctuation.definition.variable'. This causes no colorization in most schemes, such then cause colorization issues in double quoted text subexpressions.

Expected Behavior

I might think that it should be in a 'keyword' scope. To play around, I modified the VS Code powershell.tmLanguage.json file, in two places, to change the $ in '$(' to be scoped as 'keyword.other.subexpression.powershell'.

This seems to get the result with themes, causing it to colorize the same as the @ in '@(' or '@{' which also scope as a 'keyword'.

vexx32 commented 5 years ago

@TylerLeonhardt would be great to have something along these lines tbh, this is such a common thing in a lot of code I see and it can look really weird in strings at the moment. 😔

msftrncs commented 5 years ago

Related items: #106, #59

@vexx32, in a recent PR #156 commit, I have set the ${ and } of an interpolation (not a substatement) to scope as 'punctuation.section.embedded.begin/end.powershell'. Some editors/themes then have a highlight for that. See #59 above, I've been considering added a PR to get this handled... but I think I was running into the issue that substatement and interpolated substatement are handled in the same rule, would get same scopes, and so I would need to separate that out. That's what I did in PR #156, so I suppose that's inevitable. There was also some naming issues around some of those rules, which lead to confusion.

Would the punctuation.section.embedded scope be better than just keyword.other? I found the themes in VS Code supported the embedded scope for some other languages, but not PowerShell. Atom on the other hand appears to support the scope in all languages in the default theme.

It might be possible to apply both scopes to the interpolated version of the definition, then support in the theme can fallback to which ever is supported.

vexx32 commented 5 years ago

Hard to say from where I'm standing, I'm afraid. I'm not super familiar with how the different scopes are handled, but I'm painfully aware that a lot of themes have a bit of inconsistent support for many of the scope rules.

I guess it seems logical that if we can apply multiple that are sensible, at least one of them is likely to be covered by a given theme?

msftrncs commented 5 years ago

I've partially misspoke, I just found out that while VS Code's Monokai Dimmed does not highlight punctuation.section.embedded, the default Dark+/Light+ themes do.

My goal is still to try to post a small PR for this and related issues.

msftrncs commented 5 years ago

Disappointingly ATOM does not support specifying multiple scopes. This means that the tests in this project will fail since they are based on ATOM. :(

msftrncs commented 5 years ago

I think I found a work-around for the ATOM issue.

VS Code: image

ATOM: image