search

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.22k stars 87 forks source link

Handle the new netrw <Plug>-based mapping if present #88

Closed matthewd closed 6 years ago

matthewd commented 6 years ago

I had a quick try at fixing #87... it seems to work, but (particularly the double-execute) doesn't feel particularly elegant.

It's really just a variation on https://github.com/tpope/vim-vinegar/issues/87#issuecomment-343347094, but sticks more closely to re-using the existing - mapping, and avoids an explicit version check.

(I haven't confirmed this still works with older netrw, though it seems likely.)

mcmillion commented 6 years ago

I've still got my machines on different versions between the change and it works on both.

BoltsJ commented 6 years ago

You may want to use =~? to ignore the case on the match instead of the case sensitive =~#.

samuelmasuy commented 6 years ago

Thank you, now it goes through, but I get:

Error detected while processing function <SNR>25_opendir:
line    7:
E490: No fold found
Press ENTER or type command to continue
BoltsJ commented 6 years ago

I took a look a look at your vimrc and you have <space> mapped to za. What's happening is that the trailing space character in s:netrw_up is being played and triggers your mapping. This can be avoided by running a substitute on s:netrw_up to strip the trailing whitespace. 

...
if s:netrw_up =~? "^<Plug>"
  let s:netrw_up = substitute(s:netrw_up, '\s\+$', '', '')
  let s:netrw_up = 'call feedkeys("\' . s:netrw_up . '")'
else
...
cdalsass commented 6 years ago

I applied the patch locally and the fix worked for me. (Was having same problem reported in #87 myself).

I am on Macvim/Sierra.

Seems others could benefit... Any reason not to merge?

daryllxd commented 6 years ago

Agree with "applying the patch" locally. Honestly I just pasted the actual code into my vimrc as a quick workaround if this gets patched asap.

hcnelson99 commented 6 years ago

Any update on this PR? I've applied the patch manually in the mean time. Could we get this merged?