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: add blink.cmp hlgroups #263

Closed
wants to merge 1 commit into from
Closed

Conversation

jonsch318
Copy link
Contributor

Nothing should really change, since blink already reads most of these from pmenu per default. However explicitly defining this is better in my opinion. I orientated myself on the existing Cmp groups.

Julian added a commit to Julian/dotfiles that referenced this pull request Dec 20, 2024
BlinkCmpSignatureHelpBorder = { link = "FloatBorder" },
BlinkCmpSignatureHelpActiveParameter = { link = "LspSignatureActiveParameter"},

BlinkCmpItemKindText = { fg = theme.ui.fg },
Copy link

@aliaksandr-trush aliaksandr-trush Dec 25, 2024

Choose a reason for hiding this comment

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

Should it be just BlinkCmpKind<kind> as it described in https://github.com/Saghen/blink.cmp/blob/main/docs/configuration/appearance.md ?

Copy link
Contributor

@WieeRd WieeRd Jan 1, 2025

Choose a reason for hiding this comment

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

BlinkCmpLabel = { fg = theme.ui.pmenu.fg },
BlinkCmpLabelMatch = { fg = theme.syn.fun },
BlinkCmpLabelDeprecated = { fg = theme.syn.comment, strikethrough = true },
BlinkCmpGhostText = { fg = theme.syn.comment },
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's better to use theme.ui.nontext for ghost texts to avoid confusion when typing inside comments. It's also the default highlight group blink.cmp chooses by default.

BlinkCmpScrollBarGutter = {link = "PmenuSbar" },
BlinkCmpLabel = { fg = theme.ui.pmenu.fg },
BlinkCmpLabelMatch = { fg = theme.syn.fun },
BlinkCmpLabelDeprecated = { fg = theme.syn.comment, strikethrough = true },
Copy link
Contributor

Choose a reason for hiding this comment

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

The same goes for the deprecated items, blink uses NonText by default.

Copy link
Contributor

Choose a reason for hiding this comment

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

Never mind this, just realized that using NonText for deprecated items makes it practically invisible on the default pmenu bg. I wasn't aware of this because I was using darker pmenu bg. But my point about the ghost text still stands.

Copy link
Contributor

Choose a reason for hiding this comment

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

Most of the blink menu highlights are meant to be consistent with Pmenu* groups.
So { link = "Pmenu*" } or theme.ui.pmenu.* should be preferred if possible.
The end result is the same regardless of using links or directly accessing the palette.
But if the user (me, for example :P) happens to be customizing the theme through .setup(),
using links will properly propagate the changes to Pmenu* to plugin highlights.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just realized you mostly copied the nvim-cmp groups from below.
I think the same changes should be applied to cmp groups as well.
Guess I'll write a PR^2 to your PR, to avoid duplicate PRs on this main repo.

WieeRd added a commit to WieeRd/kanagawa.nvim that referenced this pull request Jan 1, 2025
Changes suggested in rebelot#263 (review)

- Prefer using links to Pmenu groups.
- Added missing `CmpGhostText` group.
- Removed some misplaced whitespaces.
- Did NOT change ghost texts and deprecated items to `NonText`
  since it might be just my personal preference.
Comment on lines +204 to +229
BlinkCmpItemKindText = { fg = theme.ui.fg },
BlinkCmpItemKindMethod = { link = "@function.method" },
BlinkCmpItemKindFunction = { link = "Function" },
BlinkCmpItemKindConstructor = { link = "@constructor" },
BlinkCmpItemKindField = { link = "@variable.member" },
BlinkCmpItemKindVariable = { fg = theme.ui.fg_dim },
BlinkCmpItemKindClass = { link = "Type" },
BlinkCmpItemKindInterface = { link = "Type" },
BlinkCmpItemKindModule = { link = "@module" },
BlinkCmpItemKindProperty = { link = "@property" },
BlinkCmpItemKindUnit = { link = "Number" },
BlinkCmpItemKindValue = { link = "String" },
BlinkCmpItemKindEnum = { link = "Type" },
BlinkCmpItemKindKeyword = { link = "Keyword" },
BlinkCmpItemKindSnippet = { link = "Special" },
BlinkCmpItemKindColor = { link = "Special" },
BlinkCmpItemKindFile = { link = "Directory" },
BlinkCmpItemKindReference = { link = "Special" },
BlinkCmpItemKindFolder = { link = "Directory" },
BlinkCmpItemKindEnumMember = { link = "Constant" },
BlinkCmpItemKindConstant = { link = "Constant" },
BlinkCmpItemKindStruct = { link = "Type" },
BlinkCmpItemKindEvent = { link = "Type" },
BlinkCmpItemKindOperator = { link = "Operator" },
BlinkCmpItemKindTypeParameter = { link = "Type" },
BlinkCmpItemKindCopilot = { link = "String" },
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you not tested your PR at all?
blink.cmp uses BlinkCmpKind*, not BlinkCmpItemKind*.
I was so confused what was going on with item highlights not applying...

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed in jonsch318#1

WieeRd added a commit to WieeRd/kanagawa.nvim that referenced this pull request Jan 3, 2025
Changes suggested in rebelot#263 (review)

- Prefer using links to Pmenu groups.
- Added missing `CmpGhostText` group.
- Removed some misplaced whitespaces.
- Did NOT change ghost texts and deprecated items to `NonText`
  since it might be just my personal preference.
@WieeRd
Copy link
Contributor

WieeRd commented Jan 3, 2025

@jonsch318 jonsch318 closed this Jan 3, 2025
rebelot added a commit that referenced this pull request Jan 7, 2025
feat: add blink.cmp hlgroups (proceeds #263)
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