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

feat(syntax)!: reimplement the syntax highlighter #481

Closed

Conversation

Jint-lzxy
Copy link
Contributor

@Jint-lzxy Jint-lzxy commented May 9, 2023

Initially proposed in ayamir/nvimdots#676 (comment). The goal of this PR is also described there :)

This PR updated copious highlight groups and added support for LSP Semantic Tokens. The following is a list of modified hl groups with reasons for such modifications. The hl groups marked as NS indicate I'm still on the fence about whether these changes should be implemented:

  • 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
Include fg: R(mauve, teal) To have a similar color to Included (which links to String).
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.
Delimiter fg: NEW(teal) The definition for hl-group Delimiter has changed: "delimiters (e.g. ; / . / ,)". Therefore, a suitable color was chosen for it.
@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.
@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.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.
Diagnostics···Hint fg: R(teal, rosewater) IMO "Hints" shouldn't be that eye-catching. It almost matches the color of DiagnosticsInfo.

@sgoudham
Copy link
Contributor

sgoudham commented May 9, 2023

Hiya there 👋
Thanks for submitting the PR and putting a lot of thought into it!

We actually merged in our code editor style guide a while ago to try and normalise the colours within all Catppuccin editors. While we can't expect every editor to follow the guideline set, we aim for it to be detailed enough for editors to implement well.

I would encourage you to submit a PR against the main repository against that table as there are definitely improvements that can be made (e.g. we were having discussions on our discord about trying to standardise the search highlights) instead of just neovim as it will be more transparent for everyone to comment on before merging in improvements to all the editors

Hope that's okay! @Jint-lzxy

@sgoudham sgoudham marked this pull request as draft May 9, 2023 15:50
@nullchilly
Copy link
Contributor

nullchilly commented May 9, 2023

@class @struct @enum @enumMember @event @interface @modifier @regexp @typeParameter @decorator

Yes, these highlights were removed in favor of lsp rule-based semantic in neovim/neovim@1cc23e1

@Jint-lzxy
Copy link
Contributor Author

Hi @sgoudham Sorry for the delay! I did notice the style guide last year, but at that time it was still a WIP. Glad to hear that it has been updated recently 👍

I would encourage you to submit a PR against the main repository against that table as there are definitely improvements that can be made.

OK no probs - I will sort out a list of possible highlight categories based on :h group-name, :h treesitter-highlight-groups, and Language Server Protocol Specification and open a PR against the main repository that applies to generic code editors (including reasons for such changes like this PR), where we can continue our discussion 😃

@ayamir
Copy link

ayamir commented May 27, 2023

Any progress for it?

@Jint-lzxy
Copy link
Contributor Author

Any progress for it?

Sorry for my discontinued availability. I've created a PR at catppuccin/catppuccin#2109 😃

@Jint-lzxy Jint-lzxy force-pushed the refactor/syntax-highlighting branch from 2310246 to 87b9ad3 Compare July 10, 2023 16:10
@nullchilly
Copy link
Contributor

I don't think @keyword.operator should be linked to Operator

Before

image

After

image

@Jint-lzxy
Copy link
Contributor Author

Jint-lzxy commented Jul 10, 2023

I don't think @keyword.operator should be linked to Operator

My original motivation was to "differentiate @keyword.operator (i.e. operators that are English words) from other elements" - which mainly emphasizes the operator part:

Before F2
After F1

Otherwise, these nodes may have colors similar to many other elements (under the newly proposed style guide): like "Tags", "Reserved Constants", "Specials", etc. Thus implicitly(?) indicating that they have similar "intrinsics". (My intention was to prevent unrelated nodes from having similar colors)

But indeed this is not a perfect solution, especially when considering function-style operators: The relatively bright color C.sky can make a single element stand out, thus would be useful on isolated nodes, but when the element comes with a pre- (or suf-)fix of different color(s), this time the overly bright color can make the node look peculiar. Maybe we can make a special rule for this?

@Jint-lzxy Jint-lzxy force-pushed the refactor/syntax-highlighting branch from d6a30d6 to ff151e7 Compare July 11, 2023 08:24
@Jint-lzxy Jint-lzxy force-pushed the refactor/syntax-highlighting branch from e49cda7 to 1f76a94 Compare July 17, 2023 10:02
@nekowinston nekowinston changed the title refactor(syntax)!: reimplement the syntax highlighter feat(syntax)!: reimplement the syntax highlighter Aug 10, 2023
@Jint-lzxy
Copy link
Contributor Author

I'm going to close this PR and open a new one that follows the updated standard captures (working on this right now lmao), bc nvim-treesitter/nvim-treesitter#5895 (and #630) have already been merged, and I kinda messed up the commit history 🙈

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.

4 participants