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 usability of source.gc_details code action #71106

Open
adonovan opened this issue Jan 3, 2025 · 3 comments
Open

x/tools/gopls: improve usability of source.gc_details code action #71106

adonovan opened this issue Jan 3, 2025 · 3 comments
Assignees
Labels
gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@adonovan
Copy link
Member

adonovan commented Jan 3, 2025

"Toggle compiler optimization details" is now a code action, which is progress. However the user experience still leaves a lot of room for improvement:

  1. We should show distinct "(Show|hide) compiler optimization details for package p" messages, as this (a) makes the current and intended states clear and (b) conveys that there is one state variable per package.

  2. There is an asymmetry in the command handler, which toggles the state flag for the narrowest package for a given file. Consequently, toggling it from an in-package test file will enable it for the test package, causing diagnostics to appear on the production code files too; but toggling it "off" from one of those files will actually enable it again for the primary package. And for reasons I don't understand, we don't see any diagnostics from the in-package test file. We should do better. Perhaps a single flag for all the packages in a given directory might be simpler.

  3. The compiler doesn't get as far as the optimizer if there were compilation errors, but this is impossible to tell from the user interface: it just silently does nothing. We should document this limitation, and surface a diagnostic for each file's package decl (or perhaps even each function, so that it is visible?) saying "couldn't optimize this function due to compilation error", or somesuch.

@adonovan adonovan added the gopls Issues related to the Go language server, gopls. label Jan 3, 2025
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Jan 3, 2025
@gopherbot gopherbot added this to the Unreleased milestone Jan 3, 2025
@findleyr
Copy link
Member

findleyr commented Jan 3, 2025

We should address these for v0.18.0, since the code action is more visible. Responses below:

  1. Definitely. This should be an easy fix.
  2. I think we should change the toggle to be per-file, since that is a more obvious UX. Then we can derive the set of packages to be a covering set for the toggled files. This avoids any side effects from invoking the toggle on a file.
  3. The behavior w.r.t. compilation errors is the same as for analyzers. I'm not so sure it's that confusing. If there are errors in the problems panel, it is reasonable that other features won't work. (perhaps errors are not prominent enough in emacs+eglot).

@adonovan
Copy link
Member Author

adonovan commented Jan 3, 2025

2: The unit of compilation in Go is the package, and when optimizing it's common to be working on more than one file in the package, so I think it makes more sense for the flag to have per-package granularity... but the intuitive notion of package gets a little fuzzy around tests, which is why I suggest using the directory. It avoids the need to apply a second file-based filter to the results we got from the compiler, and it's easier to explain.

3: What makes it different from analyzers is that it is explicitly enabled by a user action, then theywait for quite a while... and nothing happens. That's why I think it needs some kind of feedback to confirm it did what was asked.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/640395 mentions this issue: gopls/internal/golang: improve "toggle compiler opt details"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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
Development

No branches or pull requests

3 participants