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

Suggestion: complete fzf_colors #581

Closed
3 of 9 tasks
trevordmiller opened this issue Feb 17, 2018 · 16 comments
Closed
3 of 9 tasks

Suggestion: complete fzf_colors #581

trevordmiller opened this issue Feb 17, 2018 · 16 comments
Labels

Comments

@trevordmiller
Copy link

trevordmiller commented Feb 17, 2018

  • Category
    • Question
    • Bug
    • Suggestion
  • OS
    • Linux
    • macOS
    • Windows
    • Etc.
  • Vim
    • Vim
    • Neovim

I've added custom FZF colors to a Vim theme I maintain using the fzf color variables described in the docs like this:

let g:fzf_colors =
\ { "fg":      ["fg", "Normal"],
  \ "bg":      ["bg", "Normal"],
  \ "hl":      ["fg", "IncSearch"],
  \ "fg+":     ["fg", "CursorLine", "CursorColumn", "Normal"],
  \ "bg+":     ["bg", "CursorLine", "CursorColumn"],
  \ "hl+":     ["fg", "IncSearch"],
  \ "info":    ["fg", "IncSearch"],
  \ "border":  ["fg", "Ignore"],
  \ "prompt":  ["fg", "Comment"],
  \ "pointer": ["fg", "IncSearch"],
  \ "marker":  ["fg", "IncSearch"],
  \ "spinner": ["fg", "IncSearch"],
  \ "header":  ["fg", "WildMenu"] }

This looks great with :FZF:

image

But a few of the other commands have other colors that can't be customized and are out of place:

:Buffers
The green and yellows here:
image

:Ag
The blue here:
image

:Snippets
The blue here:
image

:Lines
The blue and purple here:
image

:BLines
The purple here:
image

:Marks
The blue here:
image

:Windows
The green and yellow here:
image

:History:
The yellow and green here:
image

:History/
The yellow and green here:
image

It would be great if g:fzf_colors keys could be provided for all of these colors consistently so I could replace all the colors that don't match the theme.

I have latest fzf and fzf.vim and have reproduced with a minimal config.

Thank you so much for fzf and fzf.vim! They are fantastic :)

@rinetd
Copy link

rinetd commented Feb 27, 2018

cool !!!

@bluz71
Copy link

bluz71 commented Apr 4, 2018

I also maintain a Vim theme, moonfly, which I have just FZF'ified in the last couple of days with the existing g:fzf_colors option. That option covers most needs, but it would be great to have 100% coverage as this post identifies.

I also echo @trevordmiller statement, thanks to @junegunn for fzf and fzf.vim! :)

@trevordmiller
Copy link
Author

@junegunn What do you think about this? I'm happy to submit a pull request if you could give me some guidance on how to contribute and add to g:fzf_colors for the situations mentioned above ^ without breaking things.

@junegunn
Copy link
Owner

Although I don't want to add more configuration options, I'll see if I can improve the consistency of the colors. Please note that most of those colors are extracted from the current color scheme, so you shouldn't be seeing completely random colors. Try switching color schemes and see if the colors adapt accordingly, if they don't, let me know.

@bluz71
Copy link

bluz71 commented Aug 29, 2018

@junegunn,

A few color hard-coded items I notice with fzf.vim:

  • autoload/fzf/vim.vim line 172: let s:ansi = {'black': 30, 'red': 31, 'green': 32, 'yellow': 33, 'blue': 34, 'magenta': 35, 'cyan': 36}. When 256 colors are enabled the choices for 30, 31, 32, etc are not black, red, green but are: Turquoise4, DeepSkyBlue3, etc. When terminal 256 terminal colors are enabled the choices should really be: let s:ansi = {'black': 0, 'red': 1, 'green': 10, 'yellow': 11, 'blue': 4, 'magenta': 5, 'cyan': 6}. I believe the use of 30/31/32/etc is a legacy choice from 16-color days.

  • autoload/fzf/vim.vim line 695: \ '--color', 'hl:68,hl+:110']. The hard-coded choice of 68 (SteelBlue3) and 110 (LightSkyBlue3) do not work well with many Vim themes. I suggest maybe using colors 4 (Blue) and 12 (Bright Blue) instead since theme writers (such as myself) usually overwrite those colors in our themes.

Fixing those 2 items would solve all my issues with fzf.vim and my moonfly Vim theme.

Lastly, thanks for fzf, it is wonderful.

@junegunn
Copy link
Owner

The numbers in the dictionary are not from 256-color code table. They are base color codes prefixed by 3 for generating ANSI escape sequences.

echo -e "\x1b[31mRED"
echo -e "\x1b[41mON_RED"

# To use 256-color scheme, you need to prefix 38;5; and fzf.vim doesn't do it
echo -e "\x1b[38;5;31mNOT RED"

And those colors are only used when fzf fails to extract colors from the current color scheme:

" Try to extract LineNr color. If failed, use yellow (33)
s:yellow(" %4d ", "LineNr")

As for blue colors in Ag, you can currently override the options by redefining the command like I suggested here: #454 (comment)

But using blue and bright blue seems to make sense, and it works okay for my color scheme.

@bluz71
Copy link

bluz71 commented Aug 29, 2018

@junegunn,

Interesting with regards to the s:ansi dictionary. My experimentation with modifying that, as I noted, yielded improved theme integration. But, it was so long ago that maybe I just got it wrong? Clearly so as you explain. Please ignore.

At least the 2nd suggestion was more useful 😃 I think this one fix should solve some of Trevor's issues, the blue ones especially, as noted in the first post of this issue.

@junegunn
Copy link
Owner

Yeah, realized I also like the red pair, hl:1,hl+:9, colors really pop, I'm a bit torn :)

@bluz71
Copy link

bluz71 commented Aug 29, 2018

But an unintended pop?

After reading your previous response more clearly I now understand why misinterpreted fix for s:ansi colors improved things for my theme, it switched away from neon red/yellows (from memory). In 256 terminals the use of [38;5;31 is the correct approach (aka in Bash LS_COLORS for instance).

The poping red did not look good in my theme, and likely Nova as well.

Why not use Color 1 and Color 9(of 256 colors) which should be color-theme-appropriate Red and Bright Red?

P.S. I use Rg instead of Ag with fzf.vim, but that's not a discussion for here.

@junegunn
Copy link
Owner

Why not use Color 1 and Color 9(of 256 colors) which should be color-theme-appropriate Red and Bright Red?

I think you're confused. They are not different.

echo -e "\x1b[38;5;1mred / \x1b[31msame red"

@bluz71
Copy link

bluz71 commented Aug 29, 2018

Ok, I have done a quick play, the incorrect/non-theme blue color seen here in :Marks definitely relates to s:ansi (line 175):

36346714-c638b840-1400-11e8-9f46-515ef82d595b

When I change the definition of yellow, and nothing else, in s:ansi from 33 to 3 then the blue column becomes yellow (yellow from my theme). Where does blue in :Marks come from? That is one of the colors that I would like changed since it does not look good in my theme or Trevor's. The choice of yellow (33 --> 3) looks nice, but I don't mind which color is used as long as it is from the first 16 colors (aka theme colors).

I think this experience was why I said 30 should become 3, 31 becomes 1, etc. Now I am not sure about where the :Marks blue is coming from.

I know this is so pedantic, but having fzf integrate perfectly with Vim themes really does make a difference for the better.

@junegunn
Copy link
Owner

Oh, it seems like a bug.

let s:ansi = {'black': 0, 'red': 1, 'green': 2, 'yellow': 3, 'blue': 4, 'magenta': 5, 'cyan': 6}

Right?

@junegunn
Copy link
Owner

junegunn commented Aug 29, 2018

Actually, this is how it should be:

diff --git a/autoload/fzf/vim.vim b/autoload/fzf/vim.vim
index dfb5e54..d81f219 100644
--- a/autoload/fzf/vim.vim
+++ b/autoload/fzf/vim.vim
@@ -182,7 +182,7 @@ endfunction
 function! s:ansi(str, group, default, ...)
   let fg = s:get_color('fg', a:group)
   let bg = s:get_color('bg', a:group)
-  let color = s:csi(empty(fg) ? s:ansi[a:default] : fg, 1) .
+  let color = (empty(fg) ? s:ansi[a:default] : s:csi(fg, 1)) .
         \ (empty(bg) ? '' : ';'.s:csi(bg, 0))
   return printf("\x1b[%s%sm%s\x1b[m", color, a:0 ? ';1' : '', a:str)
 endfunction
@@ -692,7 +692,7 @@ function! fzf#vim#grep(grep_command, with_column, ...)
   \ 'column':  a:with_column,
   \ 'options': ['--ansi', '--prompt', capname.'> ',
   \             '--multi', '--bind', 'alt-a:select-all,alt-d:deselect-all',
-  \             '--color', 'hl:68,hl+:110']
+  \             '--color', 'hl:4,hl+:12']
   \}
   function! opts.sink(lines)
     return s:ag_handler(a:lines, self.column)
@@ -941,7 +941,7 @@ endfunction
 " Marks
 " ------------------------------------------------------------------
 function! s:format_mark(line)
-  return substitute(a:line, '\S', '\=s:yellow(submatch(0))', '')
+  return substitute(a:line, '\S', '\=s:yellow(submatch(0), "Number")', '')
 endfunction
 
 function! s:mark_sink(lines)

@junegunn
Copy link
Owner

Please update.

@bluz71
Copy link

bluz71 commented Aug 30, 2018

Hello @junegunn,

Sorry for my delayed response.

Yes, I am very pleased with all the changes you have done in the last day or so. I no longer see any color issues with my Vim moonfly theme. From my perspective this is now a resolved issue. Hopefully @trevordmiller will confirm the same with his Nova theme.

Many thanks, I genuinely appreciate the improvements you have implemented 👏

@trevordmiller
Copy link
Author

@junegunn @bluz71 I just upgraded to latest and tested and all of the situations from my screenshots have been fixed! Thank you so much. This marks 100% consistent colors for my theme 🎉. Y'all are awesome ❤️ Closing.

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

No branches or pull requests

4 participants