cocopon / vaffle.vim

:file_folder: Lightweight, window-based file manager for Vim
MIT License
284 stars 20 forks source link

[add] support for icon-font #43

Closed get-me-power closed 4 years ago

get-me-power commented 4 years ago

Hello, @cocopon . Thanks for creating great Vim plugin. I wrote a patch to support vim-devicons.

vim-devicons url

https://github.com/ryanoasis/vim-devicons

This patch can rendering icon font. Could you check this PR?

screen shot

スクリーンショット 2020-05-02 0 31 46 スクリーンショット 2020-05-02 0 31 54 スクリーンショット 2020-05-02 0 47 07
cocopon commented 4 years ago

Thank you for the contribution! Adding icon to each item looks nice, but there are some points that I'm concerned about.

About user interface

I think a file icon and its name should be placed side by side because they express the same subject.

About implementation

Current implementation is tightly coupled with vim-devicons. If user want to use another way to show icon, we should add another renderer option and related fixes to Vaffle. I think that providing a custom function to render icon is better solution. For example:

function! RenderMyFavoriteIcon(item)
  " Use vim-devicons or other favorite way
  return ...
endfunction

let g:vaffle_render_custom_icon = 'RenderMyFavoriteIcon'
get-me-power commented 4 years ago

@cocopon Thanks for review! How about this patch?

example

function! RenderMyFavoriteIcon(item)
  return printf('%s %s %s',
        \ a:item.selected ? '*' : ' ',
        \ WebDevIconsGetFileTypeSymbol(a:item.basename, a:item.is_dir),
        \ a:item.basename . (a:item.is_dir ? '/' : ''))
endfunction

let g:vaffle_render_custom_icon = 'RenderMyFavoriteIcon'

If you writing this config in vimrc, vaffle can render icon font.

cocopon commented 4 years ago

Thanks!

get-me-power commented 4 years ago

I adjusted. How about this?

example

function! RenderMyFavoriteIcon(item)
  return WebDevIconsGetFileTypeSymbol(a:item.basename, a:item.is_dir)
endfunction

let g:vaffle_render_custom_icon = 'RenderMyFavoriteIcon'
cocopon commented 4 years ago

Looks good! Looking into the details...

get-me-power commented 4 years ago

In addition, I changed the syntax highlighting. I think it will render correctly.

get-me-power commented 4 years ago

I updated document and syntax name.

get-me-power commented 4 years ago

@cocopon Thanks for review, I fixed.

cocopon commented 4 years ago

Great! Finally, could you squash all commits into a single one and force-push it? I'll merge it.

get-me-power commented 4 years ago

OK, I squashed!

get-me-power commented 4 years ago

@cocopon Sorry.., I fixed.

cocopon commented 4 years ago

Merged. Thank you for your contribution!

get-me-power commented 4 years ago

Thank you for your kind review and merge.