-
Notifications
You must be signed in to change notification settings - Fork 333
feat/making plugin padding customizable #352
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
base: master
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughA new configuration option for plugin padding in the tmux Dracula theme is introduced. Documentation is updated with instructions and examples for customizing or removing padding around plugins. The main script is modified to support left and right padding options, applying them consistently to plugin outputs in all display modes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
hi @druxorey, may i ask you to review this pr? i feel like this is something you probably know well and may like |
Hello, sorry for the delay. I've already checked and tested the PR, and it's a great addition. I haven't found any problems, so I think you can merge it into main. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
diff lgtm as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
scripts/dracula.sh (1)
370-381
: Empty-plugin hiding ignores padding – revisit logic (optional).
show_empty_plugins=false
relies on#{==:$script,}
– i.e. before padding is added.
If a plugin outputs an empty string butleft_pad/right_pad
are non-empty, the pads are
still generated yet subsequently discarded by the?
conditional.
That works, but it also means every evaluation executes (and discards) the pads,
which is unnecessary processing and can be confusing when custom pads include colour
formatting.
Consider basing the test onpad_script
(after quoting fix) or wrapping the padding
inside the false-branch so it is only produced when actually rendered.docs/CONFIG.md (2)
91-94
: Minor grammar & clarity nit – tighten the new paragraph.Current sentence is hard to parse.
-Whilst the padding is one space per default, can be whatever you want it to be, whether that's whitespace or other characters. +Padding defaults to a single space but can be *any* string – additional whitespace, symbols, even colour codes.Also consider explicitly noting that the zero-width space shown below is invisible; many readers
won’t realise the character is present.
95-101
: Invisible zero-width spaces – add a comment for copy-pasting users.Because the zero-width space is… well, invisible, many users end up pasting an empty string
instead and complain that padding remains.
A quick inline comment helps:-set -g @dracula-left-pad '' -set -g @dracula-right-pad '' +# zero-width space between the quotes ↓ +set -g @dracula-left-pad '' +set -g @dracula-right-pad ''
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/CONFIG.md
(1 hunks)scripts/dracula.sh
(2 hunks)
🔇 Additional comments (1)
scripts/dracula.sh (1)
366-368
:pad_script
duplicates quoting bug & can break with spaces in padding.Even after fixing the previous comment,
$left_pad$script$right_pad
is unquoted.
If either pad contains a space, word-splitting will again trim or split the string, potentially mangling
the status-line fragment.- pad_script="$left_pad$script$right_pad" +pad_script="${left_pad}${script}${right_pad}"Enclosing the components fixes both word-splitting and glob-expansion edge-cases.
Likely an incorrect or invalid review comment.
left_pad=$(get_tmux_option "@dracula-left-pad" " ") | ||
right_pad=$(get_tmux_option "@dracula-right-pad" " ") | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Leading / trailing-space padding is silently stripped – quote command substitutions.
get_tmux_option
may legitimately return strings that start or end with spaces (e.g. ' ° '
in the docs).
Because the result of a command-substitution is subject to word-splitting, the assignment
left_pad=$(get_tmux_option "@dracula-left-pad" " ")
drops those leading / trailing blanks – they never make it into left_pad
/ right_pad
.
- left_pad=$(get_tmux_option "@dracula-left-pad" " ")
- right_pad=$(get_tmux_option "@dracula-right-pad" " ")
+ left_pad="$(get_tmux_option "@dracula-left-pad" " ")"
+ right_pad="$(get_tmux_option "@dracula-right-pad" " ")"
Quoting preserves the exact byte sequence the user configured (including zero-width spaces).
Same issue exists anywhere we later read the option back; fixing it here is sufficient.
🤖 Prompt for AI Agents
In scripts/dracula.sh around lines 41 to 43, the command substitutions assigning
left_pad and right_pad are unquoted, causing leading and trailing spaces to be
stripped due to word splitting. To fix this, wrap the command substitutions in
double quotes like left_pad="$(get_tmux_option "@dracula-left-pad" " ")" and
right_pad="$(get_tmux_option "@dracula-right-pad" " ")" to preserve all
whitespace exactly as returned.
replace the space that would be added to the left and right of a plugin by customizable variables.
both the left and right pad variables are one space per default in order not to change the status quo and break user configs.
this enables tighter packing of plugins, by using a zero width space for padding.
or fancy wide configs