Skip to content

Add MSYS2 support as a vim plugin #1677

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Dec 15, 2019
Merged

Add MSYS2 support as a vim plugin #1677

Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
43213de
Add MSYS2 support as a vim plugin
msr1k Aug 31, 2019
c95d52f
Change shellescape default value depending on s:is_win flag
msr1k Aug 31, 2019
502432f
Make TERM environment empty only when gui is running
msr1k Sep 22, 2019
ca770ec
Stop checking &shell in fzf#shellescape function
msr1k Sep 22, 2019
abde451
Take neovim into consideration when to set shellcmdflag
msr1k Sep 26, 2019
637d59f
Add &shellxquote control
msr1k Dec 6, 2019
a1bbd6b
Merge branch 'master' into vim-plugin-msys-support
junegunn Dec 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions plugin/fzf.vim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ if s:is_win
" Use utf-8 for fzf.vim commands
" Return array of shell commands for cmd.exe
function! s:wrap_cmds(cmds)
return map(['@echo off', 'for /f "tokens=4" %%a in (''chcp'') do set origchcp=%%a', 'chcp 65001 > nul'] +
return map(['@echo off', 'set TERM= > nul', 'for /f "tokens=4" %%a in (''chcp'') do set origchcp=%%a', 'chcp 65001 > nul'] +
\ (type(a:cmds) == type([]) ? a:cmds : [a:cmds]) +
\ ['chcp %origchcp% > nul'], 'v:val."\r"')
endfunction
Expand All @@ -75,7 +75,7 @@ function! s:shellesc_cmd(arg)
endfunction

function! fzf#shellescape(arg, ...)
let shell = get(a:000, 0, &shell)
let shell = get(a:000, 0, s:is_win ? 'cmd.exe' : &shell)
Copy link
Contributor

@janlazo janlazo Sep 3, 2019

Choose a reason for hiding this comment

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

Is this for fzf#wrap only? fzf#run calls s:use_sh to set the shell.

Copy link
Contributor Author

@msr1k msr1k Sep 3, 2019

Choose a reason for hiding this comment

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

It is for external program such as junegunn/fzf.vim's vim script which seemingly calls this function before fzf#run execution.

Copy link
Contributor

@janlazo janlazo Sep 21, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

If you're doing this, go all the way and don't check &shell in this function. Change all invocations of fzf#shellescape within the context of fzf#run to pass &shell. Then, change this line to

let shell = get(a:000, 0, s:is_win ? 'cmd.exe' : 'sh')

Copy link
Contributor Author

@msr1k msr1k Sep 23, 2019

Choose a reason for hiding this comment

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

I'm sorry but I can't get what exactly means to add &shell for all invocations of fzf#shellescape.
Could you tell me what aim is laid here, or what problems exist if I don't do that.

In my opinion, since s:use_sh have already replaces &shell to sh or cmd.exe,
there are not so much differences regardless of whether I do that or not.

Copy link
Contributor

@janlazo janlazo Sep 23, 2019

Choose a reason for hiding this comment

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

Idea is to not set the shell in s:use_sh on Windows in the future, similar to what vim-plug does, because there are many shell option combinations to consider depending on the OS and editor. vim-plug passes &shell to fzf#shellescape when it needs to run a shell command with the user's &shell.

Perhaps, it's too much to do it all in this PR. fzf supported cmd.exe on Windows so doing it halfway (ie. hardcode the default fallback shell to cmd.exe for fzf#shellescape) is fine for now.

Copy link
Contributor Author

@msr1k msr1k Sep 26, 2019

Choose a reason for hiding this comment

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

I partially introduced your suggestion at ca770ec for now.

if shell =~# 'cmd.exe$'
return s:shellesc_cmd(a:arg)
endif
Expand Down Expand Up @@ -334,19 +334,20 @@ function! fzf#wrap(...)
endfunction

function! s:use_sh()
let [shell, shellslash] = [&shell, &shellslash]
let [shell, shellslash, shellcmdflag] = [&shell, &shellslash, &shellcmdflag]
if s:is_win
set shell=cmd.exe
set shellcmdflag=/c
Copy link
Contributor

@janlazo janlazo Sep 21, 2019

Choose a reason for hiding this comment

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

Setting shell options have too many edge cases on Windows because there are 3 incompatible shells to work with and fzf's Vim plugin supports neovim. Like vim-plug, don't set any shell options on Windows.

Copy link
Contributor

@janlazo janlazo Sep 23, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Not setting shell options here will take a lot of work so consider Neovim's default shell options here and call it a day.

let &shellcmdflag = has('nvim') ? '/s /c' : '/c'

Copy link
Contributor Author

@msr1k msr1k Sep 26, 2019

Choose a reason for hiding this comment

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

Thank you for your advice.
I applied it at abde451.

set noshellslash
else
set shell=sh
endif
return [shell, shellslash]
return [shell, shellslash, shellcmdflag]
endfunction

function! fzf#run(...) abort
try
let [shell, shellslash] = s:use_sh()
let [shell, shellslash, shellcmdflag] = s:use_sh()

let dict = exists('a:1') ? s:upgrade(a:1) : {}
let temps = { 'result': s:fzf_tempname() }
Expand Down Expand Up @@ -416,7 +417,7 @@ try
call s:callback(dict, lines)
return lines
finally
let [&shell, &shellslash] = [shell, shellslash]
let [&shell, &shellslash, &shellcmdflag] = [shell, shellslash, shellcmdflag]
endtry
endfunction

Expand Down