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

Configurable preview highlighter for Linux #707

Closed

Conversation

gbcreation
Copy link

I would like to pass custom options to highlighters used for previews (for example to select a specific color theme for highlight).

This PR is to add a new configuration variable g:fzf_preview_highlighter that allows to specify which highlighter to use with specific command line options.

The implementation works only on Linux (successfully tested) and probably on Mac (can someone test and confirm please?). I don't have Windows, would be glad if someone could adapt changes for it :)

Example:

let g:fzf_preview_highlighter = "highlight -O xterm256 --line-number --style rdark --force"

allows to use highlight with the rdark theme.

If the g:fzf_preview_highlighter variable is not set, the default behavior is similar to preview.rb: check if highlight, coderay, rougify and cat exist, and use the first one available.

There is also a new g:fzf_preview_line_highlight variable that allows to specify the ansi color code used to highlight the line in the preview window that corresponds to the selected one in the list. Default value is \x1b[7m ("REVERSE", the same color code used in preview.rb).

Examples:

To highlight the line with the bright black background color, use:

let g:fzf_preview_line_highlight = '\x1b[101m'

For terminals supporting 256-colors, you can specify a 8-bit color code:

let g:fzf_preview_line_highlight = '\x1b[48;5;240m'

You can even specify a 24-bit (true color) color code for terminals supporting it like Konsole:

let g:fzf_preview_line_highlight = '\x1b[48;2;80;80;80m'

Screenshot:

vim_enhanced_fzf_preview

@wookayin
Copy link
Contributor

wookayin commented Oct 7, 2018

This is a nice feature. However I have a small comments and concern with its implementation:

  • Most changes are on fzf.vim sources rather than on preview.rb or preview.sh, and you have copied the highlighter logic from those scripts into vim plugin scripts. In this case, we should avoid a redundant implementation.
  • I very like having a configuration point such as g:fzf_preview_highlighter, which can be used to specify any program that has a similar behavior as preview.rb (e.g. highlighter[:LINENO][:IGNORED]). Given that, I believe the reset of lines L48~L59 are not necessary. As in previous, we can simply keep L35 (where s:bin.preview is set).
  • Having another configuration point g:fzf_preview_line_highlight is awesome. I'd suggest to pass this variable in terms of command-line flag or environment variable so that preview.rb or preview.sh can be aware of this escape sequence.

Great idea!

@gbcreation
Copy link
Author

Hi @wookayin! Thanks a lot for your feedback.

Glad you appreciate this feature.

Actually, my changes could be considered as a factorization of the logic you're talking about 🙂
Let me explain that: currently, this logic is already duplicated into both preview.rb and preview.sh files. Every time you want to modify or enhance this logic, you have to make changes to both files and test both files. With my changes, all this logic is moved to a unique place in fzf.vim. It's easier to modify and test.

In fact, if you work on Linux (and probably on MacOs), you could remove the preview.rb and preview.sh files as they are no longer needed with my changes. I keep them only for compatibility with Windows platforms (that's why any help would be welcome to adapt my changes for Windows 😉 )

@junegunn
Copy link
Owner

Thanks for your interest in the project. I'm not really into the idea of introducing and maintaining more configuration knobs for fzf#vim#with_preview() function, which can give the impression to the users that they have to use the function and tinker with those variables for setting up previewer when it's a purely optional helper function whose only role is to add a few more options to the dictionary passed as the argument.

echo fzf#vim#with_preview({})

" See https://github.com/junegunn/fzf/blob/master/README-VIM.md#fzfwrap
let opts = { 'source': 'ls', 'window': 'tabnew' }
echo fzf#wrap(opts)
echo fzf#vim#with_preview(fzf#wrap(opts))

It's quite trivial to replicate what it does without using it if you know about the options. For most users, --preview option is all they need.

" Files with bat previewer
command! -bang -nargs=? -complete=dir Files
  \ call fzf#vim#files(<q-args>, {'options': ['--preview', 'bat -p --color always {}']}, <bang>0)

" Rg using custom preview script
command! -bang -nargs=* Rg
\ call fzf#vim#grep(
\   'rg --column --line-number --no-heading --color=always --smart-case '.shellescape(<q-args>), 1,
\   'options': ['--preview', '~/bin/my-custom-preview-script {}']
\   <bang>0)

this logic is already duplicated into both preview.rb and preview.sh

True, we don't need both scripts. sh version was added later by a contributer to support systems without Ruby. Currently it doesn't use an external syntax highlighters unlike the Ruby version, but I believe we can extend it and cover all use cases and eventually remove the original Ruby script.

And that's what I think we should do. I still prefer to have an external script file instead of having some shell script embedded somewhere in the bowels of a VimScript file. That way it's easier to test and maintain given that most users are not familiar with VimScript. You can use it outside of Vim separately, and easily copy and modify it to your liking.

@wookayin
Copy link
Contributor

wookayin commented Oct 25, 2018

+1 for having an external script for previewing.

I suggest we should be able to simply configure which highlighter (e.g. bat, coderay, etc.) should be used (as proposed with g:fzf_preview_highlighter, etc.) provided one just uses the default previewer script.

Using --preview (with custom preview scripts) in fzf#vim#...() is flexible enough, but it still can be cumbersome to have a redundant implementation to support the LINENO feature (e.g. preview.rb:LINENO:IGNORED).

@junegunn
Copy link
Owner

junegunn commented Nov 9, 2018

With #719 merged, you can now customize the preview command fzf#vim#with_preview uses via $FZF_PREVIEW_COMMAND. I'm still a bit undecided on having the variable. Should we allow users to explicitly pass it as an argument to the function? But having an environment variable makes sense if we're going to use the script outside of Vim. So I'll keep it undocumented until I'm sure there's no better option. See 0dbcfb2 for details.

" let $FZF_PREVIEW_COMMAND = 'nl {} || cat {}'
command! -nargs=* Rg
  \ call fzf#vim#grep(
  \   'rg --column --line-number --no-heading --color=always --smart-case '.shellescape(<q-args>),
  \   1,
  \   fzf#vim#with_preview('right:50%'))

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.

None yet

3 participants