redhat-developer / vscode-java

Java Language Support for Visual Studio Code
Eclipse Public License 2.0
2.08k stars 441 forks source link

Peek References show to the current selected one #3850

Closed mamilic closed 1 week ago

mamilic commented 2 weeks ago

When the action on a method is triggered from context menu, right click -> Peek -> Peek References it will show the current location of method from where the action is triggered. As the developers are mainly interested for references of that method that occur in the project, showing the current one where developer is already located at, and where action originates from is somewhat not best experience. Also I belive this behaviour is present in all navigation type actions.

Could the method/propery/class where the action originates from be excluded from results?

Image

rgrunber commented 1 week ago

Here's what the sample request looks like to the language server :

[Trace - 10:56:17] Sending request 'textDocument/references - (43)'.
Params: {
    "textDocument": {
        "uri": "file:///home/rgrunber/git/lemminx/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/utils/DOMUtils.java"
    },
    "position": {
        "line": 66,
        "character": 26
    },
    "context": {
        "includeDeclaration": true
    }
}

We actually respect this at https://github.com/eclipse-jdtls/eclipse.jdt.ls/blob/f155e1fc90618b558899c3722606feccfc89e210/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/ReferencesHandler.java#L87 .

We don't control includeDeclaration with any setting though. It comes from somewhere in VS Code or maybe the vscode-languageserver-node client. See https://github.com/microsoft/vscode/issues/68071 , https://github.com/microsoft/vscode/issues/74237 . The latter makes me think the goal was to provide more information when the references are called.

I'm not sure https://github.com/microsoft/vscode/blob/01340a9057c63ebaf0693884c11f5bb330cbfb08/src/vs/editor/contrib/gotoSymbol/browser/goToSymbol.ts#L79-L87 is the exact code but it could be something similar to that. We could change this by overriding the request to respect some setting we create on our end, like java.references.includeDeclarations. I guess the issue with disabling by default is that a declaration's identifier is still some kind of reference, that should probably be included.

rgrunber commented 1 week ago

Proposal is a setting java.references.includeDeclarations (defaults to true) to control this.