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

Jump to bottom

x/tools/gopls: only use import text as range for textDocument/documentLink responses #35565

Closed
zikaeroh opened this issue Nov 13, 2019 · 4 comments
Closed

x/tools/gopls: only use import text as range for textDocument/documentLink responses #35565

zikaeroh opened this issue Nov 13, 2019 · 4 comments
Labels
Documentation FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@zikaeroh
Copy link
Contributor

Please answer these questions before submitting your issue. Thanks!

What did you do?

Have import statements, a mix of normal imports, aliased imports, _ imports.

What did you expect to see?

Links which span the package path.

What did you see instead?

The links span the entire import statement, and not just the link. See:

image

Note how the package aliases are also underlined, and even the _ imported package. Including the quotes also feels wrong. I would personally prefer them to look like:

image

Which is just a change to:

toProtocolLink(view, m, target, n.Path.Pos()+1, n.End()-1)

(But I'm not sure if this is the correct way to get the span of a literal, just a PoC.)

Build info

golang.org/x/tools/gopls master
    golang.org/x/tools/gopls@v0.1.8-0.20191113163402-bc1376d635b8 h1:xSNm5FMZDYQjehCHcVzpGm4D9oNz/+AD+C/pRROxjMk=
    github.com/BurntSushi/toml@v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
    github.com/sergi/go-diff@v1.0.0 h1:Kpca3qRNrduNnOQeazBd0ysaKrUJiIuISHxogkT9RPQ=
    golang.org/x/sync@v0.0.0-20190423024810-112230192c58 h1:8gQV6CLnAEikrhgkHFbMAEhagSSnXWGV915qUMm9mrU=
    golang.org/x/tools@v0.0.0-20191113163402-bc1376d635b8 h1:3xDEBhGRwlUGJue2GW3jVaqWs/HrAm+Lz4HAoQVWuJs=
    golang.org/x/xerrors@v0.0.0-20190717185122-a985d3407aa7 h1:9zdDQZ7Thm29KFXgAX/+yaf3eVbP7djjWp/dXAppNCc=
    honnef.co/go/tools@v0.0.1-2019.2.3 h1:3JgtbtFHMiCmsznwGVTUWbgGov+pVqnlf1dEJTNAXeM=

Go info

go version go1.13.4 linux/amd64

GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/jake/.cache/go-build"
GOENV="/home/jake/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/jake/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/jake/zikaeroh/hortbot/hortbot/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build705508798=/tmp/go-build -gno-record-gcc-switches"
@gopherbot gopherbot added this to the Unreleased milestone Nov 13, 2019
@gopherbot gopherbot added Documentation Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Nov 13, 2019
@gopherbot
Copy link

Thank you for filing a gopls issue! Please take a look at the Troubleshooting guide, and make sure that you have provided all of the relevant information here.

@heschi
Copy link
Contributor

heschi commented Nov 13, 2019

We can't take patches in issues, but I agree that linking just the import path would be better. Would you like to send a PR/CL?

@zikaeroh
Copy link
Contributor Author

zikaeroh commented Nov 13, 2019 •
edited

I will look into it later today; I typically only report issues and haven't signed the CLA (need to figure out if I can submit a patch / if I should be including my name, etc). Feel free to write something better, I'm not convinced manual tweaking of token positions is right either and I only included the tweak because it was really easy to do.

@gopherbot
Copy link

Change https://golang.org/cl/207137 mentions this issue: internal/lsp: use import path literal for documentLink range

@golang golang locked and limited conversation to collaborators Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Documentation FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
3 participants
@gopherbot @heschi @zikaeroh