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

Ensure the initial symbol a user searches for is top of hte find-refs list #43472

Merged
7 commits merged into from
Apr 22, 2020

Conversation

CyrusNajmabadi
Copy link
Member

Also, collapse all definitions we cascade to by default. Looks like this:

image

@@ -28,9 +28,6 @@ internal abstract partial class AbstractFindUsagesService
if (moniker == null)
return;

// Let the find-refs window know we have outstanding work
await using var _ = await context.ProgressTracker.AddSingleItemAsync().ConfigureAwait(false);
Copy link
Member Author

Choose a reason for hiding this comment

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

intentional removal. progress tracking doesn't work well for richnav as we don't really know how many items it will need to search. so trying to evne add progress items for it just ends up interfering with the good progress bar we have for the normal find-refs for user code in their solution.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, so we did all that refactoring work to then not use it? 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

it's used elsewhere :) just here it's not particularly good and def degrades the experience. i.e. if you do have code-index, we just jump to the 50% point immediately because it says "i have one piece of work", and then finishes it. The other part of find refs has has added one piece of owrk that is not done, causing us to be at 1-of-2 done.

This is unlike normal find refs, which can add a bunch of known work it needs to do (like looking at all files with bloom filter matches) which gives far more fine-grained progress.

@CyrusNajmabadi CyrusNajmabadi removed the request for review from devopsjesus April 19, 2020 01:51
Comment on lines +51 to +52
// Only grab the first 500 results. This keeps server load lower and is acceptable for //build demo purposes.
const int PageCount = 5;
Copy link
Member

Choose a reason for hiding this comment

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

Did this mean to come in this PR or something else?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes. i pulled this in from my main richnav PR (which i'm still trying to get in).

@dotnet dotnet deleted a comment from jasonmalinowski Apr 21, 2020
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Auto-approval

@ghost ghost merged commit 27aab41 into dotnet:master Apr 22, 2020
@jinujoseph jinujoseph added this to the 16.7.P1 milestone Apr 23, 2020
@CyrusNajmabadi CyrusNajmabadi deleted the findRefsCollapsing branch April 23, 2020 18:51
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants