Skip to content

Implement missing functionality for call hierarchy API#9681

Merged
colin-grant-work merged 1 commit intoeclipse-theia:masterfrom
colin-grant-work:feature/vscode-call-hierarchy-support
Jul 16, 2021
Merged

Implement missing functionality for call hierarchy API#9681
colin-grant-work merged 1 commit intoeclipse-theia:masterfrom
colin-grant-work:feature/vscode-call-hierarchy-support

Conversation

@colin-grant-work
Copy link
Contributor

@colin-grant-work colin-grant-work commented Jun 30, 2021

What it does

Fixes #9612 by implementing the commands used by the vscode-references-view plugin to retrieve call hierarchy information.
It also:

How to test

  1. You should be able to use the Calls: Show Call Hierarchy Command
  2. You should be able to get both incoming (default) and outgoing calls.
  3. When you click on a node, it should reveal the location of that function. Confirm that your cursor is at the beginning of the function.

Review checklist

Reminder for reviewers

Signed-off-by: Colin Grant colin.grant@ericsson.com

@colin-grant-work colin-grant-work force-pushed the feature/vscode-call-hierarchy-support branch from 0ddc096 to 30957f7 Compare June 30, 2021 18:23
mappings['vscode.prepareCallHierarchy'] = ['vscode.prepareCallHierarchy', CONVERT_VSCODE_TO_MONACO, CONVERT_MONACO_TO_VSCODE];
mappings['vscode.provideIncomingCalls'] = ['vscode.provideIncomingCalls', CONVERT_VSCODE_TO_MONACO, CONVERT_MONACO_TO_VSCODE];
mappings['vscode.provideOutgoingCalls'] = ['vscode.provideOutgoingCalls', CONVERT_VSCODE_TO_MONACO, CONVERT_MONACO_TO_VSCODE];
mappings['vscode.open'] = ['vscode.open', CONVERT_VSCODE_TO_MONACO];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Strictly speaking, this isn't necessary to get the version of the plugin downloaded by our scripts to work because it uses vscode.window.showTextDocument rather than this command. However, later versions of the plugin use the vscode.open command, and we have an off-by-one issue when handling TextDocumentShowOptions passed by plugins into that command.

@colin-grant-work colin-grant-work force-pushed the feature/vscode-call-hierarchy-support branch 4 times, most recently from 90d9069 to c7d27e4 Compare July 2, 2021 14:04
@RomanNikitenko
Copy link
Contributor

RomanNikitenko commented Jul 2, 2021

@colin-grant-work
I see that you picked up the latest changes from the master - just fyi - some tests will fail due to #9692, so such failure is not related to your changes

@colin-grant-work
Copy link
Contributor Author

... some tests will fail due to #9692, so such failure is not related to your changes

Thanks for the heads up. Always nice to have someone else to blame 🙂

@RomanNikitenko
Copy link
Contributor

@colin-grant-work
I'm going to test/review the changes tomorrow.
I'm sorry for the delay...

@vince-fugnitto vince-fugnitto added call-hierarchy issues related to the call-hierarachy vscode issues related to VSCode compatibility labels Jul 6, 2021
@RomanNikitenko
Copy link
Contributor

I tested a little the changes and can confirm that in general it works well for me!

I noticed that the source is not displayed for items:

VS Code:

vscode_source

Theia:

theia_source

Another difference which I found:
VS Code moves cursor to the corresponding method when I use double click, but Theia does nothing for me.

VS Code
double_click_vscode

Theia
double_click_theia

@colin-grant-work
Does it work for you?

I'm not requesting a fix - I think we can improve it in a separate PR - just sharing what I detected within review the PR...

@RomanNikitenko
Copy link
Contributor

RomanNikitenko commented Jul 7, 2021

@colin-grant-work

Confirm that your cursor is at the beginning of the function.

What do you mean?
I don't see that my cursor moves to a function. Should it work at double click?
maybe I'm doing something wrong...

@colin-grant-work
Copy link
Contributor Author

I noticed that the source is not displayed for items

Good find. I'm not sure what's going on here - I'll check how that data is specified in the tree items created by the plugin and how we're handling them.

Another difference which I found:
VS Code moves cursor to the corresponding method when I use double click, but Theia does nothing for me.

@colin-grant-work
Does it work for you?

It does work... sort of. I think the difference here is that VSCode transfers focus to the editor on double click, so you can see your cursor, but Theia transfers focus back to the reference view, so the editor cursor disappears. If you click the editor tab to focus the editor, the cursor should be in the same place in Theia as it is in VSCode. I recall seeing code with something like preserveFocus: true, so I'll check whether that's on our side or the plugin's, and whether it's an appropriate choice for all situations.

@colin-grant-work
Copy link
Contributor Author

colin-grant-work commented Jul 13, 2021

@RomanNikitenko, I think that what you're seeing in VSCode may be a fairly recent addition or depend on other functionality. Running VSCode 1.50 in the same environment as I run Theia, here's what I see:

image

And when double clicking to open the reference, focus stays in the references widget (orange/brown is the focus color for the tree, and no cursor is present in the editor):

image

If you activate the editor, the cursor should be at the beginning of the function that calls the function you're interested in. The plugin is passing preserveFocus true to the document opener, so I believe that that behavior is correct.

Running the most recent VSCode, I get the behavior you describe, but the version of the plugin that we're using is quite old (0.0.47 vs current 0.0.80), so I think we shouldn't expect to see the exact same behavior in the two contexts.

@colin-grant-work colin-grant-work force-pushed the feature/vscode-call-hierarchy-support branch from c7d27e4 to f3e78a2 Compare July 13, 2021 20:03
@RomanNikitenko
Copy link
Contributor

@colin-grant-work

@RomanNikitenko, I think that what you're seeing in VSCode may be a fairly recent addition or depend on other functionality.

thank you for the investigation and detailed explanation!

@RomanNikitenko
Copy link
Contributor

@colin-grant-work
btw: your PR fixes #8333
at least vscode.open command

@colin-grant-work
Copy link
Contributor Author

@colin-grant-work
btw: your PR fixes #8333
at least vscode.open command

Thanks for pointing that out. I'll check the diff command, and if it's as straightforward to fix as vscode.open, I'll add it to this PR.

@colin-grant-work colin-grant-work force-pushed the feature/vscode-call-hierarchy-support branch from f3e78a2 to e1b4995 Compare July 15, 2021 16:55
@colin-grant-work
Copy link
Contributor Author

Thanks for pointing that out. I'll check the diff command, and if it's as straightforward to fix as vscode.open, I'll add it to this PR.

Adding vscode.diff to the list of known commands resulted in the same behavior as #8334. I've created an issue (#9746) for the odd behavior of the diff widget on opening, which I believe to be a separate problem.

@colin-grant-work
Copy link
Contributor Author

I'll merge this tomorrow if there are no objections, to leave some time before the release.

Signed-off-by: Colin Grant <colin.grant@ericsson.com>
@colin-grant-work colin-grant-work force-pushed the feature/vscode-call-hierarchy-support branch from 8087f06 to f01a647 Compare July 16, 2021 20:53
@colin-grant-work colin-grant-work merged commit ec9084c into eclipse-theia:master Jul 16, 2021
@colin-grant-work colin-grant-work deleted the feature/vscode-call-hierarchy-support branch July 16, 2021 21:07
@vince-fugnitto vince-fugnitto added this to the 1.16.0 milestone Jul 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

call-hierarchy issues related to the call-hierarachy vscode issues related to VSCode compatibility

Projects

None yet

3 participants