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: improve highlighting #34496

Closed
stamblerre opened this issue Sep 24, 2019 · 6 comments
Closed

x/tools/gopls: improve highlighting #34496

stamblerre opened this issue Sep 24, 2019 · 6 comments
Assignees
Milestone

Comments

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Sep 24, 2019

Document highlight doesn't provide highlighting for fields or multiple uses of the same function. Should it? This issue will track missing behaviors for the textDocument/highlight command.

@stamblerre

This comment has been minimized.

Copy link
Contributor Author

@stamblerre stamblerre commented Nov 18, 2019

@ridersofrohan will be working on this

The first step here would be to improve the tests, which are not very robust (see testdata/highlights/highlights.go). The issue here is that the tests seem to only be testing one result, even though the expectations for the tests seem to be correct (for every test case, there is a list of expected ranges, as you can see in internal/lsp/tests/tests.go). The correct approach here would be to have a marker for each expected result, and then the tests can look something more like:

var foo = F{bar: 52} //@mark(fooDeclaration, "foo"),highlight("foo", fooDeclaration, fooUse)

func Print() {
	fmt.Println(foo) //@mark(fooUse, "foo"),highlight("foo", fooDeclaration, fooUse)
}
@ridersofrohan

This comment has been minimized.

Copy link

@ridersofrohan ridersofrohan commented Nov 22, 2019

Commit 0a3398

Improved tests for highlights by allowing you to specify what should be highlighted for each mark. Also fixed issue #2785.

@ridersofrohan

This comment has been minimized.

Copy link

@ridersofrohan ridersofrohan commented Nov 22, 2019

Commit 66c5a5

Adds control flow highlighting within for loops.

@ridersofrohan ridersofrohan self-assigned this Nov 26, 2019
@stamblerre

This comment has been minimized.

Copy link
Contributor Author

@stamblerre stamblerre commented Dec 4, 2019

I think that this can be closed after https://go-review.googlesource.com/c/tools/+/208267. Thanks @ridersofrohan!

@stamblerre stamblerre closed this Dec 4, 2019
@stamblerre stamblerre modified the milestones: Unreleased, gopls v1.0 Dec 4, 2019
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Dec 13, 2019

Change https://golang.org/cl/211338 mentions this issue: internal/lsp: skip return highlighting if cursor is in arglist of func

gopherbot pushed a commit to golang/tools that referenced this issue Dec 13, 2019
If the cursor is within an argument that is within a callExpr which is
in a return statement, we only want it to highlight the ident that the cursor
is in. We do not want it to highlight the entire function.

Updates golang/go#34496

Change-Id: If4025660a99fd5df90098e0560a5e9e7260e33c8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211338
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Dec 19, 2019

Change https://golang.org/cl/212137 mentions this issue: internal/lsp: fix highlighting of inexact matches

gopherbot pushed a commit to golang/tools that referenced this issue Dec 19, 2019
…rom the right

There is an issue when highlighting a single character identifier if the cursor is on the right of it. This problem
is a result of an assumption made in astutil.PathEnclosingInterval relating to the arguments passed in. Specifically,
if start==end, the 1-char interval following start is used instead. As a result, we might not get an exact match
so we should check the 1-char interval to the left of the passed in position to see if that is an exact match.

Updates golang/go#34496

Change-Id: If689fdf695df6ec1bc1935088e50d3de055bd5db
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212137
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.