-
Notifications
You must be signed in to change notification settings - Fork 419
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 Go to Super Implementation to context menu and command palette #1395
Conversation
The fact the menu item shows up everywhere is very distracting, given than there's no feedback when you call it |
@akaroml Let's sync with VS Code team about this issue. We need the API to show the editor message. Otherwise the |
Update: VS Code team has exposed a new API to show the editor message. I will update this PR after the next VS Code stable version released. |
@fbricon @testforstephen PR updated |
src/standardLanguageClient.ts
Outdated
return commands.executeCommand( | ||
'editor.action.goToLocations', | ||
window.activeTextEditor.document.uri, | ||
window.activeTextEditor.selection.active, | ||
[], | ||
'goto', | ||
'No super implementation found' | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CommandsRegistry.registerCommand({
id: 'editor.action.goToLocations',
description: {
description: 'Go to locations from a position in a file',
args: [
{ name: 'uri', description: 'The text document in which to start', constraint: URI },
{ name: 'position', description: 'The position at which to start', constraint: corePosition.Position.isIPosition },
{ name: 'locations', description: 'An array of locations.', constraint: Array },
{ name: 'multiple', description: 'Define what to do when having multiple results, either `peek`, `gotoAndPeek`, or `goto' },
{ name: 'noResultsMessage', description: 'Human readable message that shows when locations is empty.' },
]
},
Can you also use this function to jump to target location when there is result?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried but seems not working. I'm asking in VS Code repo...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finally it turns out that all the types should be generated by their constructors in API. Simply making the structure won't work.
@snjeza can you check this PR on Theia/Che? |
The Go to Super Implementation command doesn't work on Theia.
|
Is it caused by the new command introduced in VS Code 1.47.0: |
}, | ||
{ | ||
"command": "java.action.navigateToSuperImplementation", | ||
"when": "javaLSReady && editorTextFocus && editorLangId == java", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we add a supportsGoToSuperImplementation
flag, computed dynamically, similar to javaLSReady?
@@ -735,6 +745,10 @@ | |||
"command": "java.project.import", | |||
"when": "javaLSReady" | |||
}, | |||
{ | |||
"command": "java.action.navigateToSuperImplementation", | |||
"when": "javaLSReady && editorLangId == java" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also check if supportsGoToSuperImplementation
preserveFocus: true, | ||
selection: range, | ||
}); | ||
context.subscriptions.push(commands.registerCommand(Commands.NAVIGATE_TO_SUPER_IMPLEMENTATION_COMMAND, async (location: LinkLocation | Uri) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only register if supportsGoToSuperImplementation === true
It works in Theia without this PR. |
you mean a |
I mean it is required. |
@jdneo could you resolve conflicts? |
Signed-off-by: Sheng Chen <sheche@microsoft.com>
PR updated. Now it's using In Che/Theia, since that command so far does not support to show such message, so nothing will happen when no result found. An issue has been created in the theia repo: eclipse-theia/theia#8212 |
I also observed that in my machine, sometimes it will take ~10+s to get the result of super implementation. @fbricon @snjeza @testforstephen @Eskibear Have you met that case? |
works fine for me |
@jdneo thanks! |
Note: so far, if the selected element has no super implementation, we are not able to show the editor message. This need VS Code to support such API. See: microsoft/vscode#95308
Signed-off-by: Sheng Chen sheche@microsoft.com