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 syntax highlighting #259

Merged
merged 8 commits into from
Jun 18, 2023

Conversation

tmillr
Copy link
Member

@tmillr tmillr commented May 23, 2023

These changes might require multiple PR's in order to: quickly deliver highlight fixes, achieve cleaner/smaller pr's, etc.. Feedback and suggestions are welcomed!

It may be possible to refactor all of the palette/* files into a single file? Although I'm not sure if this is something that is wanted or desired, plus after all is said and done, there may remain some custom differences/tweaks among the different colorschemes when it comes to the non-syntax colors/highlights within these files? So for now, I am thinking that it may be best to simply put such questions (of refactoring, etc.) off until later, and in the meantime just develop a new file/module containing all of the common mappings (from hl-group to primer primitives color) that will then be required() within each of the palette/* files.

Todo's, Notes, Ideas, Considerations, Etc.

  • Handle rgb() and rgba() color strings (not applicable to pl syntax colors)
  • Interaction with user config and overrides
  • Interaction with colorscheme compilation?
  • Tests (at least manual testing/debugging for now, but ultimately proper tests ideally)
  • Go over TODO comments
  • Relocate + rename newly introduced types as needed
  • Update CHANGELOG.md
  • Update other docs
  • Cleanup commits and commit messages
  • Cleanup PR description

wakatime

See #252

@tmillr tmillr mentioned this pull request May 23, 2023
9 tasks
@tmillr tmillr force-pushed the fix-incorrect-highlights branch 5 times, most recently from 3f74e75 to e73b01b Compare May 28, 2023 11:20
@tmillr
Copy link
Member Author

tmillr commented May 28, 2023

Should be ready to merge soon! 👍 (next 2-3 days)

@tmillr tmillr force-pushed the fix-incorrect-highlights branch 2 times, most recently from 0d9adb6 to 0e00d08 Compare May 29, 2023 22:59
@tmillr tmillr changed the title fix(palette): incorrect colors/highlights fix: incorrect syntax highlighting May 31, 2023
@tmillr tmillr force-pushed the fix-incorrect-highlights branch 2 times, most recently from 3cd8e14 to 43a9da1 Compare May 31, 2023 21:45
@tmillr tmillr marked this pull request as ready for review June 1, 2023 15:11
@tmillr
Copy link
Member Author

tmillr commented Jun 1, 2023

@ful1e5 I think this one is pretty much ready (although it depends on #273 so that should be merged first). I think that further refinements such as tweaking highlights even further, refactoring, or addressing less commonly used languages, etc., can come later if needed. I didn't touch the CHANGELOG.

This pr ended up touching many files so it may be easier to view the diffs one commit at a time. Thanks

@tmillr tmillr force-pushed the fix-incorrect-highlights branch 2 times, most recently from 28e1972 to 339564c Compare June 1, 2023 18:12
Comment-out several redundant hl-group links in:

- lua/github-theme/group/syntax.lua
- lua/github-theme/group/modules/treesitter.lua

These are redundant because they specify links which are identical to
Neovim defaults (e.g. `@string --> String`, `@function.call -->
@function`, etc.). They are mimicking default links that Neovim already
ships and starts-up with.

Colorschemes may break Neovim's default links arbitrarily, and `:hi clear`
restores them. Since we don't know if/which default links have been
broken by the previous colorscheme when loading ours, we should restore
these default links—either with `:hi clear`, or manually—before ours is
loaded. This already appears to be happening within compiled
colorschemes (i.e. `hi clear` gets embedded within them). Furthermore,
colorscheme compilation happens automatically by default and cannot be
disabled.
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
Copy link
Member

@ful1e5 ful1e5 left a comment

Choose a reason for hiding this comment

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

Please make the necessary changes to README.md#Configuration.

@tmillr tmillr requested a review from ful1e5 June 6, 2023 14:43
@tmillr
Copy link
Member Author

tmillr commented Jun 6, 2023

Please make the necessary changes to README.md#Configuration.

Oops. Sorry for missing that.

See cb601d80975e57447b8bf94a2c2f73e99968ddcd

@tmillr tmillr requested a review from ful1e5 June 17, 2023 10:53
@ful1e5 ful1e5 merged commit 777851c into projekt0n:main Jun 18, 2023
17 checks passed
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

Successfully merging this pull request may close these issues.

None yet

2 participants