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

add pane-border-indent option #3798

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

andreykaipov
Copy link

Hi there - this patch removes the hardcoded 2's in screen-redraw.c and makes them customizable through a new pane-border-indent option. I've been running this patch for a few years but have been a bit reluctant opening a PR with it because of this comment from a related issue #2028 (comment):

We are moving away from having options for every little thing, so I don't want pane-border-indent, it needs to be expressed in the style, which would also allow it to be used in other contexts.

I'd love to know if your thoughts have since changed and if you'd be open to an option like this. I tried achieving border indentation using a style directive like mentioned in the comment, but I wasn't successful because the hardcoded 2's got in the way regardless.

Please let me know what you think!


Here's a demo using the following config:

set -g pane-border-status bottom
set -g pane-border-style ""
set -g pane-active-border-style ""
set -g pane-border-format "#[#{?#{pane_active},bg=red fill=red,bg=blue fill=blue}]"
set -g pane-border-indent 0

With default indentation:



With zero indent:

andreykaipov and others added 2 commits January 6, 2024 15:48
parametrizes the hardcoded 2's in screen-redraw.c
@nicm
Copy link
Member

nicm commented Jan 6, 2024

I'd rather see this done through the style or format option, so remove the hardcoded 2 and then add some way to express this in the style. I think pad would be a better name than indent since this is on both sides. This way it could also be used in other places.

@andreykaipov
Copy link
Author

I appreciate you taking a look! I pushed a WIP commit for the pad style option but I'm not entirely sure what I'd have to change in format-draw.c after passing it through. It doesn't feel quite as elegant so please let me know if it's not the right idea. I used the existing fill as an example to work from.

remove the hardcoded 2 and then add some way to express this in the style

Also maybe I'm misunderstanding, but if we do this, will the default padding effectively be zero in screen-redraw.c? And preserving existing behaviour will come from a default pad=2 in the default style? Wouldn't that cause any users that already have a style set to no longer have that padding, since they'll essentially be overwriting it?

@nicm
Copy link
Member

nicm commented Jan 7, 2024

I'll take a look at your changes later but yes you will need to change the default to include pad=2 and we will need to put it in the change log and release issue so people can update their configs if needed.

Copy link
Author

Choose a reason for hiding this comment

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

Woops I'll remove this file. Had changed it to make sure my build process was actually picking up my changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants