[#1094] Add support for Call Hierarchy #1096
Conversation
| {atom(), non_neg_integer()}) -> | ||
| binary(). | ||
| function_signature({M, F, A}) -> | ||
| els_utils:to_binary(io_lib:format("~p:~ts/~p", [M, F, A])); |
There was a problem hiding this comment.
| els_utils:to_binary(io_lib:format("~p:~ts/~p", [M, F, A])); | |
| els_utils:to_binary(io_lib:format("~p:~ts/~b", [M, F, A])); |
| #{ <<"item">> := Item } = Params, | ||
| #{ <<"uri">> := Uri } = Item, | ||
| POI = els_call_hierarchy_item:poi(Item), | ||
| Applications = applications_in_function_range(Uri, POI), |
There was a problem hiding this comment.
this took me a while to understand, applications are function applications, but in Erlang applications usually mean Erlang Applications.
Also I understand applications more as applications of the marked function. Maybe rename to calls_in_function_range or outgoing_calls
There was a problem hiding this comment.
Yeah, it can be confusing. This refers to application in erl_syntax terms. We should probably bring clarity around this, either by renaming or via a comment.
|
|
||
| -spec wrapping_functions(item(), non_neg_integer(), non_neg_integer()) -> | ||
| [poi()]. | ||
| wrapping_functions(Document, Line, Column) -> |
There was a problem hiding this comment.
Is this expected to return more than one function?
How is the behavior with funs?
There was a problem hiding this comment.
Good eye :)
It will be always be a [] or a list with exactly one element. Returning a list allows to avoid caseing around in the calling code (if the list is empty, [] will be sent back to the client).
There was a problem hiding this comment.
This is not taking funs into account at this stage.
| reference_to_item(Reference) -> | ||
| #{uri := RefUri, range := RefRange} = Reference, | ||
| {ok, RefDoc} = els_utils:lookup_document(RefUri), | ||
| [WrappingPOI] = els_dt_document:wrapping_functions(RefDoc, RefRange), |
There was a problem hiding this comment.
can this return more than one POI?
There was a problem hiding this comment.
Not in this case, this should always be one (see my comment above about why returning a list).
There was a problem hiding this comment.
In your referenced comment you state zero or one. When zero, we get a crash.
|
I saw your comments only after the merge @TheGeorge , sorry about that 🙏 |
els_dt_document:wrapping_functions/2 can return zero or one items in a list. Explicitly deal with both options. See #1096 (comment)
els_dt_document:wrapping_functions/2 can return zero or one items in a list. Explicitly deal with both options. See #1096 (comment)
els_dt_document:wrapping_functions/2 can return zero or one items in a list. Explicitly deal with both options. See #1096 (comment)
Description
This PR introduces support for Call Hierarchy.
For a nicer overview of what the Call Hierarchy feature provides please watch this video.
There are still a couple of improvements possible (for example when it comes to recursive functions), which will be addressed as follow-ups.
The feature steps up the game when it comes to finding references (aka who calls this function). Erlang LS is already able to quickly identify callers, but that approach forces the developer to keep in mind the tree of already explored paths. Now that mental tree is available in the UI.
Emacs Support
To use it In Emacs you will require the
lsp-treemacspackage.To see the incoming calls hierarchy:
To see the outgoing calls hierarchy:
VS Code Support
Before this is available in VS Code we need to bump the
languageclientpackage to7.0.0, which is a breaking change.Fixes #1094.