search

fatih / vim-go

Go development plugin for Vim
https://www.patreon.com/bhcleek
Other
15.98k stars 1.45k forks source link

mingw: Resolve path issue, make gopls work on CYGWIN/MSYS2/GitBash (`:GoDoc`, `:GoInfo` and `:GoDef`) #3612

Open rwxguo opened 9 months ago

rwxguo commented 9 months ago

This patch can resolve the following issues:

The root cause is that gopls cannot identify the correct path sent from vim-go.

When talking about cygwin, it generally means the CYGWIN, MSYS2 and GitBash (main-stream system).

Cygwin mixed the 2 path systems (Win & Unix) which benefit from two world, but also brought some problem. The path like /c/mypath is used to represent the corresponding windows path c:\mypath. However, when communicating with gopls, the path becomes /c:/mypath (please note there is a :) which is neither cygwin path nor windows path. It is probably because the GitBash parse the path from/to the gopls with a forward slash prefix /. The problem is how to handle the comma properly.

Another finding is that, CYGWIN path has a prefix /cygdrive compare to MSYS2/GitBash. This should also be handled.

Thus, the following matrix is showing the path compatibility:

Path CYGWIN MSYS2 GitBash Windows
c:\path (\ and /) ✅ ✅ ✅ ⭕ (c:\path or "c:/path")
/c/path ❌ ✅ ✅ ❌
/cygdrive/c/path ✅ ❌ ❌ ❌
/c:/path ❌ ❌ ❌ ❌

I didn't modify the logic of go#util#IsWin() because it is called by many places in the program, to prevent other issues happen. Instead, I treated the Cygwin system as another system (Neither Windows, nor Unix).

Vim provide a cygwin checking has('win32unix') and it should be enough. Refer to the bhcleek's reply. For CYGWIN specific checking, we can use uname shell command.

I have only tested :GoDoc, :GoInfo and :GoDef, which I think are the most popular functions for cygwin users.

rwxguo commented 9 months ago

BTW, to make code clean, I would further change my cygwin checking to using the go#util#IsCygwin created in this PR after it's merged.

bhcleek commented 6 months ago

@rwxguo I know this and the other PRs you've submitted, #3611 and #3608, have been sitting for a while. Thank you for submitting these, and my apologies for the delay. I wanted to give you some idea for why they've been sitting unmerged, though.

These are all related and go to a common problem, so I want to merge them via a manual octopus merge instead of relying on GitHub's merge pull request functionality. I plan to do that octopus merge locally on my machine and then test this fully. I suspect these three PRs are not sufficient to fully address the problems littered throughout vim-go when trying to use a Vim compiled with mingw, and if vim-go is going to tackle this, then I'd like to address it fully if possible.

Also, I'm not convinced this is the right way to go. When using vim-go with Vim compiled with mingw, there's a strong argument that the Go version used to compile the helper binaries (e.g. gopls, dlv, etc.) should also have been compiled with mingw (e.g. https://packages.msys2.org/base/mingw-w64-go). I think that might resolve all these problems naturally, though I'm not certain. I'm sure there would be PATH, GOPATH, and GOBIN considerations, too, if that approach were chosen. As an alternative to that idea, users could use a Vim that's native to their system instead of trying to use a mingw Vim, vim-go, and binaries that are not derivative of mingw.

rwxguo commented 6 months ago

@bhcleek Totally agree. Thanks for your kind reply and detailed explanation. Thanks for keeping an eye on my changes, that's very kind of you, and your direction is totally correct. I myself also use native vim with vim-go. Mingw has many problem not only in vim-go. And I will feel contented just if my workaround can help anyone who are trmporarely using vim-go with mingw at the moment. But that workaround should not be merged just as you said, which is not good way enough to resolve the issue of mingw. Take it easy, and happy coding. ;)