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

Remove text-coloring that interferes with fzf theming #484

Merged
merged 2 commits into from
Nov 6, 2024

Conversation

peterldowns
Copy link
Contributor

@peterldowns peterldowns commented Nov 5, 2024

This PR is based on the thread of discussion in #477. The goal of this PR is to make it so that in order to theme fzf-tab with the same colors as your normal fzf theme, users can use just the following settings in their .zshrc:

zstyle ':fzf-tab:*' use-fzf-default-opts yes
source ~/code/fzf-tab/fzf-tab.zsh

As fzf-tab works today, the required settings are:

zstyle ':fzf-tab:*' use-fzf-default-opts yes
# necessary to override the hard-coded color flag passed to fzf by fzf-tab:
#
#     --color=hl:$(( header_lines == 0 ? 188 : 255 )) \ 
#
zstyle ':fzf-tab:*' fzf-flags --color=hl:myThemeColor
# necessary to replace the default value of \[37m, "hard white",
# which is used when there is only one completion group — which
# overrides any color values of `hl` or `hl+` being set in other fzf flags
#
#      -ftb-zstyle -s default-color default_color || default_color=$'\x1b[37m' 
#
zstyle ':fzf-tab:*' default-color ""
source ~/code/fzf-tab/fzf-tab.plugin.zsh

As a new user of fzf-tab, I am extremely grateful for this project. It works very well and between the fzf knowledge, shell knowledge, and c knowledge required to make it work so quickly, I am seriously impressed by the work that has gone into it. I am hoping that this change will make the user experience better by making it way easier to theme fzf-tab to look like any other fzf instance.

This would be a breaking change. I do not know the history of these default values, but the single-group default color has been [37m since grouping was introduced, although it was originally known as FZF_TAB_NO_GROUP_COLOR: https://github.com/Aloxaf/fzf-tab/pull/23/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R121

I generally believe that PRs to update default themes/colors should be rejected by maintainers by default, but in this case I'd like you to consider that these changes only make it easier to customize fzf-tab's theme and make it look the way the user wants, by removing defaults present in the project.

I tested this change by using these zshrc settings, and observing that the fzf style matched my default fzf theme as expected, in both light mode and in dark mode:

zstyle ':fzf-tab:*' use-fzf-default-opts yes
source ~/code/fzf-tab/fzf-tab.plugin.zsh

If this PR is accepted as-is, the wiki entry for default-color will also need to be updated.

@villem
Copy link

villem commented Nov 6, 2024

This pull request fixes the hl+ coloring issues. I can now use any zsh-tinted theme. Excellent! And +1 fzf-tab being superb, utility.

@Aloxaf Aloxaf merged commit 6aced3f into Aloxaf:master Nov 6, 2024
10 checks passed
@peterldowns
Copy link
Contributor Author

@Aloxaf thank you for merging my contribution, and thank you for creating and maintaining fzf-tab.

Now that this PR is merged, you should probably update the wiki entry for default-color and close issue #477.

Thanks again 🙇

@Aloxaf
Copy link
Owner

Aloxaf commented Nov 7, 2024

Done.
Thanks for your contribution.

@peterldowns peterldowns deleted the pd/simplify branch November 7, 2024 00:46
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