wooorm / starry-night

Syntax highlighting, like GitHub
MIT License
1.45k stars 30 forks source link

Highlight differences with GitHub.com #10

Closed sebastinez closed 1 year ago

sebastinez commented 1 year ago

I found out that in JavaScript and TypeScript the name of a const is assigned a span with a pl-c1 class which doesn't match with what GitHub does in their highlighting, where they mark it as a pl-s1.

Current starry-night with source.js.js grammar

Bildschirm­foto 2022-12-13 um 07 27 28

Current GitHub rendering of the same file

Bildschirm­foto 2022-12-13 um 07 27 51

It seems that the name of the pattern is constant.other at least in Javascript. I'm still trying to get into the codebase but am I right that the mapping between grammar pattern names and css classes are done here? https://github.com/wooorm/starry-night/blob/main/lib/theme.js

In that case I would propose to add, to /lib/theme.js

  "constant.other": "pl-s1"

Also there is some different behaviour in method calls e.g. string.slice() slice is also assigned a pl-c1 instead of pl-en.

wooorm commented 1 year ago

https://github.com/wooorm/starry-night#what-is-prettylights

This is because GH uses TreeLights for some languages.

sebastinez commented 1 year ago

This is because GH uses TreeLights for some languages.

Thanks @wooorm I'm aware of that, but I think we should be able to mimic some syntax decisions of TreeLights in PrettyLights? Or is there something that would break in some other language maybe? Or better said the mapping between pattern names and css classes is not to be touched with?

wooorm commented 1 year ago

Or better said the mapping between pattern names and css classes is not to be touched with?

Correct. They are not to be touched.

It’s explained more in the next paragraph:

starry-night does what PrettyLights does, not what TreeLights does. I’m hopeful that that will be open sourced in the future and we can mimic both.

GH has three things:

    -----
    | x |
    +---+
    |   |
----+   +---|
| y |   | z |
-----   -----

…where y is pretty lights and z is tree lights.

This project implements y, which uses grammars of one kind, and needs dependencies of one kind to handle those grammars. z has different grammars and different dependencies.

I don’t think the solution is to change grammars in y. I think the solutions is to implement z. Perhaps then the next step then is to make this project like x. However, that comes at the cost of doubling dependencies included in this project. Which might not make sense for everyone.

sebastinez commented 1 year ago

Thanks @wooorm for the explanation, will close the issue due to your great explanation, thanks for your time 🙇

sebastinez commented 1 year ago

Hey @wooorm I still wanted to ask you if you could give a reason why you don't want to touch the mapping between regex capture groups and css classes? It seems like the best place to add more granularity and fix some weird highlighting.

wooorm commented 1 year ago

why you don't want to touch the mapping between regex capture groups and css classes

I didn’t say this I believe. If there are bugs in that, they can be fixed.

You are talking about how GH uses treelights. Which is a different project. This project is currently about prettylights. The solution is not to hack around prettylights to make it look like treelights. The solution is to implement treelights. Or upstream your changes: https://github.com/atom/language-javascript.

It seems like the best place to add more granularity and fix some weird highlighting.

There should not be weird highlighting.