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

Add SortDocumentSymbols to make the outline hierarchical (again) #2084

Merged
merged 3 commits into from
Oct 19, 2023

Conversation

andyleejordan
Copy link
Member

This is probably what OmniSharp was doing for us. Fixes the bug introduced by #2083.

Hey, at least I didn't use LINQ. Thanks Bing Chat for the algorithm help. This should probably wait for the pre-release after the next stable release.

This is probably what OmniSharp was doing for us.
@andyleejordan andyleejordan force-pushed the andyleejordan/hierarchical-symbols branch from 7519e9d to 27b5325 Compare October 12, 2023 16:04
Copy link
Member Author

@andyleejordan andyleejordan left a comment

Choose a reason for hiding this comment

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

Feedback from code review meeting.

Copy link
Collaborator

@SeeminglyScience SeeminglyScience left a comment

Choose a reason for hiding this comment

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

LGTM! Love it ❤️

This finishes the optimizations by relying on the fact that we're
passing a reference to the list which means we can modify it in-place.
We sort the whole list of symbols on the first pass only, and then as
sort children into parents' buckets we *move* them by adding them to
`parent.Children` and removing them from the current list, and then
recurse and modify those lists until we're done.

I don't normally like relying on side-effects such as modifying in the
called function, but it makes the most sense in this case for
optimizations and is duly noted.
Copy link
Collaborator

@SeeminglyScience SeeminglyScience left a comment

Choose a reason for hiding this comment

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

Even better ❤️

@andyleejordan andyleejordan added this pull request to the merge queue Oct 19, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Oct 19, 2023
@andyleejordan andyleejordan added this pull request to the merge queue Oct 19, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 19, 2023
@andyleejordan andyleejordan added this pull request to the merge queue Oct 19, 2023
Merged via the queue into main with commit 993a54c Oct 19, 2023
13 checks passed
@andyleejordan andyleejordan deleted the andyleejordan/hierarchical-symbols branch October 19, 2023 23:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants