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: show hover information for runes #38239

Closed
stamblerre opened this issue Apr 3, 2020 · 9 comments
Closed

x/tools/gopls: show hover information for runes #38239

stamblerre opened this issue Apr 3, 2020 · 9 comments
Labels
FeatureRequest FrozenDueToAge gopls Issues related to the Go language server, gopls. help wanted Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
gopls/v0.7.3

Comments

@stamblerre
Copy link
Contributor

From microsoft/vscode-go#3149:

In Go, runes can be expressed as single-quoted readable glyphs, single-quoted escaped \u codes, or number literals (usually hex). It’s often hard to know what a particular rune is, short of extensive commenting as in this example.

It would be very helpful in VSCode to hover over any of these forms to get info on the rune, such as:

  • visual glyph (if any)
  • description e.g. “ARABIC THOUSANDS SEPARATOR”
  • \u representation
  • hex representation

Thanks for the consideration, happy to discuss.

This can be implemented using https://pkg.go.dev/golang.org/x/text/unicode/runenames.
@gopherbot gopherbot added this to the Unreleased milestone Apr 3, 2020
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Apr 3, 2020
@stamblerre stamblerre mentioned this issue Apr 3, 2020
Closed
@stamblerre stamblerre removed this from the Unreleased milestone Jul 23, 2020
@stamblerre stamblerre added this to the gopls/unplanned milestone Oct 21, 2020
@jhchabran
Copy link

If this ticket is still up to date, I'd like to implement it.

Is that okay if I take it @stamblerre?

@stamblerre
Copy link
Contributor Author

Absolutely, @jhchabran! Please comment here if you need any additional guidance.

@jhchabran
Copy link

jhchabran commented May 3, 2021 •
edited

Thanks! I think I need some guidance regarding the marker tests.

I wrote some regtests for hovering runes and got them green, but I am unsure how to proceed with the marker tests or even if I should write some for this feature.

From what I understand, roughly speaking, the @hover("something", mark) works by matching something and checking that hovering it references the object that was marked.

Hovering a rune does not reference anything, so I don't see how I could express that with a marker test, assuming I understood how it presently works. Maybe some self-reference?

Do you think I should dig into the markers more and eventually see how they could be made to work with the rune hovering or the regtests would be enough for such an "isolated" feature?

@stamblerre
Copy link
Contributor Author

Thanks for working on this! The regtests might be sufficient for this case, but if you'd like to add marker-based tests, you could also adjust the way that the @hover test works, here: https://github.com/golang/tools/blob/062bf4eb8ab9c68e2c22214899f3e17b66ddbbf3/internal/lsp/tests/tests.go#L437.

@jhchabran
Copy link

jhchabran commented May 14, 2021 •
edited

Thanks for the pointer! I spent more time on the markers and got how they work.
It now basically looks like this:

// internal/lsp/testdata/basiclit/baciclit.go
// (...) 
_ = '\U0001F30A' //@hovertooltip("'\\U0001F30A'", "'🌊', U+1F30A, WATER WAVE")

Another marker called @hovertooltip was added because modifying the @hover to make it work with expecting a reference to a definition or a direct string representation of the expected result is probably too convoluted and unclear to read.

➡️ Is that okay to have added another one? I think it makes sense especially if there are other hovers that may be added later on.

I also get the feeling that renaming @hover to @hoverdef would make more sense now, but that's probably better to discuss it over a CL rather than here.

@stamblerre
Copy link
Contributor Author

Yep, totally fine to add a new marker type! Happy to continue the conversation over a CL.

jhchabran added a commit to jhchabran/tools that referenced this issue May 21, 2021
@jhchabran
internal/lsp: Add support for hovering runes
d11d901 
Runes expressed in various forms can now be hovered within basic
litterals. A quick summary is displayed if a rune is found: a printable
version (if it exists), its codepoint and its name.

Rune literals always display the summary when hovered, string litterals
only display it when a escaped rune sequence is used finally, number
litterals only when its expressed as a hexadecimal number whose size
ranges from one to eight bytes.

Fixes golang/go#38239
jhchabran added a commit to jhchabran/tools that referenced this issue May 21, 2021
@jhchabran
internal/lsp: Add support for hovering runes
fdefb12 
Runes expressed in various forms can now be hovered within basic
litterals. A quick summary is displayed if a rune is found: a printable
version (if it exists), its codepoint and its name.

Rune literals always display the summary when hovered, string litterals
only display it when a escaped rune sequence is used finally, number
litterals only when its expressed as a hexadecimal number whose size
ranges from one to eight bytes.

Fixes golang/go#38239
jhchabran added a commit to jhchabran/tools that referenced this issue May 21, 2021
@jhchabran
internal/lsp: Add support for hovering runes
fe67cd1 
Runes expressed in various forms can now be hovered within basic
litterals. A quick summary is displayed if a rune is found: a printable
version (if it exists), its codepoint and its name.

Rune literals always display the summary when hovered, string litterals
only display it when a escaped rune sequence is used finally, number
litterals only when its expressed as a hexadecimal number whose size
ranges from one to eight bytes.

Fixes golang/go#38239
jhchabran added a commit to jhchabran/tools that referenced this issue May 22, 2021
@jhchabran
internal/lsp: Add support for hovering runes
405a5b7 
Enable to hover runes expressed in various forms within basic literals.
A quick summary is displayed if a rune is found: a printable version (if
it exists), its codepoint and its name.

Rune literals always display the summary when hovered, string literals
only display it when an escaped rune sequence is foudn, and number
literals only when it is expressed as a hexadecimal number whose size
ranges from one to eight bytes.

Fixes golang/go#38239
@gopherbot
Copy link

Change https://golang.org/cl/321810 mentions this issue: internal/lsp: Add support for hovering runes

@jhchabran
Copy link

@stamblerre Being the first time I submit a CL, I just wanted to be sure that I am not missing anything that would block the process. Is there something else to do on my part before my CL (see above) gets reviewed ?

If not, I'll happily wait until someone reviews it :)

@stamblerre
Copy link
Contributor Author

Thanks for pinging this, @jhchabran! Looks like something went wrong and owners didn't get added as reviewers. I've added myself and will take a look soon.

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

No branches or pull requests
3 participants
@jhchabran @stamblerre @gopherbot