code intel: Remove search context from search based code intel #58010
Conversation
|
Tagging @keynmol @varungandhi-src for review instead of myself |
Using search context is problematic when viewing a file/repo that is not
part of the selected search context, which can happen when a custom
default context is set.
It doesn't seem necessary to include the context. The query I looked at
now doesn't have the context set anymore. Example:
^append$ type:symbol patternType:regexp count:50 case:yes file:\.(go)$ repo:^github.com/sourcegraph/sourcegraph$ index:only
I wasn't able to verify all code paths though.
fkling
force-pushed
the
fkling/search-based-code-intel-default-context
branch
from
58754fd
to
c6deaa3
Compare
So should we avoid setting the search context only in the specific situation when a custom default context is set? Consider the situation where someone navigated to a file with an explicit search context where the search is limited only to certain subdirectories (say because they work in a large monorepo). Then they perform a hover on a symbol. Is your point that for code intel, we should ignore their context and show results in other directories too for the sake of completeness? |
Yes, although it's less about completeness for me and more about predictability/reproducibility. E.g. in your scenario if I refreshed the page my default context would be set and I would get different search based code intel information. I think it's not obvious to users that their currently selected search context affects search based code intel results and that it's not too difficult to get into a situation where this can even lead to "not results". |
Using search context is problematic when viewing a file/repo that is not
part of the selected search context, which can happen when a custom
default context is set.
It doesn't seem necessary to include the context. The query I looked at
now doesn't have the context set anymore. Example:
^append$ type:symbol patternType:regexp count:50 case:yes file:\.(go)$ repo:^github.com/sourcegraph/sourcegraph$ index:only
I wasn't able to verify all code paths though.
Fixes #58006
Using search context is problematic when viewing a file/repo that is not part of the selected search context, which can happen when a custom default context is set.
It doesn't seem necessary to include the search context. The query I looked at now doesn't have the context set anymore. Example:
I wasn't able to verify all code paths though.
Test plan
Hover over
appendin https://sourcegraph.test:3443/github.com/sourcegraph/sourcegraph/-/blob/cmd/frontend/internal/own/resolvers/resolvers.go?L145:9 and see that the request doesn't contain a search context.