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

References panel: Fetch search based definitions if no precise definitonis were found #50157

Conversation

philipp-spiess
Copy link
Member

@philipp-spiess philipp-spiess commented Mar 30, 2023

Workaround for https://github.com/sourcegraph/customer/issues/1995

This is a workaround to fix https://github.com/sourcegraph/customer/issues/1995 until we have time for a bigger scale fix up from all of our code intel APIs on the frontend (c.f. #49199). I’m OOO the next week and want to get a fix for the customer in though, hence this PR.

I did some investigations and this seems to match the behavior of the hover tooltip: If no precise definition is found, it seems to use the presence of a search based definition to render the buttons. It also seems to do this regardless of the shouldMixPreciseAndSearchBasedReferences flag.

Test plan

(this is a bit more involved to reproduce so hear me out)

  1. Enable npm package code intel feature flag
  2. Add npm package to code host config
  3. Add a precise index for the Sourcegraph repo
  4. Open a random tsx file (e.g. SourcegraphWebApp.tsx). It should show precise indexes and should show you that there are definitions on the hover tooltip.
  5. Now click on the "Go to definition" button. Since there's no precise match, it will open the references panel.

Before the fix, the references panel would show no resources. After the fix, it will show search based definitions.

Screenshot 2023-03-30 at 13 08 36

App preview:

Check out the client app preview documentation to learn more.

@cla-bot cla-bot bot added the cla-signed label Mar 30, 2023
@github-actions github-actions bot added the team/code-exploration Issues owned by the Code Exploration team label Mar 30, 2023
@philipp-spiess philipp-spiess self-assigned this Mar 30, 2023
@philipp-spiess philipp-spiess marked this pull request as ready for review March 30, 2023 11:36
@philipp-spiess philipp-spiess requested review from olafurpg and a team March 30, 2023 11:37
@sourcegraph-bot
Copy link
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff ca52665...cacce05.

Notify File(s)
@efritz client/web/src/enterprise/codeintel/useCodeIntel.ts
client/web/src/enterprise/codeintel/useSearchBasedCodeIntel.ts

Copy link
Member

@olafurpg olafurpg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@philipp-spiess Thank you for looking into this issue! The diff LGTM 👍🏻

But I didn't manually test the changes locally.

@philipp-spiess philipp-spiess merged commit f17f88b into main Mar 30, 2023
32 checks passed
@philipp-spiess philipp-spiess deleted the ps/ref-panel-load-search-based-definitions-if-no-precise-found branch March 30, 2023 15:22
github-actions bot pushed a commit that referenced this pull request Mar 30, 2023
…tonis were found (#50157)

Workaround for sourcegraph/customer#1995

This is a workaround to fix
sourcegraph/customer#1995 until we have time
for a bigger scale fix up from all of our code intel APIs on the
frontend (c.f. #49199).
I’m OOO the next week and want to get a fix for the customer in though,
hence this PR.

I did some investigations and this seems to match the behavior of the
hover tooltip: If no precise definition is found, it seems to use the
presence of a search based definition to render the buttons. It also
seems to do this regardless of the
`shouldMixPreciseAndSearchBasedReferences` flag.

## Test plan

(this is a bit more involved to reproduce so hear me out)

1. Enable npm package code intel feature flag
2. Add npm package to code host config
3. Add a precise index for the Sourcegraph repo
4. Open a random tsx file (e.g. `SourcegraphWebApp.tsx`). It should show
precise indexes and should show you that there are definitions on the
hover tooltip.
5. Now click on the "Go to definition" button. Since there's no precise
match, it will open the references panel.

Before the fix, the references panel would show no resources. After the
fix, it will show search based definitions.

<img width="1153" alt="Screenshot 2023-03-30 at 13 08 36"
src="https://user-images.githubusercontent.com/458591/228823934-8e72b79c-d22b-45e9-86d3-fbac0d770d9c.png">

<!-- All pull requests REQUIRE a test plan:
https://docs.sourcegraph.com/dev/background-information/testing_principles
-->

## App preview:

-
[Web](https://sg-web-ps-ref-panel-load-search-based.onrender.com/search)

Check out the [client app preview
documentation](https://docs.sourcegraph.com/dev/how-to/client_pr_previews)
to learn more.

(cherry picked from commit f17f88b)
coury-clark pushed a commit that referenced this pull request Mar 31, 2023
… precise definitonis were found (#50179)

Workaround for sourcegraph/customer#1995

This is a workaround to fix
sourcegraph/customer#1995 until we have time
for a bigger scale fix up from all of our code intel APIs on the
frontend (c.f. #49199).
I’m OOO the next week and want to get a fix for the customer in though,
hence this PR.

I did some investigations and this seems to match the behavior of the
hover tooltip: If no precise definition is found, it seems to use the
presence of a search based definition to render the buttons. It also
seems to do this regardless of the
`shouldMixPreciseAndSearchBasedReferences` flag.

## Test plan

(this is a bit more involved to reproduce so hear me out)

1. Enable npm package code intel feature flag
2. Add npm package to code host config
3. Add a precise index for the Sourcegraph repo 
4. Open a random tsx file (e.g. `SourcegraphWebApp.tsx`). It should show
precise indexes and should show you that there are definitions on the
hover tooltip.
5. Now click on the &quot;Go to definition&quot; button. Since
there&#39;s no precise match, it will open the references panel.

Before the fix, the references panel would show no resources. After the
fix, it will show search based definitions.

&lt;img width=&quot;1153&quot; alt=&quot;Screenshot 2023-03-30 at 13 08
36&quot;
src=&quot;https://user-images.githubusercontent.com/458591/228823934-8e72b79c-d22b-45e9-86d3-fbac0d770d9c.png&quot;&gt;


&lt;!-- All pull requests REQUIRE a test plan:
https://docs.sourcegraph.com/dev/background-information/testing_principles
--&gt;

## App preview:

-
[Web](https://sg-web-ps-ref-panel-load-search-based.onrender.com/search)

Check out the [client app preview
documentation](https://docs.sourcegraph.com/dev/how-to/client_pr_previews)
to learn more.

 <br> Backport f17f88b from #50157

---------

Co-authored-by: Philipp Spiess <hello@philippspiess.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed team/code-exploration Issues owned by the Code Exploration team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants