-
-
Notifications
You must be signed in to change notification settings - Fork 104
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: incorrect colors/highlights #252
Comments
Hi, thanks for opening the issue. I've been trying to use this theme for months but wasn't able to because the coloring was off 😞 . Thanks for getting on this and I would be willing to pay for a working version of this theme if needed. |
Syntax highlighting is still not 100% accurate (due mostly to highlighting differences between different languages which need particular consideration), however, the accuracy of the highlights have been improved, both overall, and in terms of common languages such as: rust, ruby, ecma, c, c#, go, html, css, make, python, and lua. Fixes projekt0n#252
Syntax highlighting is still not 100% accurate (due mostly to highlighting differences between different languages which need particular consideration), however, the accuracy of the highlights have been improved, both overall, and in terms of common languages such as: rust, ruby, ecma, c, c#, go, html, css, make, python, and lua. Fixes projekt0n#252
Syntax highlighting is still not 100% accurate (due mostly to highlighting differences between different languages which require particular consideration), however, the accuracy of the highlights have been improved, both overall, and in terms of common languages such as: rust, ruby, ecma, c, c#, go, html, css, make, python, and lua. Fixes projekt0n#252
Syntax highlighting is still not 100% accurate (due mostly to highlighting differences between different languages which require particular consideration), however, the accuracy of the highlights have been improved, both overall, and in terms of common languages such as: rust, ruby, ecma, c, c#, go, html, css, make, python, and lua. Fixes projekt0n#252
Syntax highlighting is still not 100% accurate (due mostly to highlighting differences between different languages which require particular consideration), however, the accuracy of the highlights have been improved, both overall, and in terms of common languages such as: rust, ruby, ecma, c, c#, go, html, css, make, python, and lua. Fixes projekt0n#252
Syntax highlighting is still not 100% accurate (due mostly to highlighting differences between different languages which require particular consideration), however, the accuracy of the highlights have been improved, both overall, and in terms of common languages such as: rust, ruby, ecma, c, c#, go, html, css, make, python, and lua. Fixes projekt0n#252
Syntax highlighting is still not 100% accurate (due mostly to highlighting differences between different languages which require particular consideration), however, the accuracy of the highlights have been improved, both overall, and in terms of common languages such as: rust, ruby, ecma, c, c#, go, html, css, make, python, and lua. Fixes projekt0n#252
Syntax highlighting is still not 100% accurate (due mostly to highlighting differences between different languages which require particular consideration), however, the accuracy of the highlights have been improved, both overall, and in terms of common languages such as: rust, ruby, ecma, c, c#, go, html, css, make, python, and lua. See projekt0n#252
Syntax highlighting is still not 100% accurate (due mostly to highlighting differences between different languages which require particular consideration), however, the accuracy of the highlights have been improved, both overall, and in terms of common languages such as: rust, ruby, ecma, c, c#, go, html, css, make, python, and lua. See projekt0n#252
Syntax highlighting is still not 100% accurate (due mostly to highlighting differences between different languages which require particular consideration), however, the accuracy of the highlights have been improved, both overall, and in terms of common languages such as: rust, ruby, ecma, c, c#, go, html, css, make, python, and lua. See projekt0n#252
Syntax highlighting is still not 100% accurate (due mostly to highlighting differences between different languages which require particular consideration), however, the accuracy of the highlights have been improved, both overall, and in terms of common languages such as: rust, ruby, ecma, c, c#, go, html, css, make, python, and lua. See projekt0n#252
Thanks! #259 has been merged now, so most of the highlights should be fixed for several popular/common languages. Other languages should see some improvement as well. There are links on the project home page if you'd like to support it and/or the owner/maintainer. Also, I have a sponsor button on my profile if you'd like to support me or my work on this issue (just a disclaimer though, I'm not officially apart of this project and am a relatively new/recent contributor to it). You can try installing or updating the plugin to test out the new changes if you'd like. However, I personally have had some issues with getting this plugin to update and it appears to have something to do with the compilation/caching mechanism, so there may be a bug (I'm not totally sure, I haven't done a thorough investigation or yet heard if others have experienced the same). So, after you update the plugin, you might need to manually delete the cache (and then restart neovim) for the new changes/updates to take effect. In that case, you can locate the cache directory that needs to be removed using: echo "$(nvim -i NONE -u NONE -n --headless --cmd 'echo stdpath("cache")' --cmd 0cq 2>&1)/github-theme" Feel free to leave feedback here if you run into this issue as well, or run into any other issues regarding the accuracy of the highlights 👍. |
Gave you a sponsor, keep up the good work! |
@AlexXi19 Oh awesome! Thank you for being so kind! |
Related: #237 #256
At this time, it appears that much of the incorrect highlighting has to do with that of syntax items (i.e. syntax highlighting). I have some ideas on what I think is the most optimal way to go about fixing these incorrect highlights, and I thought that this belonged in its own issue, so I will pitch them here. This issue may also serve as a tracking issue and central location for tracking any progress made on this topic.
I'm still trying to learn and figure out the structure of this repo and understand how everything works (as time allows), so do correct me if I'm wrong, but currently it appears that all of the colors are being set manually, by-hand, for each theme/colorscheme? This is somewhat tedious, and does not make it easy to adapt to ongoing changes made upstream. I think that there's a better way to do this that will lead to easier maintenance and adaptation to upstream changes. GitHub actually regularly maintains, builds and distributes all of their themes as a public npm package under the permissible MIT license. Within this package, there are color/highlight definition files available in multiple formats (json, ts, css, etc.) for each theme. This means that we can easily automate the process of obtaining these colors in CI; however, this won't solve everything:
prettylights
highlight groups to Neovim's.=
is highlighted using the highlight group forconstant
, although in Lua it is highlighted using the highlight group forkeyword
. Furthermore, for most if not all of the languages, variables simply go unhighlighted even though there is avariable
color definition/highlight group defined. Etc. etc.These are all things that we have to consider when determining the correct mappings from GitHub
prettylights
highlight group to Neovim highlight group.Anyway, the following is an overview of what I think should ultimately be done.
1. CI
Introduce a GitHub workflow which runs regularly (e.g. daily) (and optionally, on every push or pr as well) to pull in the highlight groups/color definitions for each theme from the npm pkg. The workflow goes something like this:
2. Fix Highlighting
Map all of GitHub's
prettylights
highlight groups to the appropriate Neovim highlight group. This can be achieved in Lua or JavaScript (both languages can parse json into objects/tables), and either at runtime or within CI (if done at runtime we'd just use Lua). In theory, the determination of these mappings could probably be automated as well, but it would probably take alot of time and be alot of work; for now, I'd suggest to just determine the correct mappings manually/by-hand (this only needs to be done one time and does not need to be repeated for each theme as it appears that the mappings from syntax items to GitHub's highlight groups are the same regardless of which theme is in use — i.e. it is only the colors of these highlight groups that change). Getting all of these mappings figured out and completed correctly might take some time, so I'd suggest that any such updates are applied incrementally via multiple commits (while in the meantime, any Neovim highlight groups which haven't been considered yet simply continue to use their current colors/definitions as already currently defined/specified within the palette files). This way, users will be able to benefit from any corrected highlights ASAP instead of having to wait for the whole thing to be finished (all mappings to be determined). This step isn't entirely trivial and straightforward for several reasons, and as mentioned previously, different languages can sometimes use different highlight groups for the same kind of syntax item.3. Misc
If feasible, and once all of the previous steps have been completed and most or all of the colors have been corrected, consider:
(Optional) Refactor all of the palette files into a single file.
(Optional) Repair/retain Neovim's default highlight-group links (i.e. the links from treesitter to non-treesitter highlight groups that Neovim ships with by default) and/or making the theme compatible with vim's builtin, non-treesitter syntax highlighting. This one is not nearly important as the others—I'm merely including it here so that the idea is documented—however, keeping this in mind and retaining the default links may potentially ease the mapping process above.
References & Useful Links
dist/json
dir)The text was updated successfully, but these errors were encountered: