Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
rwxguo wants to merge10 commits intofatih:master
base:master
Choose a base branch
Loading
fromrwxguo:gopls-gitbash-compatible

Conversation

@rwxguo
Copy link
Contributor

@rwxguorwxguo commentedDec 16, 2023
edited
Loading

This patch can resolve the following issues:

The root cause is thatgopls cannot identify the correct path sent fromvim-go.

When talking about cygwin, it generally means theCYGWIN,MSYS2 andGitBash (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 pathc:\mypath. However, when communicating withgopls, 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 thegopls 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 toMSYS2/GitBash. This should also be handled.

Thus, the following matrix is showing the path compatibility:

PathCYGWINMSYS2GitBashWindows
c:\path (\ and/)⭕ (c:\path or"c:/path")
/c/path
/cygdrive/c/path
/c:/path

I didn't modify the logic ofgo#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 checkinghas('win32unix') and it should be enough. Refer to thebhcleek's reply. ForCYGWIN specific checking, we can useuname shell command.

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

@rwxguorwxguo reopened thisDec 18, 2023
@rwxguo
Copy link
ContributorAuthor

rwxguo commentedDec 25, 2023
edited
Loading

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

@rwxguorwxguo closed thisDec 25, 2023
@rwxguorwxguo reopened thisDec 25, 2023
@rwxguorwxguo changed the titlemingw: 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 commentedMar 22, 2024
edited
Loading

@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 bePATH,GOPATH, andGOBIN 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 reacted with thumbs up emoji

@rwxguo
Copy link
ContributorAuthor

@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 freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@bhcleekbhcleekbhcleek left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@rwxguo@bhcleek

[8]ページ先頭

©2009-2025 Movatter.jp