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

[vim] Use backslash for Windows filepaths #896

Merged
merged 12 commits into from
Apr 22, 2017
Next Next commit
[vim] Use backslash for Windows filepaths
janlazo committed Apr 2, 2017
commit d0c5508d7afab6c41f2d6796c84b3fa2c32b1912
96 changes: 70 additions & 26 deletions plugin/fzf.vim
Original file line number Diff line number Diff line change
@@ -26,10 +26,62 @@ if exists('g:loaded_fzf')
endif
let g:loaded_fzf = 1

function! fzf#shellescape(path)
if s:is_win
let shellslash = &shellslash
try
set shellslash
return '"'.shellescape(a:path).'"'
finally
let &shellslash = shellslash
endtry
endif
return shellescape(a:path)
endfunction

function! fzf#getcwd()
if s:is_win
let shellslash = &shellslash
try
set noshellslash
return getcwd()
finally
let &shellslash = shellslash
endtry
endif
return getcwd()
endfunction

function! fzf#fnamemodify(fname, mods)
if s:is_win
let shellslash = &shellslash
try
set noshellslash
return fnamemodify(a:fname, a:mods)
finally
let &shellslash = shellslash
endtry
endif
return fnamemodify(a:fname, a:mods)
endfunction

function! fzf#expand(fmt)
if s:is_win
let shellslash = &shellslash
try
set noshellslash
return expand(a:fmt)
finally
let &shellslash = shellslash
endtry
endif
return expand(a:fmt)
endfunction

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Three functions are almost identical. We can refactor the code using call().

function! s:without_shellescape(fn, ...)
  if s:is_win
    let shellslash = &shellslash
    try
      set noshellslash
      return call(a:fn, a:000)
    finally
      let &shellslash = shellslash
    endtry
  endif
  return call(a:fn, a:000)
endfunction

function! fzf#expand(fmt)
  return s:without_shellescape('expand', a:fmt)
endfunction

" ...

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, I'm not sure if we should expose these functions to the users by using fzf# prefix? Should they be script-local? What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used fzf# prefix because I thought that fzf.vim can use it to handle shellslash. However, these new functions are not suppose to change and are integral to fzf's Vim/Neovim plugin. Outside of fzf and fzf.vim, users shouldn't rely on these new functions. Do you prefer to minimize Vimscript dependencies between fzf and fzf.vim?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Ideally, users of the core fzf plugin, such as fzf.vim, should only rely on fzf#run and fzf#wrap but nothing else, and fzf#run should handle all the escaping issues automatically. So I'm not happy that we have to change the implementation of :FZF command.

Can we do better? If it's not feasible to properly escape a raw options string for different platforms, we can consider extending fzf#run so that it can process array arguments like so:

call fzf#run({'options': [['--prompt', getcwd()], '--reverse', ['--expect', 'ctrl-a']]})

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about fzf#shellescape()? Make it a script-local function as well?
For :FZF, what should I do to not change the implementation? Move it to fzf#wrap or fzf#run?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll think about it and let you know. I need to find time to test the plugin on Windows before making the decision. Thanks.

let s:default_layout = { 'down': '~40%' }
let s:layout_keys = ['window', 'up', 'down', 'left', 'right']
let s:is_win = has('win32') || has('win64')
let s:base_dir = expand('<sfile>:h:h')
let s:base_dir = fzf#expand('<sfile>:h:h')
let s:fzf_go = s:base_dir.'/bin/fzf'
let s:fzf_tmux = s:base_dir.'/bin/fzf-tmux'
let s:install = s:base_dir.'/install'
@@ -133,7 +185,7 @@ function! s:has_any(dict, keys)
endfunction

function! s:open(cmd, target)
if stridx('edit', a:cmd) == 0 && fnamemodify(a:target, ':p') ==# expand('%:p')
if stridx('edit', a:cmd) == 0 && fzf#fnamemodify(a:target, ':p') ==# fzf#expand('%:p')
return
endif
execute a:cmd s:escape(a:target)
@@ -148,11 +200,11 @@ function! s:common_sink(action, lines) abort
if len(a:lines) > 1
augroup fzf_swap
autocmd SwapExists * let v:swapchoice='o'
\| call s:warn('fzf: E325: swap file exists: '.expand('<afile>'))
\| call s:warn('fzf: E325: swap file exists: '.fzf#expand('<afile>'))
augroup END
endif
try
let empty = empty(expand('%')) && line('$') == 1 && empty(getline(1)) && !&modified
let empty = empty(fzf#expand('%')) && line('$') == 1 && empty(getline(1)) && !&modified
let autochdir = &autochdir
set noautochdir
for item in a:lines
@@ -242,7 +294,7 @@ function! fzf#wrap(...)

" History: g:fzf_history_dir
if len(name) && len(get(g:, 'fzf_history_dir', ''))
let dir = expand(g:fzf_history_dir)
let dir = fzf#expand(g:fzf_history_dir)
if !isdirectory(dir)
call mkdir(dir, 'p')
endif
@@ -262,19 +314,6 @@ function! fzf#wrap(...)
return opts
endfunction

function! fzf#shellescape(path)
if s:is_win
let shellslash = &shellslash
try
set noshellslash
return shellescape(a:path)
finally
let &shellslash = shellslash
endtry
endif
return shellescape(a:path)
endfunction

function! fzf#run(...) abort
try
let oshell = &shell
@@ -304,7 +343,7 @@ try
endtry

if has('nvim') && !has_key(dict, 'dir')
let dict.dir = getcwd()
let dict.dir = fzf#getcwd()
endif

if !has_key(dict, 'source') && !empty($FZF_DEFAULT_COMMAND)
@@ -394,13 +433,13 @@ endfunction

function! s:pushd(dict)
if s:present(a:dict, 'dir')
let cwd = getcwd()
let cwd = fzf#getcwd()
if get(a:dict, 'prev_dir', '') ==# cwd
return 1
endif
let a:dict.prev_dir = cwd
execute 'lcd' s:escape(a:dict.dir)
let a:dict.dir = getcwd()
let a:dict.dir = fzf#getcwd()
return 1
endif
return 0
@@ -674,19 +713,24 @@ let s:default_action = {
\ 'ctrl-v': 'vsplit' }

function! s:shortpath()
let short = pathshorten(fnamemodify(getcwd(), ':~:.'))
return empty(short) ? '~/' : short . (short =~ '/$' ? '' : '/')
let short = pathshorten(fzf#fnamemodify(fzf#getcwd(), ':~:.'))
let slash = s:is_win ? '\' : '/'
return empty(short) ? '~'.slash : short . (short =~ slash.'$' ? '' : slash)
endfunction

function! s:cmd(bang, ...) abort
let args = copy(a:000)
let opts = { 'options': '--multi ' }
if len(args) && isdirectory(expand(args[-1]))
if len(args) && isdirectory(fzf#expand(args[-1]))
let opts.dir = substitute(substitute(remove(args, -1), '\\\(["'']\)', '\1', 'g'), '[/\\]*$', '/', '')
let opts.options .= ' --prompt '.fzf#shellescape(opts.dir)
if s:is_win
let opts.dir = substitute(opts.dir, '/', '\\', 'g')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A minor nit. Indentation should be 2 spaces.

endif
let prompt = opts.dir
else
let opts.options .= ' --prompt '.fzf#shellescape(s:shortpath())
let prompt = s:shortpath()
endif
let opts.options .= ' --prompt '.fzf#shellescape(prompt)
let opts.options .= ' '.join(args)
call fzf#run(fzf#wrap('FZF', opts, a:bang))
endfunction