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

refactor(catppuccin)!: reimplement basic hl groups #676

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Jint-lzxy
Copy link
Collaborator

WIP but mostly completed. Ref: #627.

TODO:

  • Fully support LSP semantic tokens (should be linked to basic hl)
  • Support all miscellaneous highlights (e.g., gitcommit*)

@Jint-lzxy Jint-lzxy mentioned this pull request Apr 25, 2023
12 tasks
Base automatically changed from 0.9 to main April 30, 2023 14:58
This commit reimplemented basic hl groups and link Treesitter's
highlights to these basic hl groups to reduce maintenance pressure
@Jint-lzxy Jint-lzxy force-pushed the refactor/treesitter-hl branch from 6c8ee1d to fc9fee2 Compare April 30, 2023 15:05
@nullchilly
Copy link
Contributor

nullchilly commented Apr 30, 2023

Could you port this PR to upstream if you have time? With better highlights decisions and specific language highlights (C/C++) are really well tested, I feel like it would be a waste to remain in a config repo.

Also our semantic tokens highlights reputation are under fire right now (Everything is just blue 😅) and the current treesitter highlights aren't much better, for example ["@method"] = { link = "Function" } makes much more sense

@Jint-lzxy
Copy link
Collaborator Author

@nullchilly Sure! In fact, I plan to open a PR upstream after this PR is merged. The plan is to adjust those treesitter highlights according to the color palette document & existing color presets and attempt to reduce maintenance pressure by linking as many hl groups as possible. But this PR may take more time than expected, as I'm trying to disentangle this "intertwined puzzle" - IMHO upstream support for treesitter is a bit chaotic. Since catppuccin/nvim@f079dda, it has gone through a lot of repairs, which makes the current integration look like a compromise btw old and new palettes1 (e.g., @field was changed to C.teal in catppuccin/nvim@8a3b892, intended to "<make> current syntactic rules better"2, but it got reverted to the previous color in a subsequent commit3), leading to a sense of disharmony between hl groups (which also makes it challenging to support semantic tokens).

This PR will try to make the hl groups self-explanatory, but we definitely need to receive more feedback and make several changeovers before merging this upstream 😄

Footnotes

  1. catppuccin/nvim#134

  2. catppuccin/nvim#134 (comment)

  3. catppuccin/nvim@b17cfa0

@Jint-lzxy
Copy link
Collaborator Author

Jint-lzxy commented May 8, 2023

I'm placing the "what's changed" table here to help me better store & track the changes I've made. Perhaps it can also help facilitate some discussions 👍 Sometimes builtin constants are not just constants literally, so it was given another color for differentiation.


  • LNK(group): Links to group
  • NEW(col): [For undefined highlight groups] New color col
  • IFP(parent, child): Inherited from the parent class. child, classified under parent, should have a similar color to the parent class.
  • R(before, after): Replace before with after
  • [+/-] B/I/U: Bold, Italics, Underline
  • FNC: Follow Naming Convention
  • NS: Not Sure
NS hl group operation reason
Search bg: R(darkened sky, surface1)
fg: R(text, pink)
style: +B
Selected a color similar to CurSearch to avoid confusion (things could get worse when using many highlights with different responsibilities).
Added B for differentiation.
IncSearch bg: R(darkened sky, pink)
fg: R(mantle, surface1)
Invert of Search
Identifier fg: R(flamingo, text) Following @variable. It is widely used, so should have a trivial color.
@variable LNK(Identifier) FNC: Any variable name
Label fg: R(sapphire, rosewater) IFP(Statement, Label)
(The reason for not using the same color as the parent class is b/c they still have subtle semantic differences)
Keyword fg: R(mauve, red?) Classified as any other keyword, so it should have a different color than the parent class.
I haven't figured out whether to use red or maroon yet. (Should avoid having the same color as @variable.builtin)
Exception fg: NEW(peach) Inherited from @exception
@exception LNK(Exception) FNC: try, catch, throw
Include fg: R(mauve, teal) To have a similar color to Included (which links to String).
Float LNK(Number) A floating point constant is a number constant
Macro LNK(Constant?) Although both vim/nvim link it to Define by default, macros are still constants, aren't they?
StorageClass LNK(Keyword?) Although it is classified under Type, AFAIK many syntax highlighters tend to label that as Keyword (also by definition):
C++ Standard - §5.11 [lex.key]
Structure LNK(Keyword?) ditto. Structure itself is not a valid type.
Tag fg: NEW(lavender)
style: +B
Inherited from @text.reference
@text.reference LNK(Tag) FNC
Delimiter fg: NEW(teal) The definition for hl-group Delimiter has changed: "delimiters (e.g. ; / . / ,)". Therefore, a suitable color was chosen for it.
@punctuation.delimiter LNK(Delimiter) FNC: delimiters (e.g. ; / . / ,)
@punctuation.special LNK(Special) FNC: special punctuation that does not fall in the categories before (e.g. {} in string interpolation)
@string.special LNK(Special) FNC: other special strings (e.g. dates)
@float LNK(Float) Here, @float should be linked to Float to prevent situations where the user changes the underlying highlight group (Float) but Treesitter highlight doesn't reflect this change.
@function.call LNK(Function) Should be linked to the underlying highlight group, not @function.
@function.macro LNK(Constant?) I tend to link it to Constant for the same reason as Macro. But I'm not sure if it looks good in every language.
@method LNK(Function?) IMHO there's no need to set a separate color for @method b/c methods are also functions.
@method.call LNK(Function) ditto.
@parameter fg: R(maroon, rosewater) rosewater is more suitable for the elements under this classification.
@keyword.operator LNK(Operator) FNC: new keyword operator
@keyword.return fg: R(maroon, pink) N/A
@variable.builtin style: +styles.properties Add styles.properties to indicate builtin types (same hereinafter)
@constant LNK(Constant) FNC: constants
@constant.builtin fg: R(peach, lavender?) Sometimes builtin constants are not just constants literally, so it was given another color for differentiation.
For example, in C++ nullptr is a prvalue of type std::nullptr_t.
@parameter fg: R(maroon, pink) Replaced by a softer color.
@type.qualifier LNK(Constant?) FNC: Type qualifiers should be keywords.
@field
@property
fg: R(lavender, rosewater) Same as Identifier. Assigned a trivial color.
@class
@struct
@enum
@enumMember
@event
@interface
@modifier
@regexp
@typeParameter
@decorator
DELETED? The comment said those groups are related to LSP Semantic Tokens, but I cannot find any documentation about that.
Can someone point out to me where those groups are documented?
Diagnostics···Hint fg: R(teal, rosewater) IMO "Hints" shouldn't be that eye-catching. It almost matches the color of DiagnosticsInfo.

@Jint-lzxy
Copy link
Collaborator Author

@CharlesChiuGit
Copy link
Collaborator

Hey @Jint-lzxy
what's the status of this PR? lol

@Jint-lzxy
Copy link
Collaborator Author

@CharlesChiuGit It'll probably stay here longer tho, as I'm still working on it upstream lol

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.

3 participants