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

nvim-cmp with border issue disucussion #72

Open
slugbyte opened this issue Sep 12, 2024 · 12 comments
Open

nvim-cmp with border issue disucussion #72

slugbyte opened this issue Sep 12, 2024 · 12 comments

Comments

@slugbyte
Copy link
Owner

          while we're at it i think `FloatBorder` highlight also could be something better.

This is with default pallet:

img

This is with Floatborder guibg=#101010:

img

Originally posted by @psynyde in #69 (comment)

@slugbyte
Copy link
Owner Author

as discussed in #69 this is approved, I just need to figure out how I want to implement it :), I should have a patch in the next week :)

@slugbyte
Copy link
Owner Author

slugbyte commented Sep 14, 2024

@psynyde Ive been messing around with this and haven't found a good solution

The main issue has to do with nvim-cmp.. basically when you tell nvim cmp to use a borders, if switches from inheriting from Pmenu for its background and instead uses Normal instead of NormalFloat :( This means that if I make borders look good for nvim-cmp it will just end up not looking good for other plugins that uses a float boarder.

So I'm not sure what I want to do going forward, I think adding a section in the docs on there readme to tell people how to have nice floatboarder for nvim-cmp might be the best option.

-- make nvim-cmp's boarders look pretty :) 
local lackluster = require("lackluster")
lackluster.setup({
    tweak_highlight = {
        FloatBorder = {
            fg = lackluster.color.gray5,
            bg = lackluster.color_specail.main_background,
        },
    },
})
vim.cmd.colorscheme("lackluster-hack")

what are your thoughts on this?

@slugbyte
Copy link
Owner Author

slugbyte commented Sep 14, 2024

After some more digging I decided to open an issue with nvim-cmp hrsh7th/nvim-cmp#2039 to see if we can solve this issue by patching nvim-cmp :)

That issue has a more through description of what I think the issue is.

@psynyde
Copy link

psynyde commented Sep 14, 2024

So I'm not sure what I want to do going forward, I think adding a section in the docs on there readme to tell people how to have nice floatboarder for nvim-cmp might be the best option.

I also think this'd be the better option until it's fixed by cmp author.

Edit: before having a conclusion can you set vim.cmd("highlight FloatBorder guibg=#101010") and check if anything looks out of place? I tried it right now and everything looks as it was (i stopped using border recently. So, no change on completion menu also). Although I don't use some plugins like whichkey for example so it's hard for me to test properly.

@slugbyte
Copy link
Owner Author

yep yep, vim.cmd("highlight FloatBorder guibg=#101010") looks good for nvim cmp but breaks other plugins that use NormalFloat for their backgound color including the example you gave example (which-key).

@slugbyte
Copy link
Owner Author

I'll wait for a reply on the nvim-cmp issue before I decide how to resolve this issue :)

@slugbyte
Copy link
Owner Author

created a new nvm-cmp issue after realizing the one i previously made did not properly use their bug template hrsh7th/nvim-cmp#2042

@slugbyte
Copy link
Owner Author

slugbyte commented Sep 19, 2024

@psynyde here is a snippet that you can use to test the proposed change to nvim-cmp if you are intersted

cmp.setup({
	window = {
		completion = vim.tbl_extend("force", cmp.config.window.bordered(), {
			winhighlight = "NormalFloat:NormalFloat,FloatBorder:FloatBorder,CursorLine:Visual,Search:None",
		}),
		documentation = vim.tbl_extend("force", cmp.config.window.bordered(), {
			winhighlight = "NormalFloat:NormalFloat,FloatBorder:FloatBorder,CursorLine:Visual,Search:None",
		}),
	},
	-- rest of your config
})

@psynyde
Copy link

psynyde commented Sep 21, 2024

@psynyde here is a snippet that you can use to test the proposed change to nvim-cmp if you are intersted

hm this seems to fix the issue
image

@slugbyte slugbyte changed the title suggestion: optional float boarder nvim-cmp with border disucussion Sep 23, 2024
@slugbyte slugbyte changed the title nvim-cmp with border disucussion nvim-cmp with border issue disucussion Sep 23, 2024
@fhawk12
Copy link

fhawk12 commented Sep 26, 2024

this fix my problem, maybe it works for you, too

		cmp.setup({
			snippet = {
				expand = function(args)
					vim.snippet.expand(args.body)
				end,
			},
			window = {
				completion = cmp.config.window.bordered(),
				documentation = cmp.config.window.bordered(),
			},
                        ...
    }
			lackluster.setup({
				tweak_syntax = {
					comment = lackluster.color.gray5,
				},
				tweak_background = {
					-- normal = "default", -- main background
					normal = "none", -- transparent
				},
				tweak_highlight = {
					FloatBorder = {
                                                overwrite = true,
						fg = lackluster.color.gray5,
						bg = "NONE",
					},
					["@keyword"] = {
						overwrite = true,
						bold = false,
						italic = true,
						fg = lackluster.color.gray6,
					},
					["@keyword.return"] = {
						overwrite = true,
						bold = false,
						italic = true,
						fg = lackluster.color.green,
					},
				},
			})

image

@slugbyte
Copy link
Owner Author

slugbyte commented Sep 26, 2024

@fhawk12 thanks for sharing! Makes me realize I need to make a tweak to allow for transparency with floats.. I'm on holiday rn but I'll make a patch next week sometime! float = "none" in tweak_background or something

Funny enough though, it's one of those things we're the right outcome is happening but for the wrong reason, nvim-cmps bug essentially doesn't effect you because you wanted transparency, and the thing they are assigning to background for the cmp-window happens to be transparent, which is lucky I guess :) when/if the bug gets patched, it should still look the way you want it to but for the right reason :)

@slugbyte
Copy link
Owner Author

I got the changed merged into magazine.nvim (a nvim-cmp fork)

iguanacucumber/magazine.nvim#3

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

3 participants