Fix fzf with no cmdline arguments on Cygwin#4383
Fix fzf with no cmdline arguments on Cygwin#4383clarity20 wants to merge 1 commit intojunegunn:masterfrom
Conversation
|
This PR makes fzf work in my Cygwin64 installation on Windows 10. |
| argStr := escapeSingleQuote(args[0]) | ||
| _argStr := "" | ||
|
|
||
| osname, _ := exec.Command("uname", "-s").Output() |
There was a problem hiding this comment.
So we would have to start a process to determine if we're running Cygwin, but this has a cost. Can there be a lighter way to do it, like checking if an environment variable exists?
There was a problem hiding this comment.
You have a good point. Based on their respective man pages, in bash and zsh we can check that os.Getenv("OSTYPE") == "cygwin", but fish makes no mention of OSTYPE and I don't see a better option than uname in that case. Perhaps that little complication can be ironed out by looking at opts.{Bash,Zsh,Fish}?
There was a problem hiding this comment.
Is there any environment variable that shows you're on Cygwin? Something like $CYGWIN or $CYG*?
There was a problem hiding this comment.
os.Getenv() picks up on OSTYPE but again that leaves the fish shell in the lurch.
However, bash also has a BASH_VERSINFO and I see that BASH_VERSINFO[5]="x86_64-pc-cygwin" in my setup. I am looking for something analogous on fish. If I find it I will keep you updated.
There was a problem hiding this comment.
And telling folks to setup an environment variable on their system would not be an option? In case we cannot find a common pre-existing environment variable, I mean.
There was a problem hiding this comment.
I like your idea of having users set up their own envs. In my opinion they should also be reminded to set this value separately in each environment where they want to run fzf.
FWIW, my research inclines me to think there really isn't a "grand unified" approach through canonical environment or shell variables. See for instance this discussion on Stack Overflow.
There was a problem hiding this comment.
So there is no special environment variable that is guaranteed to be present in Cygwin, right? That's unfortunate.
Just to be clear, IIUC, this issue is caused by the use of winpty, so what happens when you tell fzf not to use it?
fzf --no-winptyThere was a problem hiding this comment.
Maybe this can help. In windows it is almost warrantee that the OS environment variable will exist with the value Windows_NT. It seems to have been there since windows XP and from my experience it is very useful to detect windows https://stackoverflow.com/questions/2788720/environment-variable-to-determine-the-os-type-windows-xp-windows-7.
Then you can check if BASH_VERSION, ZSH_VERSION or FISH_VERSION exists. Each one of them should be available when running the respective shell in interactive mode.
Now, I do not now of any cygwin specific variable but you can check for the existence of MSYSTEM which is set by msys or git bash. If it is not present, it should be very likely that you are running cygwin.
There was a problem hiding this comment.
You can simply check PATH.
When fzf is called from Cygwin, PATH should contain the bin directory of Cygwin for (almost) 100%. I think you also want to check MSYS2 paths because there should be the same problem in MSYS2, which is a fork of Cygwin. MSYS2 is the base system of MinGW and Git for Windows, so its user base isn't negligible. If you care about the cases where Cygwin/MSYS2 is installed in a strange path, you can instead check if cygwin1.dll / msys-2.0.dll / msys-1.0.dll exists in the PATH.
This addresses issue #4382.
In module
src/winpty_windows.go, functionrunWinpty()is handing off the executable's full pathname torunProxy()as the first argument. The path was coming over Windows-style and causing Cygwin to choke. So I'm checkinguname -sfor Cygwin and callingcygpathto reformat if needed. Very basic test runs now looking good: