Add MSYS2 support as a vim plugin by msr1k · Pull Request #1677 · junegunn/fzf

msr1k · 2019-08-31T10:43:03Z

Add &shellcmdflag and TERM environment variable treatment.

Make &shellcmdflag /C when &shell turns into cmd.exe
Delete %TERM% environment variable before fzf execution

On MSYS2 environment, vim is typically configured as follows:

&shell: bash
&shellcmdflag: -c
$TERM(%TERM%): xterm or something

Since this plugin does not change &shellcmdflag while it replaces &shell with cmd.exe,
vim on MSYS2 environment cannot start fzf properly.
(It try to do cmd.exe -c something... but it should be cmd.exe /C something....)

And at the time of starting cmd.exe %TERM% is no longer needed.
So delete this environment variable in the batch file to be executed.

Add &shellcmdflag and TERM environment variable treatment. - Make &shellcmdflag `/C` when &shell turns into `cmd.exe` - Delete %TERM% environment variable before fzf execution

msr1k · 2019-08-31T22:02:00Z

junegunn/fzf.vim uses fzf#shellescape function.

But when it calls this function, &shell is not changed to cmd.exe
and shellescaping is executed not for cmd.exe though it is eventually passed to cmd.exe.

So I changed shell default value &shell to cmd.exe when s:is_win is true at c95d52f.

junegunn · 2019-09-03T01:49:13Z

@janlazo Hi, can you take a look at this?

janlazo · 2019-09-03T02:49:51Z


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


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

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

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')

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.

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.

I partially introduced your suggestion at ca770ec for now.

janlazo · 2019-09-03T03:03:00Z

This is ok for a first step since you set shellcmdflag but to support custom shells, junegunn/vim-plug#856 and junegunn/vim-plug#860 should be ported over.

Workaround for junegunn/fzf#1677 (comment)

Ubuntu xenial has Vim 7.4. Neovim 0.2.1 bumped its 'v:version' to 800. Neovim 0.2.2 is in Ubuntu bionic and Debian sid. Define commands outside of feature folds. 8.1.1210+ includes the +user_commands feature. Update termguicolor restrictions. Require Neovim v0.3.2+. neovim/neovim@98eaf60 Require Vim v8.1.0839+ with +vcon feature on Windows. vim/vim@f58d81a Unset $TERM in GUIs for fzf on Windows Workaround for junegunn/fzf#1677 (comment) Set nvim defaults in vimrc shared.vim should have shared defaults only. Reorder features in Vim 8.1.x.

janlazo · 2019-09-21T16:14:07Z

+  let [shell, shellslash, shellcmdflag] = [&shell, &shellslash, &shellcmdflag]
  if s:is_win
    set shell=cmd.exe
+    set shellcmdflag=/c


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.

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'

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

This funcion's behavior is controlled by only if it is Windows or not. So there is no need to check &shell.

janlazo · 2019-12-07T01:11:09Z

Wait for junegunn/vim-plug#913 to be merged to vim-plug and then be ported to fzf. Then, rebase your branch so that it can be merged.

msr1k · 2019-12-09T14:39:08Z

I got it. Thank you for your review.

junegunn · 2019-12-12T03:39:01Z

junegunn/vim-plug#913 is backported now, and I've resolved the conflict. Is everything okay?

janlazo · 2019-12-12T03:41:00Z

    return map([
      \ '@echo off',
      \ 'setlocal enabledelayedexpansion']
+    \ + (has('gui_running') ? ['set TERM= > nul'] : [])


Should port this over to vim-plug.

janlazo · 2019-12-12T03:45:43Z

Yes. It's safe to merge but we can wait for @msr1k to verify. Concern in junegunn/vim-plug#913 is unicode and msys2/mingw/cygwin allows unicode.

junegunn · 2019-12-15T09:27:23Z

I'm going to merge this as I want to push out a new release with it. Thanks @msr1k and @janlazo

janlazo · 2019-12-15T11:04:07Z

Tested it on a Windows VM today. It works with my config before and after switching the shell to bash. It's good to go, at least for English users.

msr1k added 2 commits August 31, 2019 18:59

Add MSYS2 support as a vim plugin

43213de

Add &shellcmdflag and TERM environment variable treatment. - Make &shellcmdflag `/C` when &shell turns into `cmd.exe` - Delete %TERM% environment variable before fzf execution

Change shellescape default value depending on s:is_win flag

c95d52f

junegunn added the windows Windows label Sep 3, 2019

janlazo reviewed Sep 3, 2019

View reviewed changes

Comment thread plugin/fzf.vim Outdated

janlazo reviewed Sep 3, 2019

View reviewed changes

janlazo added a commit to janlazo/dotvim8 that referenced this pull request Sep 3, 2019

vim: unset $TERM in GUIs for fzf on Windows

a68b16a

Workaround for junegunn/fzf#1677 (comment)

janlazo reviewed Sep 21, 2019

View reviewed changes

msr1k added 3 commits September 22, 2019 10:32

Make TERM environment empty only when gui is running

502432f

Stop checking &shell in fzf#shellescape function

ca770ec

This funcion's behavior is controlled by only if it is Windows or not. So there is no need to check &shell.

Take neovim into consideration when to set shellcmdflag

abde451

janlazo reviewed Oct 2, 2019

View reviewed changes

Comment thread plugin/fzf.vim

Add &shellxquote control

637d59f

Merge branch 'master' into vim-plugin-msys-support

a1bbd6b

janlazo reviewed Dec 12, 2019

View reviewed changes

junegunn assigned msr1k Dec 12, 2019

junegunn merged commit a9906c7 into junegunn:master Dec 15, 2019

janlazo mentioned this pull request May 21, 2020

fzf.vim commands opening up incorrect shell and not invoking fzf processes. junegunn/fzf.vim#703

Open

9 tasks

Uh oh!

Conversation

msr1k commented Aug 31, 2019

Uh oh!

msr1k commented Aug 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

junegunn commented Sep 3, 2019

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

janlazo Sep 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

janlazo commented Sep 3, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

janlazo Sep 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

janlazo commented Dec 7, 2019

Uh oh!

msr1k commented Dec 9, 2019

Uh oh!

junegunn commented Dec 12, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

janlazo commented Dec 12, 2019

Uh oh!

junegunn commented Dec 15, 2019

Uh oh!

janlazo commented Dec 15, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

msr1k commented Aug 31, 2019 •

edited

Loading

janlazo Sep 21, 2019 •

edited

Loading

janlazo Sep 23, 2019 •

edited

Loading