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

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

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

rwxguo
Copy link
Contributor

@rwxguo rwxguo commented Dec 16, 2023

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 rwxguo reopened this Dec 18, 2023
autoload/go/def.vim Outdated Show resolved Hide resolved
@rwxguo
Copy link
Contributor Author

rwxguo commented Dec 25, 2023

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.

@rwxguo rwxguo closed this Dec 25, 2023
@rwxguo rwxguo reopened this Dec 25, 2023
@rwxguo rwxguo changed the title mingw: Resolve path issue, make gopls work on GitBash (:GoDoc, :GoInfo and :GoDef) mingw: Resolve path issue, make gopls work on CYGWIN/MSYS2/GitBash (:GoDoc, :GoInfo and :GoDef) Dec 25, 2023
@bhcleek
Copy link
Collaborator

bhcleek commented Mar 22, 2024

@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
Copy link
Contributor Author

rwxguo commented Mar 22, 2024

@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. ;)

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

Successfully merging this pull request may close these issues.

2 participants