Skip to content

Change border_style to a more adaptable border_style with options #174

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

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

Conversation

jorgebef
Copy link
Contributor

Options: default, bright, flat
This brings an additional option to the previous bright_border option which could be more tweakable.

Options: `default`, `bright`, `flat`
This brings an additional option to the previous `bright_border` option
which could be more tweakable.
@DOD-101
Copy link
Collaborator

DOD-101 commented Mar 19, 2025

Could you please provide some screenshots?

@jorgebef
Copy link
Contributor Author

Here are a few screenshots of both neotree and cmp plugins with each of the 3 options for border_style
border_style='bright':
border_style bright - cmp
border_style bright - neotree

border_style='default'
border_style default - cmp
border_style default - neotree

border_style='flat'
border_style flat - cmp
border_style flat - neotree

@AlexvZyl
Copy link
Owner

Didn't check the code but I like the addition :)

@AlexvZyl
Copy link
Owner

Please just check how this interacts with setting stuff like the telescope style. Might break if you set both to different settings.

@jorgebef
Copy link
Contributor Author

As far as I've checked, some plugins (like telescope) have a specific setting for their integration for style.
I believe this is a good approach, since a general approach like border_style could be used as a sane default and would be probably too hard to satisfy a wide range of options.
Leveraging a more specific setting inside telescope object in the setup for more intricate details would allow to configure each plugin in more detail if needed.

Copy link
Collaborator

@DOD-101 DOD-101 left a comment

Choose a reason for hiding this comment

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

image

This is what bright looks like for me. Not sure why.

kitty 0.40.0 Terminal Font: FiraCodeNFM-Reg (14pt)

I would also suggest changing telescope.style based on border_style while of course leaving users the option to set telescope.style separately if they so choose.

Apologies for the long wait. I just came back from vacation.

@@ -156,7 +156,7 @@ function M.get_groups()
-- TermCursorNC= { } -- cursor in an unfocused terminal
G.ErrorMsg = { fg = C.error } -- error messages on the command line
G.VertSplit = { fg = C.border_fg } -- the column separating vertically split windows
G.WinSeparator = { fg = C.border_fg, bg = C.border_bg } -- the column separating vertically split windows
G.WinSeparator = { fg = O.border_style == 'bright' and C.white0 or C.black0, bg = C.border_bg } -- the column separating vertically split windows
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems to me this change isn't necessary, since border_fg will change based on the border_style already.

The only difference being that when border_style=flat there would be no visible separator between splits, which seems to me at least to fit with the "flat" style.

Comment on lines +66 to +68
C.fg_float_border = options.border_style == 'bright' and C.white0
or options.border_style == 'default' and C.black0
or options.border_style == 'flat' and C.bg_float_border
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
C.fg_float_border = options.border_style == 'bright' and C.white0
or options.border_style == 'default' and C.black0
or options.border_style == 'flat' and C.bg_float_border
C.fg_float_border = options.border_style == 'flat' and C.bg_float_border or C.border_f

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