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.

Sign up for GitHub

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 fmt.Stringer representation of a const on mouse over #61838

Open
biohazduck opened this issue Aug 8, 2023 · 3 comments
Open

x/tools/gopls: show fmt.Stringer representation of a const on mouse over #61838

biohazduck opened this issue Aug 8, 2023 · 3 comments
Labels
FeatureRequest gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
gopls/unplanned

Comments

@biohazduck
Copy link

What did you do?

Moused over Meters or any of the other consts.

package si

type Distance int64

const (
      Nanometers  = 1
      Micrometers = 1000 * Nanometers
      Millimeters = 1000 * Micrometers
      Meters      = 1000 * Millimeters
)

// String pretty formats the units to a reasonable human readable representation i.e. "100m", "500nm", 
func (v Velocity) String() string {
      return formatWithBase(v, baseNano, 'm')
}

What did you expect to see?

What I want is what is currently done today with the time.Duration type:
image

I understand this implies a HUGE change to gopls, since it would mean somehow running arbitrary code. This carries some worrysome implications:

  • Security risk
  • How to implement this without exploding gopls binary size
  • Recursive risk

I'd understand if this cannot be done today, however I want to start the discussion since this would be a large quality of life improvement for me since I regularly work with SI unit consts to define program behaviour.

I'd also like to refer you all the TinyGo interp package which safely evaluates Go code on initialization: https://github.com/tinygo-org/tinygo/tree/release/interp.

What did you see instead?

Got very long numbers which are not immediately interpretable, much less when defining other more complex consts with the Distance type.
image
@biohazduck biohazduck added gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. labels Aug 8, 2023
@gopherbot gopherbot added this to the Unreleased milestone Aug 8, 2023
@findleyr
Copy link
Contributor

findleyr commented Aug 8, 2023

Hi, thanks for starting the conversation.

As you note, gopls doesn't run user code so this would be a huge leap, and one we're unlikely to take. We'd probably need some sort of sandboxed interpreter as you suggest. That's a ton of engineering and risk, for arguably marginal improvement.

In the meantime, could we improve the statically generated hover text? For example by using scientific notation (1e9) or thousands separators (1_000_000_000)?

@jamalc jamalc modified the milestones: Unreleased, gopls/unplanned Aug 10, 2023
@soypat
Copy link

soypat commented Aug 12, 2023 •
edited

could we improve the statically generated hover text

Yes! Both options seem better than what is present today. One may be better than the other depending on the amount of trailing zeros though.

Implementation idea: Prefer scientific when significant figures is 3 or less:

1e3   // ok
13e3  // ok
132e3 // ok
1323e3 // not ok, should be 1_323_000

Edit: Another idea: leave a hint for gopls to format numeric const whose type has units:

// Distance is a fixed point representation based on nanometers (1e-9m).
//
//gopls:units=si base=-9 rune=m
type Distance int64

@adonovan
Copy link
Member

We could interpret String methods that consist of a single call to fmt.Sprintf, but I doubt it would cover enough cases to make it worthwhile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
FeatureRequest 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
gopls/unplanned
Development

No branches or pull requests
6 participants
@adonovan @gopherbot @jamalc @soypat @findleyr @biohazduck