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.

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

Fixed ECMAScript private member highlighting #10554

Merged
merged 1 commit into from
Apr 27, 2024

Conversation

lizclipse
Copy link
Contributor

It's bugged me that private members haven't been highlighted in editor I've used so far, so I've added the queries to allow actually restyling. Currently, they're targeted as a .private child under variable.other.member and function.method so that they automatically take on the existing highlighting properly while being individually style-able. This was a personal preference, since it would match other langs which don't colour public & private methods/field differently while allowing the unique syntax of JS to be taken advantage of, however there's 2 other options that could also work:

  • Remove all the .private parts and match other langs exactly
  • Rename to not be a child and instead standalone (eg variable.other.private-member) to preserve the current lack of styling

Example code that shows what syntax it now highlights:

export class Test {
  regularField: string = "working highlights";

  #privateField: string = "fixed with these changes";
}

Funnily enough, even GitHub doesn't handle private props properly. Not sure if that's an oversight or a stylistic choice, but it bugs the hell out of me.

@lizclipse
Copy link
Contributor Author

I have no idea what that test failure is about. Running it on my own mac I can't repro it and it's complaining about the Rust grammar, which I haven't touched.

@archseer
Copy link
Member

Github CI switched their macOS runners to ARM. We cleared the cache yesterday so the tests should pass now

Copy link
Member

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cotneit
Copy link
Contributor

cotneit commented Apr 25, 2024

I would drop .private, adding new keys feels unnecessary here

@lizclipse
Copy link
Contributor Author

@the-mikedavis Done

@cotneit I've made them child keys so that they can just be ignored, but my preference is to keep them because it allows colouring the private fields/methods slightly differently if you wish - eg in my custom theme I have private members the same colours with the saturation turned down a bit to make them distinct.

@the-mikedavis the-mikedavis added S-waiting-on-review Status: Awaiting review from a maintainer. A-language-support Area: Support for programming/text languages labels Apr 25, 2024
@pascalkuthe pascalkuthe merged commit bc03b6b into helix-editor:master Apr 27, 2024
6 checks passed
@lizclipse lizclipse deleted the fix-ecma-private-idents branch April 27, 2024 20:08
Vulpesx pushed a commit to Vulpesx/helix that referenced this pull request Jun 7, 2024
Chirikumbrah pushed a commit to Chirikumbrah/helix that referenced this pull request Jun 15, 2024
smortime pushed a commit to smortime/helix that referenced this pull request Jul 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-language-support Area: Support for programming/text languages S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants