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

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

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

Comments

@zikaeroh
Copy link

@zikaeroh zikaeroh commented Nov 13, 2019

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

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Nov 13, 2019

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.

@heschik

This comment has been minimized.

Copy link
Contributor

@heschik heschik 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

This comment has been minimized.

Copy link
Author

@zikaeroh zikaeroh commented Nov 13, 2019

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

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Nov 14, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.