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

fzf-git functions not working on MSys2 bash (Git bash) #4

Closed
rashil2000 opened this issue Aug 24, 2021 · 7 comments
Closed

fzf-git functions not working on MSys2 bash (Git bash) #4

rashil2000 opened this issue Aug 24, 2021 · 7 comments

Comments

@rashil2000
Copy link

Not sure if this is the right place to ask for help, but I want to give it a shot anyways.

While the functions themselves are bash statements, fzf' preview argument gets executed under cmd (and not bash) on Windows. This causes certain problems, listed below:

  1. Semicolon, used in the preview argument here, is not a command separator in cmd.
    --preview '(git diff --color=always -- {-1} | sed 1,4d; cat {-1}) | head -500' |
  2. In the preview argument here, the <<< operator is used which does not exist in cmd. It also uses command substitution $(...) which cmd doesn't support.
    --preview 'git log --oneline --graph --date=short --color=always --pretty="format:%C(auto)%cd %h%d %s" $(sed s/^..// <<< {} | cut -d" " -f1) | head -'$LINES |
  3. The <<< operator is used here too.
    --preview 'grep -o "[a-f0-9]\{7,\}" <<< {} | xargs git show --color=always | head -'$LINES |

It might be too much to ask, but is there a way to modify them so that they don't use such operators?

@akinomyoga
Copy link
Owner

Thank you for the report! These functions are actually copied from https://gist.github.com/junegunn/8b572b8d4b5eddd8b85e5f4d40f17236, so maybe you also want to report it there.


It might be too much to ask, but is there a way to modify them so that they don't use such operators?

You can copy the file, edit it as you like and load the edited file in ~/.blerc.

$ mkdir -p ~/.local/share/blesh/local
$ cp /path/to/contrib/fzf-git.bash ~/.local/share/blesh/local/my-fzf-git.bash
$ EDIT ~/.local/share/blesh/local/my-fzf-git.bash
# blerc

ble-import -d my-fzf-git

The custom search path of ble-import can be configured by bleopt import_path whose default value is ${XDG_DATA_HOME:-$HOME/.local/share}/blesh/local. The option import_path is a colon-separated list of directory paths just like the PATH environment variable.

@rashil2000
Copy link
Author

These functions are actually copied from https://gist.github.com/junegunn/8b572b8d4b5eddd8b85e5f4d40f17236, so maybe you also want to report it there.

Yes, I knew that but the gist interface isn't really very useful for giving feedback (and upon seeing the thread, the author doesn't seem to be very active in the comments).

Anyway, I doubt that I can edit the functions myself and make them work 😅

Thanks for quick reply. Should I close this then?

@akinomyoga
Copy link
Owner

akinomyoga commented Aug 24, 2021

Maybe we can work around it by specifying the shell in the following way:

-    --preview 'xxxxx' | 
+    --preview 'bash -c "xxxxx"' | 

but I'm not sure if that is the appropriate or intended way of fzf. I have checked the source code of fzf. The preview command is used in the following piece of codes in fzf.

util.ExecCommand is defined for Unix and Windows differently:

We notice that we can change the executing shell through the environment variable SHELL in Unix but not in Windows. Maybe we could ask for fzf to recognize some environment variables for the executing shell also in the Windows environment. Also, I'm wondering if it is possible to build fzf targeted for MSYS2 which provides the Unix API.

  1. So the first thing you should do is to try to build the proper fzf for the MSYS2 environment using Unix API of MSYS2. If you don't know how to do that, please ask it at fzf.
  2. If that is technically difficult, then you can next request fzf adding the environment variable or option to change the executing shell from cmd to a user-specified one.
  3. If that was not accepted, finally we need to edit the preview command as bash -c "xxxxx" for the MSYS2 environment though I don't think it is the proper way to solve the problem.

@rashil2000
Copy link
Author

rashil2000 commented Aug 25, 2021

So the first thing you should do is to try to build the proper fzf for the MSYS2 environment using Unix API of MSYS2. If you don't know how to do that, please ask it at fzf.

I don't think this is possible. Golang only supports the native toolchain on Windows. While there are numerous Unix toolchains for Golang, they include neither Cygwin nor MSys2. The only way to do this is to build Golang itself from source by studying and defining the appropriate Go build flags (corresponding to the ones listed here) and use that to build fzf.

Surprisingly, the Cygwin repos do include a very old port of fzf. Looks like it was manually patched for Cygwin. No luck for MSys2 though. The Go compiler can be installed through pacman in MSys2, but that package is in the mingw repos so I don't think it uses the Unix APIs.

If that is technically difficult, then you can next request fzf adding the environment variable or option to change the executing shell from cmd to a user-specified one.

This is a very tricky request. Let's say we propose a variable $FZF_DEFAULT_SHELL. Most people who use Windows will likely not have bash in their path (the bash being either from Git, MSys2 or Cygwin), so this makes the option very niche.

Also, since the introduction of WSL, there actually is a bash in the path, pointing to C:\\Windows\\System32\\bash.exe, which is WSL's bash. We obviously don't want to accidentally use this bash as it will cause immense slowdowns and possibly wrong results.

If that was not accepted, finally we need to edit the preview command as bash -c "xxxxx" for the MSYS2 environment though I don't think it is the proper way to solve the problem.

This might not sound good on principle, but it might just be the simplest and most non-invasive workaround. We can specify bash -c xxxxx only for the preview window as required, and we'll also avoid unexpected problems that can arise due to the intermingling of escape sequences (fzf doesn't even work on mintty terminal FWIW) if bash is used as the default execution binary for fzf on Windows.

Edit: typo

@akinomyoga
Copy link
Owner

akinomyoga commented Aug 25, 2021

So the first thing you should do is to try to build the proper fzf for the MSYS2 environment using Unix API of MSYS2. If you don't know how to do that, please ask it at fzf.

I don't think this is possible. Golang only supports the native toolchain on Windows. While there are numerous Unix toolchains for Golang, they include neither Cygwin nor MSys2. The only way to do this is to build Golang itself from source by studying and defining the appropriate Go build flags (corresponding to the ones listed here) and use that to build fzf.

Surprisingly, the Cygwin repos do include a very old port of fzf. Looks like it was manually patched for Cygwin.

Doesn't that mean it is possible to make it work in Cygwin (and also in its fork MSYS2) though I'm not sure how much it has been modified in patching?

Edit: I have checked fzf of Cygwin version, and it seems that it is the version that had been written in Ruby (fzf version 0.12.1-2). It's not patched; it just works without modification since it is written in portable Ruby.

If that is technically difficult, then you can next request fzf adding the environment variable or option to change the executing shell from cmd to a user-specified one.

This is a very tricky request. Let's say we propose a variable $FZF_DEFAULT_SHELL. Most people who use Windows will likely not have bash in their path (the bash being either from Git, MSys2 or Cygwin), so this makes the option very niche.

$FZF_DEFAULT_SHELL is not necessarily for bash, but can be other Windows shells.

Also, since the introduction of WSL, there actually is a bash in the path, pointing to C:\Windows\System32\bash.exe, which is WSL's bash. We obviously don't want to accidentally use this bash as it will cause immense slowdowns and possibly wrong results.

I'm not saying that we propose making the default shell bash but just providing the option that a user can set when the user wants to change it from cmd to something other.

We can specify sh -c xxxxx only for the preview window as required,

Use bash -c xxxxx. I'm wondering why you always try to do something different from what I suggested. Here strings <<< are non-POSIX feature so that you should explicitly specify a shell that supports here strings. In MSYS2, /bin/sh is bash, but that's just accidental. There is really no reason that you rewrite bash to sh.

unexpected problems that can arise due to the intermingling of escape sequences (fzf doesn't even work on mintty terminal FWIW) if bash is used as the default execution binary for fzf on Windows.

Are there any known problems that happen when the shell is changed to bash?

(fzf doesn't even work on mintty terminal FWIW)

I just tried fzf in mintty of MSYS2 without any modifications, and as you have told, it doesn't work. Then, I'm wondering what you try to do. This means that there is no working binary of fzf for MSYS2. Specifying --preview doesn't solve the problem. If there is no fzf that works for MSYS2, why do you bother configuring fzf for MSYS2 that will never work?

@rashil2000
Copy link
Author

rashil2000 commented Aug 26, 2021

Edit: I have checked fzf of Cygwin version, and it seems that it is the version that had been written in Ruby (fzf version 0.12.1-2). It's not patched; it just works without modification since it is written in portable Ruby.

Oh, is that so. I was aware that fzf was originally in Ruby, but I forgot at which version the author switched to Go.

$FZF_DEFAULT_SHELL is not necessarily for bash, but can be other Windows shells... I'm not saying that we propose making the default shell bash but just providing the option that a user can set when the user wants to change it from cmd to something other.

Yes yes, it won't necessarily be bash. cmd is default, the other option on Windows would be powershell. But since most of the useful and interesting tooling around fzf is written in bash (and its friends), even powershell would be of no help. I was just going with the popular choice here.

Use bash -c xxxxx. I'm wondering why you always try to do something different from what I suggested. Here strings <<< are non-POSIX feature so that you should explicitly specify a shell that supports here strings. In MSYS2, /bin/sh is bash, but that's just accidental. There is really no reason that you rewrite bash to sh.

Oh shoot! I was typing that on phone; it's an extremely stupid typo (more so because sh is also a shell 😅). I actually meant bash -c xxxxx only. I have edited my comment.

Are there any known problems that happen when the shell is changed to bash?

Yes, too many to list down here... Try launching any Rust TUI applications written for Windows from bash. More details here - crossterm-rs/crossterm#580

I just tried fzf in mintty of MSYS2 without any modifications, and as you have told, it doesn't work. Then, I'm wondering what you try to do. This means that there is no working binary of fzf for MSYS2. Specifying --preview doesn't solve the problem. If there is no fzf that works for MSYS2, why do you bother configuring fzf for MSYS2 that will never work?

fzf doesn't work with mintty terminal. It does work if you use bash from conhost (the default older Windows terminal) or from the newer Windows Terminal.

Actually, most Windows-native binaries won't work on mintty - like ssh.exe (the win32 binary, not the MSYS2 binary), neovim.exe, cmd.exe, powershell.exe or wsl.exe. There's a way to mitigate this - by setting $MSYS='enable_pcon' (more on this can be found in this discussion). But this is also a hit-and-miss, it causes some other problems as listed by the mintty authors in that thread. (for example, fzf will successfully launch, but will fail to respond to any keystrokes, forcing you to kill the terminal).

@rashil2000
Copy link
Author

Closed by junegunn/fzf#2641

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants