-
Notifications
You must be signed in to change notification settings - Fork 18k
x/tools/gopls: symbol returns matches from outside the workspace #37236
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
Comments
This is intentional - I think it's useful to be able to go to the a symbol in a dependency of your workspace packages. Is there a particular reason you would want to remove this behavior? |
The only reason I questioned whether this was the intended behaviour or not is the discrepancy, under a very strict reading at least, between the fact the method is defined as returning workspace symbols but in fact returns symbols from outside the workspace. If this is working as intended that's absolutely fine (and in many respects better). I'll add another comment to #37237 to also request a module scope. |
This has come up a few times. Several users do not like seeing results outside their workspace. Given that this is a superficial change, I would not be opposed to adding e.g. a "symbolScope" setting that alters this behavior. |
Are there any updates on this topic? Is there a work-around? It realy hurts to not have a sane search for Go symbols in vscode. Here is the my current work-around: https://stackoverflow.com/a/75233923/633961 |
Thanks for pinging, @guettli. I still prefer the current behavior, but may be in the minority. I suspect that effectiveness of the current symbol search also depends on whether your client reorders results (invalidating our down-ranking of non-workspace symbols). My client does not reorder results, but VS Code does. I will add a symbolScope setting for v0.12.0 with two values: "workspace" and "all". We can start with "workspace" as the default. |
@findleyr Thank you for the good news. I think it depends on the your-code/library-code ratio. I develop a kubernetes operator (CRD). My code is only few lines, but I import a lot of packages. If I use ctrl-t (Go to Symbol in Workspace) I see ~50 matches. Only ~5 are from my code. We use code-vendoring. Please don't include the vendored code into the results. The vendored code is (according to my definition) not part of my code. |
Change https://go.dev/cl/490935 mentions this issue: |
Just driving by to re-up #37237 in this context. Something like the approach listed in that issue would allow the caller to define the scope on a per query basis. I'm constantly bugged by the fact that I get symbols outside of the current package, let alone the workspace! |
@guettli you can now test this out by building gopls@master: e.g.:
|
@findleyr thank you very, much ctrl+t show now my code only: old: works fine, thank you! |
But that would only occur if someone opted into using the advanced query syntax. Nothing would change by default. And as you say, because a) there isn't a good interface/UX for doing this in VSCode, and b) it performs its own filtering (incorrectly) then VSCode users would not be able to leverage such a feature. And it could be documented as such. My point is that we don't need nor are we required to gate implementing such an experimental feature on whether VSCode can support it or not. Such a feature would provide a serious improvement of experience for those users able to take advantage of it. |
@myitcv this issue is about symbols outside the workspace. Did this change help you? |
@guettli - not really. Because the scope in which I want to search for symbols changes. Sometimes I want to search across the equivalent of the Having a single static config option doesn't much help therefore. Not to say that it wouldn't help others, I'm just drawing a connection to a previously discussed issue. |
Understood. I was just trying to convey priorities: right now we have a lot of high priority, impactful work to do for gopls (releasing our new, scalable architecture, working on making gopls zero-config, improving workspace support, etc). Realistically, we are unlikely to prioritize this work until more users could benefit from it. Also: let's see what the feedback is from this current change. |
@findleyr - I had missed this detail until I happened to start using ddfa2200ae0bde969aa31087e186187f4fa91da0. Why did you decide to make this the default, given the current behaviour is Would it not be better to have people opt into this, rather than changing the default behaviour? Or does "several users" actually translate to "the majority of users"? |
@myitcv we don't actually know what our users want: we only hear from users via surveys and issues, and I have only ever heard users express that they are suprised when workspace/symbol responses return results outside their opened workspace folder. We don't have evidence that folks like the current behavior: I feel like I've been a bit stubborn in keeping it, because I prefer it. However, your question has made me reconsider this decision: it may be more prudent to start with the default value of "all", and solicit feedback as to whether we should change the default. Questions like these are why we want to add telemetry to gopls: this type of question does not usually fit the scope of a survey, and yet by seeing whether folks actually jump to a symbol outside the workspace we'd immediately know whether the current behavior is useful. |
Change https://go.dev/cl/494217 mentions this issue: |
Following discussion on golang/go#37236, let's be a bit careful before changing behavior. For gopls@v0.12.0, we can keep the default of this setting at "all", and solicit feedback for which default our users prefer. Updates golang/go#37236 Change-Id: Ia92382d808983a6ce566c85d06b82afd2375fb90 Reviewed-on: https://go-review.googlesource.com/c/tools/+/494217 TryBot-Result: Gopher Robot <gobot@golang.org> gopls-CI: kokoro <noreply+kokoro@google.com> Run-TryBot: Robert Findley <rfindley@google.com> Reviewed-by: Alan Donovan <adonovan@google.com>
Arguably you have some evidence that folks don't dislike the behaviour of Points very much taken on telemetry BTW. FWIW, because this is configurable I don't really mind which way you have the default. I was just flagging the fact that the default had changed... and that surprised me given the aforementioned evidence. |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
With the following setup:
Called 'Symbol' with a query of
"main"
What did you expect to see?
A single result: the
main
function defined inmain
; because the workspace scope is defined (I think) as the main module.What did you see instead?
Lots of results from outside the workspace (main module) within dependencies:
cc @stamblerre
FYI @leitzler
The text was updated successfully, but these errors were encountered: