tpope / vim-vinegar

vinegar.vim: Combine with netrw to create a delicious salad dressing
https://www.vim.org/scripts/script.php?script_id=5671
2.23k stars 86 forks source link

Update g:netrw_list_hide from wildignore when entering netrw #69

Open kiryph opened 8 years ago

kiryph commented 8 years ago

Currently, vinegar reads 'wildignore'/'wig' only a single time when vim is started and sets g:netrw_list_hide with

if exists("g:loaded_vinegar") || v:version < 700 || &cp
  finish
endif
let g:loaded_vinegar = 1
...
let g:netrw_list_hide =
    \ join(map(split(&wildignore, ','), '"^".' . s:escape . '. "$"'), ',') . ',^\.\.\=/\=$' .
    \ (get(g:, 'netrw_list_hide', '')[-strlen(s:dotfiles)-1:-1] ==# s:dotfiles ? ','.s:dotfiles : '')

However, 'wig' can change during a vim session: user wants to

Therefore, I would propose updating g:netrw_list_hide everytime the user enters the netrw window.

kiryph commented 8 years ago

Following code change updates g:netrw_list_hide from wildignore everytime s:setup_vinegar() is called (happens for autocmd Filetype netrw) which can be triggered also manually with <C-L>:

 function! s:setup_vinegar() abort
+  if exists("w:netrw_wigkeep")
+      let g:netrw_list_hide =
+          \ join(map(split(w:netrw_wigkeep, ','), '"^".' . s:escape . '. "$"'), ',') .
+          \ ',^\.\.\=/\=$' .
+          \ (get(g:, 'netrw_list_hide', '')[-strlen(s:dotfiles):-1] ==# s:dotfiles ? ','.s:dotfiles : '')
+  endif
+
   if empty(s:netrw_up)
     " save netrw mapping
     let s:netrw_up = substitute(maparg('-', 'n'), '\c^:\%(<c-u>\)\=', '', '')

NOTES:

❯ git diff
diff --git a/plugin/vinegar.vim b/plugin/vinegar.vim
index f471141..d228e78 100644
--- a/plugin/vinegar.vim
+++ b/plugin/vinegar.vim
@@ -20,7 +20,7 @@ let g:netrw_sort_sequence = '[\/]$,*,\%(' . join(map(split(&suffixes, ','), 'esc
 let s:escape = 'substitute(escape(v:val, ".$~"), "*", ".*", "g")'
 let g:netrw_list_hide =
       \ join(map(split(&wildignore, ','), '"^".' . s:escape . '. "$"'), ',') . ',^\.\.\=/\=$' .
-      \ (get(g:, 'netrw_list_hide', '')[-strlen(s:dotfiles)-1:-1] ==# s:dotfiles ? ','.s:dotfiles : '')
+      \ (get(g:, 'netrw_list_hide', '')[-strlen(s:dotfiles):-1] ==# s:dotfiles ? ','.s:dotfiles : '')
 if !exists("g:netrw_banner")
   let g:netrw_banner = 0
 endif

To understand better the mistake in indexing see this simple example:

let s:string1 = '0123'
let s:string2 = '456789'
let s:string = s:string1.s:string2
if s:string[-strlen(s:string2):-1] == s:string2
    unsilent echom "s:string[-strlen(s:string2):-1]=".s:string[-strlen(s:string2):-1]."(== s:string2)"
endif
if s:string[-strlen(s:string2)-1:-1] == s:string2
    unsilent echom "s:string[-strlen(s:string2)-1:-1]=".s:string[-strlen(s:string2)-1:-1]."(== s:string2)"
endif

As far as I have understood the code, the only allowed pre-defined value for g:netrw_list_hide is the value of s:dotfiles. Therefore, the code could be made more readable: \ (get(g:, 'netrw_list_hide', '') ==# s:dotfiles ? ','.s:dotfiles : '')

kiryph commented 6 years ago

Update g:netrw_list_hide from 'wildignore' on event OptionSet

The event OptionSet has been added in patch 7.4.786 (17 Jul 2015) with some fixes later on.

" if has('patch-7.4.786')
augroup update_netrw_list_hide
  autocmd!
  autocmd OptionSet wildignore
            \  let g:netrw_list_hide =
       \ join(map(split(&wildignore, ','), '"^".' . s:escape . '. "$"'), ',') . ',^\.\.\=/\=$' .
       \ (get(g:, 'netrw_list_hide', '')[-strlen(s:dotfiles):-1] ==# s:dotfiles ? ','.s:dotfiles : '')
augroup END
" endif
tpope commented 6 years ago

That seems like a no brainer. You can check for exists('##OptionSet') rather than a cryptic patch number. I can't remember, is there a good way to force netrw to redraw automatically?

If the stuff in the rest of this issue is still relevant I'll entertain it too. Perhaps opening a fresh PR is the way to go?

kiryph commented 6 years ago

I can't remember, is there a good way to force netrw to redraw automatically?

I am struggling with this right now. My testing scenario is following

1) I have opened a netrw window and the listing contains *.pyc files. 2) Now I want to remove *.pyc files with :set wig+=*.pyc

I would have hoped that my netrw window would pick this up via the event OptionSet. A manual refresh with <C-l> mapped to <Plug>NetrwRefresh does not update appropriately.

However, echo g:netrw_list_hide reveals that g:netrw_list_hide is correctly updated from the OptionSet event.

Surprisingly, a FocusGained (calls s:LocalBrowseRefresh) event does what one want: *.pyc files are removed. But pressing <C-l> readds them which is weird.

Can you confirm this?

BTW: vim-sensible interfers with netrw regarding <C-l>: see https://github.com/tpope/vim-sensible/issues/121

Perhaps opening a fresh PR is the way to go?

I would open a fresh PR if the refresh issue could be figured out. (also avoiding dupliated code by introducing a function or invoking the event OptionSet to do the initial let g:netrw_list_hide =)

kiryph commented 6 years ago

I can emphasize once again that, instead of pressing <C-l> to update the listing, invoking :doautocmd FocusGained removes the *.pyc files. But pressing <C-l> readds them.

This looks like a netrw issue. I will send an email to Charles Campbell whether he can confirms this and can fix this. My own try by adding s:LocalBrowseRefresh to <Plug>NetrwRefresh has not been successul. But I am also not really familiar with the autoload/netrw.vim file.

kiryph commented 6 years ago

After some further digging around in autoload/netrw.vim, I have found the netrw function s:NetrwOptionsSafe which deliberately empties 'wildignore' for netrw windows:

"netrw version 163
1689:" s:NetrwOptionsSafe: sets options to help netrw do its job {{{2
1725:  call s:NetrwSetSafeSetting("&l:wig","") " wig=wildignore

This, however, also triggers the event OptionSet which removes my wildignore entries from g:netrw_list_hide. Apparently the FocusGained event avoids a call to the function s:NetrwOptionsSafe but not <C-l>.

IMHO it looks like Charles Campbell does not think that the value of 'wildignore' should apply for netrw windows.

kiryph commented 6 years ago

~~Since netrw changes wig itself, the advantage of updating g:netrw_list_hide only when wig changes via OptionSet does not really exist. Therefore, I do no longer propose~~

Update g:netrw_list_hide from 'wildignore' on event OptionSet

I now use following non-perfect solution which has unnecessary redraws of windows when a user changes 'wig'. If one does not need the auto-update, remove the autocmd and press manually <c-l>:

I have found a solution by shielding the autocmd OptionSet wildignore from netrw. Unfortunately, this is not a clean as I was hoping for. Essentially one checks for the existence of the stored netrw safe setting netrw_wigkeep and can determine if wildignore is changed due to the internal workings of netrw or from somewhere else. This works very well when the function s:NetrwOptionsSafe() is called with the option "w:" but not with "s:". This happens in s:NetrwBrowseChgDir(islocal,newdir,...) which is called by <Plug>NetrwRefresh. <C-L> is mapped to this which means redundant calls would occur. Therefore, the variable w:vinegar_ctrl_l avoids that the netrw window is redrawn twice when pressing <C-L>.

❯ g diff
diff --git a/plugin/vinegar.vim b/plugin/vinegar.vim
index 9473d70..c1361e5 100644
--- a/plugin/vinegar.vim
+++ b/plugin/vinegar.vim
@@ -19,9 +19,6 @@ endfunction
 let s:dotfiles = '\(^\|\s\s\)\zs\.\S\+'

 let s:escape = 'substitute(escape(v:val, ".$~"), "*", ".*", "g")'
-let g:netrw_list_hide =
-      \ join(map(split(&wildignore, ','), '"^".' . s:escape . '. "/\\=$"'), ',') . ',^\.\.\=/\=$' .
-      \ (get(g:, 'netrw_list_hide', '')[-strlen(s:dotfiles)-1:-1] ==# s:dotfiles ? ','.s:dotfiles : '')
 if !exists("g:netrw_banner")
   let g:netrw_banner = 0
 endif
@@ -36,6 +33,16 @@ nnoremap <silent> <Plug>VinegarTabUp :call <SID>opendir('tabedit')<CR>
 nnoremap <silent> <Plug>VinegarSplitUp :call <SID>opendir('split')<CR>
 nnoremap <silent> <Plug>VinegarVerticalSplitUp :call <SID>opendir('vsplit')<CR>

+if exists('##OptionSet')
+  augroup Vinegar_redraw_netrw_windows
+    autocmd!
+    autocmd OptionSet wildignore,suffixes let s:currwin=winnr() | silent windo
+            \ | if &ft=='netrw' && !exists('w:netrw_wigkeep') && !exists('w:vinegar_ctrl_l')
+            \ |   execute "normal \<C-L>" | endif
+            \ | execute s:currwin . 'wincmd w'
+  augroup END
+endif
+
 function! s:opendir(cmd) abort
   let df = ','.s:dotfiles
   if expand('%:t')[0] ==# '.' && g:netrw_list_hide[-strlen(df):-1] ==# df
@@ -97,6 +104,13 @@ function! s:escaped(first, last) abort
 endfunction

 function! s:setup_vinegar() abort
+  if exists("w:netrw_wigkeep")
+    let g:netrw_list_hide =
+            \ join(map(split(w:netrw_wigkeep, ','), '"^".' . s:escape . '. "$"'), ',') .
+            \ ',^\.\.\=/\=$' .
+            \ (get(g:, 'netrw_list_hide', '')[-strlen(s:dotfiles):-1] ==# s:dotfiles ? ','.s:dotfiles : '')
+  endif
+
   if !exists('s:netrw_up')
     let orig = maparg('-', 'n')
     if orig =~? '^<plug>'
@@ -109,6 +123,7 @@ function! s:setup_vinegar() abort
     endif
   endif
   nmap <buffer> - <Plug>VinegarUp
+  nmap <buffer> <C-L> :let w:vinegar_ctrl_l=1<bar>execute "normal \<Plug>NetrwRefresh"<bar>unlet w:vinegar_ctrl_l<CR>
   cnoremap <buffer><expr> <Plug><cfile> get(<SID>relatives('.'),0,"\022\006")
   if empty(maparg('<C-R><C-F>', 'c'))
     cmap <buffer> <C-R><C-F> <Plug><cfile>

@tpope if you are fine with the change for the function s:setup_vinegar() I would create a PR. If you think an autocmd would be great as well, I can of course add it. But afaik you do not like adding autocmds which I can understand but sometimes I prefer more immediate feedback.