-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
fix(themes): Highlight .token.null
#1727
Conversation
7e14442
to
3d87bb6
Compare
It seems curious that JSON is the only language that has @RunDevelopment Thoughts? |
Well, PHP used Lines 53 to 58 in c5160a1
And there might be other languages that do the same. |
At this point in time, the only languages which have a
Depends on the language. Most use
I also think it's better for JSON to have null as a
|
It is a subset of ES2019+, as ES2019 now allows unescaped let str = "a
b"; // This string contains an unescaped Line Separator That said, I’d still like to fix highlighting of |
|
Agreed. I'd prefer to use a token we reuse across a lot of languages than handling a token used once or twice in a theme. I find it a little curious some of them are using Appreciate you raising this to our attention! |
Well, it's not that easy... But I think that null (or conceptually similar) should not be |
Fair enough; choosing something that makes sense for each language is fine. I'm admittedly less familiar with those languages, but I think a null token for JSON & SCSSS only doesn't really make sense. So we should update the JSON + SCSS languages to give |
I dunno, the |
Maybe the alias idea from @ExE-Boss is even better. I mean, we used these token names for quite some time now, so there might be some folks out there how incorporated |
I'm on board with that. |
Just for clarification: We are not going to change the themes now, are we? @ExE-Boss @mAAdhaTTah |
I’d like to fix the themes. |
@ExE-Boss With or without the aliases? Because after the aliases, there's nothing to fix. |
Well, even with the aliases, |
So you want to change the look of |
I feel like this question should have been asked a lot sooner, but why do you want |
After a day of thinking it over, I think it might be better to just highlight it using |
I'm personally opposed to changing the theme here, as I think handling a token in all the themes that only appears in two languages is unnecessary and the correct fix is the keyword change, as suggested. |
Agreed. After #1735 there's nothing left to fix. I'm closing this now. Feel free to reopen this if further discussion is necessary. |
The JSON syntax highlighting uses
.token.null
to stylenull
:prism/components/prism-json.js
Line 15 in 4362e42
This updates the Prism stylesheet to style it using the style for
.token.boolean
and.token.number
.