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

Peek call hierarchy should resend prepareCallHierarchy request after switch the direction #87176

Open
testforstephen opened this issue Dec 17, 2019 · 3 comments
Assignees
Labels
callhierarchy under-discussion Issue is under discussion for relevance, priority, approach

Comments

@testforstephen
Copy link

In the normal "Show Call Hierarchy" view, both switching the direction and refresh will resend a prepareCallHierarchy request.

But in the "Peek Call Hierarchy" view, switching the direction doesn't resend a prepareCallHierarchy request. It's better to keep consistent with the normal call hierarchy view. Generally we will do some clean up in the prepareCallHierarchy.

@weinand weinand assigned jrieken and unassigned weinand Dec 17, 2019
@jrieken jrieken added callhierarchy under-discussion Issue is under discussion for relevance, priority, approach labels Dec 17, 2019
@jrieken
Copy link
Member

jrieken commented Dec 17, 2019

Generally we will do some clean up in the prepareCallHierarchy.

Can you explain that a little more? Is that clean up from a previous session so that the very last is never cleaned? The fact that "Show Call Hierarchy" always starts with a prepare-call is bit of an implementation detail...

@testforstephen
Copy link
Author

We have a requirement about stopping expanding the recursive calls. For example, below is code snippet for recursive call.

public void foo() {
   bar();
}

public void bar() {
   foo();
}

When hover at public void <|>foo() { to show call hierarchy, we want the call hierarchy tree to be like something below.

foo()
 |
 --- bar()
      | 
      --- foo() // Stop at here when continue expanding foo()

In order to detect the recursive ring, we have to add some cache to record the visited elements at past. And every time when expanding the call hierarchy, it will check if the node is already in the visited graph. And if it's yes, then stop expanding the children. That's why we need some signal to clean up the cache.

Recursive call looks like a general problem for all languages. If the language client can deal it directly, that seems be more reasonable. Then the server is just be stateless.

@jrieken
Copy link
Member

jrieken commented Dec 18, 2019

Recursive call looks like a general problem for all languages. If the language client can deal it directly, that seems be more reasonable. Then the server is just be stateless.

Yeah, our API idea is that servers don't bother about recursion - that's why the API only asks for the next level. We know that this doesn't prevent recursion but since the resolving of the next level is driven by users, e.g by expanding tree nodes, we think it's not a real problem.

Did you hit anything when implementing/experimenting with this or why did you add the recursion check?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
callhierarchy under-discussion Issue is under discussion for relevance, priority, approach
Projects
None yet
Development

No branches or pull requests

3 participants