Skip to content

Introduce separate tree item types in the methods usage panel#2976

Merged
koesie10 merged 1 commit intomainfrom
koesie10/methods-usage-panel-parent
Oct 16, 2023
Merged

Introduce separate tree item types in the methods usage panel#2976
koesie10 merged 1 commit intomainfrom
koesie10/methods-usage-panel-parent

Conversation

@koesie10
Copy link
Copy Markdown
Member

This creates new tree item types for methods and usages such that these can contain references to their parent and children. This allows us to easily find the parent of a usage and to find the children of a method. This removes an expensive find call in getParent.

Checklist

  • CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • Issues have been created for any UI or other user-facing changes made by this pull request.
  • [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

This creates new tree item types for methods and usages such that these
can contain references to their parent and children. This allows us to
easily find the parent of a usage and to find the children of a method.
This removes an expensive `find` call in `getParent`.
@koesie10 koesie10 marked this pull request as ready for review October 16, 2023 09:40
@koesie10 koesie10 requested a review from a team as a code owner October 16, 2023 09:40
Copy link
Copy Markdown
Contributor

@robertbrignull robertbrignull left a comment

Choose a reason for hiding this comment

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

Works for me when tested, and the code looks good. I think this is a good approach and very similar to what I tried to implement previously.

I also think renaming resolveCanonicalUsage to resolveUsageTreeViewItem is a good change. Unfortunately we can't remove this method unless the internal tree view data/type is the exact same as the data outside of the tree view, and that won't be the case now. We still do an expensive lookup in this method but I think this is an ok tradeoff and we call this a lot less frequently than we call getParent. It's also something we can look at improving again in the future if it's an issue.

@koesie10 koesie10 merged commit 6a7ce9f into main Oct 16, 2023
@koesie10 koesie10 deleted the koesie10/methods-usage-panel-parent branch October 16, 2023 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants