-
Notifications
You must be signed in to change notification settings - Fork 358
Use typeable word for custom padding instead of zero-width space #373
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
Conversation
📝 WalkthroughWalkthroughThis PR updates the tmux Dracula configuration to support setting padding to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
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: 0
🧹 Nitpick comments (1)
scripts/dracula.sh (1)
44-45: Consider case-insensitive comparison for padding normalization.The string comparison correctly normalizes
"false"to an empty string, but it's case-sensitive. Users who configure"False"or"FALSE"won't benefit from this feature.Apply this diff to handle case variations:
- if [ "$left_pad" = false ]; then left_pad=""; fi - if [ "$right_pad" = false ]; then right_pad=""; fi + if [ "${left_pad,,}" = "false" ]; then left_pad=""; fi + if [ "${right_pad,,}" = "false" ]; then right_pad=""; fiNote: The
${var,,}syntax converts to lowercase and requires Bash 4.0+. Alternatively, for broader compatibility:- if [ "$left_pad" = false ]; then left_pad=""; fi - if [ "$right_pad" = false ]; then right_pad=""; fi + case "$left_pad" in + [Ff][Aa][Ll][Ss][Ee]) left_pad="" ;; + esac + case "$right_pad" in + [Ff][Aa][Ll][Ss][Ee]) right_pad="" ;; + esac
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/CONFIG.md(1 hunks)scripts/dracula.sh(1 hunks)
🔇 Additional comments (1)
docs/CONFIG.md (1)
98-105: LGTM! Clear and helpful documentation.The addition clearly explains how to remove padding using
false, providing both explanation and example. This makes the feature discoverable and easy to use.
ethancedwards8
left a comment
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
This PR resolves #372 .
Although the issue is not always able to reproduce, I believe it is better to use a normal word instead of a specific unicode byte.
It should not affect any previous configs, including the people who were using
U+200Bas their padding.Thank you!