Fix: Windows preview window logic, including git diff preview

[psyngw]

Fix preview window problems for Windows user.

I generally use Arch for development, but recently I also started using WSL, sometimes I don't want to open WSL, so I just open nvim in terminal and use it, I'm kind of a heavy user of fzf.vim, this plugin is really good. However, I found that the preview window reported various errors when using it directly in Windows (e.g. nvim-qt);

I looked through the documentation and some other PRs, but I found that some of the solutions could cause another problem while solving one, so I tried to fix some of the problems and added an option to use other bash directly ( git bash, for example).

I've tried to minimize the impact on users using other environments, and I use WSL and Linux myself, so I understand the importance of stability.

Here's what I've tested, and even if it doesn't merge successfully, I hope this will help people who are having the same problem as me.

All with or without delta, ag. All neovim use packer. All vim use plug.

Windows:

• [x] neovim and vim x64 in cmd and powershell
• [x] nvim-qt and gVim x64
• [x] neovim and vim in WSL - ubuntu and Arch
• [x] Git bash with vim

gVim x32 can not work properly because it can't access the bash under C:\system32, some people say 32bit can only access the files under C:\Windows\Sysnative but I don't have the environment to test it for now, replace it in the configuration file to other bash (such as git bash: let g:fzf_preview_win_bash='C:\path\to\Git\bin\bash.exe') can work properly.

Linux:

• [x] Arch

Mac:

• [x] Maybe will test it next week . Tested: macOS Catalina 10.15.7 with neovim v0.8.2.

[junegunn]

I'm not a Windows user, and it's really hard for me to understand all the different problems from different setups. So let me clarify a few things.

• [ ] No problem when you're running fzf.vim on WSL bash?
• [ ] No problem when you're running fzf.vim on cmd or powershell?
• [ ] Then this patch is supposed to fix issues only on nvim-qt?
  • [ ] Does gVim x64 also have the problem?
• Can you show the error messages?
• What are the values of s:is_win and s:is_wsl on nvim-qt?

[psyngw]

@junegunn Okay, I understand your question, later when I finish my daily work, if I have time I will answer your questions one by one and try to optimize the code structure. I've been a little busy lately, with work, covid-19 and some other stuff, so please give me some time.

[junegunn]

No hurry, take your time.

[psyngw]

@junegunn For your question.

• No problem when you're running fzf.vim on WSL bash? Yeah, no problem if in the virtual WSL, obvious it's in a Linux system if in the WSL. All error occur outside the WSL, In the Windows env.
• No problem when you're running fzf.vim on cmd or powershell? No, there's same problems like on nvim-qt(some differences from Gvim, which will be mentioned below.)
• Then this patch is supposed to fix issues only on nvim-qt? No, for cmd, powershell, nvim-qt and gvim, all from Windows env.
• Does gVim x64 also have the problem? Yeah, x64 also have the problem, but less than nvim and nvim-qt. Also blow.
• Can you show the error messages? Of course, This is almost always because of the different path rules used by bash on the two systems. If errors occurred, it would be fatal:{your exec file path}, No such file or diretory
• What are the values of s:is_win and s:is_wsl on nvim-qt? Of course is 1, which means true. Just like the env nvim and vim on cmd and powershell.

And here's something new.

Let's say u use nvim on cmd or powershell, or nvim-qt directly. And u installed WSL,the env's default bash will be the linux-like bash, Ok the usual preview(e.g. :Files, :Ag) and diff preview both occur error because of the path rules.

If u use vim on cmd or powershell, or gvim directly. And u installed WSL, then the usual preview will working but the diff preview won't, here's less problem than nvim because of the if on line 41 of the original code.

For all the situations, if u didn't install WSL, That will always occur error, of course, that is because the env can not find bash, the solution is also very simple, download the required bash and the accompanying tools, add the env variables. Here only add the fzf_preview_win_bash I added to fix is impossible, because obviously we have to use a series of other tools to preview.

Then let's take a look at the new code. I've separated out a few variables, but I've also tried to make sure that it only affects the logic of the Windows environment. To make it easier to add different regex paths in the future I've extracted the linux-like bash and added another global variable so that users(who knows exactly what they're doing) could directly define whether the bash they are using is linux-like or not, but of course, the new variables will not affect the non-Windows env, no matter how the user defines them.

Here's the new code's test: Default bash: C:\Windows\System32\bash.exe. Other bash: Git Bash. All with delta and ag. All nvim use packer. All vim use plug. Each test:

• default bash without define any variable
• set g:preview_win_bash
• different g:preview_win_bash_linux_like with different situations Focus on the commands that could preview like: GFiles, GFiles?, Buffers, Files, Ag, Lines.....

Windows:

• win11 and win10
• [x] neovim and vim x64 on cmd and powershell
• [x] nvim-qt and gVim x64
• [x] neovim and vim in WSL - ubuntu and Arch
• [x] cmder and git bash use vim and nvim from outside, not their own vim

Linux:

• [x] Arch

Mac:

• [x] maybe tomorrow, again :> Done: macOS Catalina 10.15.7 with neovim and vim

For Windows: there's still some problems in Cygwin, I find out they are win32unix, But for the time being, I think it's better to just use other solutions, such as using an vim and nvim from outside, and use winpty vim in git bash.

Sorry I force-pushed, I feel 1 commit is better to read.

[psyngw]

@junegunn For the latest code: I didn't use ls to verify, because I think it seems to work in most cases but I still want to be more stable, so I added a parameter to preview.sh to test it, doesn't affect anything, I don't know what you think about this.

This time I didn't force push because I need to do more test after daily work, and also, your opinion.

Windows:

• [x] win10 passed all of the following items, wait for win11 test ... Passed
• [x] neovim and vim x64 on cmd and powershell
• [x] nvim-qt and gVim x64
• [x] neovim and vim in WSL - ubuntu and Arch
• [x] cmder and git bash(use winpty vim

Linux:

• [x] Arch

Mac:

• [x] macOS Catalina 10.15.7 with neovim and vim

[junegunn]

I have updated your branch, please see the commit message of 8c2993743b2e5b0135b2b20121775d97d1b198fc for details, and let me know if it doesn't work as expected. One thing I wasn't so sure about was the way you tested "linux-like"-ness of the bash using bash PREVIEW_SCRIPT --bash-test command. Shouldn't we be running bash -c "PREVIEW_SCRIPT --bash-test" so that the path is handled by bash itself not by cmd.exe? Anyway, the result of the check affects the path manipulation afterwards, so I felt it wasn't a good idea to use a path that needs manipulation at this stage, so I changed it to ls /mnt/[A-Z] which does not need any escaping or manipulation.

[psyngw]

Shouldn't we be running bash -c "PREVIEW_SCRIPT --bash-test"

I think we are exec the bash script directly so we don't need bash -c to run a command on windows, so cmd.exe is ok to do this.And for why I don't use ls /mnt/, I have mentioned that.

I didn't use ls to verify, because I think it seems to work in most cases but I still want to be more stable, so I added a parameter to preview.sh to test it, doesn't affect anything, I don't know what you think about this.

It seems that you still prefer ls, and I think it's fine, it should be good enough for 99.99999% of the cases : )

At last I push a new commit to fix two lines, I found no problems elsewhere and I have tested the items I tested before and they all worked ~ sorry for forced pushing because the test env is too much to see the wrong file : (

[junegunn]

I think we are exec the bash script directly

This I'm not so sure about. According to :help system(), it's a function that "Get the output of the shell command {expr} as a String". On my MacBook, echo system('/bin/echo foo') works as expected, but it fails once I unset shellcmdflag which is by default -c, and it seems to tell it's not directly "exec"ing /bin/echo.

sorry for forced pushing

No problem, will merge this, thanks!

[psyngw]

but it fails once I unset shellcmdflag

Ah, that's where I couldn't see and consider, I'll check it later.

Anyway, thank you very much for some advice and suggestions, I have learned a lot : )

[junegunn]

Merged, thanks for the work!