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

fix: incorrect colors/highlights #252

Open
7 of 9 tasks
tmillr opened this issue May 16, 2023 · 4 comments
Open
7 of 9 tasks

fix: incorrect colors/highlights #252

tmillr opened this issue May 16, 2023 · 4 comments

Comments

@tmillr
Copy link
Member

tmillr commented May 16, 2023

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:

  1. Neovim's highlight groups do not match GitHub's, therefore we will need some way of consistently and correctly mapping from GitHub's prettylights highlight groups to Neovim's.
  2. Not even considering Neovim, GitHub's own highlight group names, or rather how they are applied/mapped to actual syntax items, can seem confusing at times. For example, in most languages = is highlighted using the highlight group for constant, although in Lua it is highlighted using the highlight group for keyword. Furthermore, for most if not all of the languages, variables simply go unhighlighted even though there is a variable 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:

  1. Install/update the npm package.
  2. Copy just the json files containing the color defs for each theme into this repo (we don't need the entire pkg).
  3. (Optional) Convert the json to Lua
  4. Check for changes/differences, and if there are any, open a pr so they can be vetted and then merged/committed into this repo.
  5. Now, all of the exact colors are available within the repo, even at runtime, for internal use. Better yet, any changes made upstream are automatically tracked and easily incorporated into this repo. We can then parse or require these in order to build (either within CI, or at runtime on-the-fly) up the final "specs". We could also make these part of the public api for convenience (available to users for customization purposes, or for anything else just in case at any point they want convenient access to them).

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

@ful1e5 ful1e5 pinned this issue May 18, 2023
@AlexXi19
Copy link

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.

tmillr added a commit to tmillr/github-nvim-theme that referenced this issue May 23, 2023
tmillr added a commit to tmillr/github-nvim-theme that referenced this issue May 23, 2023
tmillr added a commit to tmillr/github-nvim-theme that referenced this issue May 26, 2023
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
tmillr added a commit to tmillr/github-nvim-theme that referenced this issue May 28, 2023
tmillr added a commit to tmillr/github-nvim-theme that referenced this issue May 28, 2023
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
tmillr added a commit to tmillr/github-nvim-theme that referenced this issue May 28, 2023
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
tmillr added a commit to tmillr/github-nvim-theme that referenced this issue May 28, 2023
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
tmillr added a commit to tmillr/github-nvim-theme that referenced this issue May 29, 2023
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
tmillr added a commit to tmillr/github-nvim-theme that referenced this issue May 29, 2023
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
tmillr added a commit to tmillr/github-nvim-theme that referenced this issue May 31, 2023
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
tmillr added a commit to tmillr/github-nvim-theme that referenced this issue May 31, 2023
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
tmillr added a commit to tmillr/github-nvim-theme that referenced this issue Jun 1, 2023
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
tmillr added a commit to tmillr/github-nvim-theme that referenced this issue Jun 1, 2023
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
tmillr added a commit to tmillr/github-nvim-theme that referenced this issue Jun 2, 2023
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
@tmillr
Copy link
Member Author

tmillr commented Jun 18, 2023

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.

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 👍.

@AlexXi19
Copy link

Gave you a sponsor, keep up the good work!

@tmillr
Copy link
Member Author

tmillr commented Jun 22, 2023

Gave you a sponsor, keep up the good work!

@AlexXi19 Oh awesome! Thank you for being so kind!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants