clangd / vscode-clangd

Visual Studio Code extension for clangd
https://marketplace.visualstudio.com/items?itemName=llvm-vs-code-extensions.vscode-clangd
MIT License
629 stars 106 forks source link

GoTo Definition for marked template type not working as expected #307

Open juxeii opened 2 years ago

juxeii commented 2 years ago

In the term std::optional<types::nr_rrc::BandList>, I mark BandList and hit Go to Definition. I would expect that this jumps to types::nr_rrc::BandList, but it jumps to std::optional.

Do I miss something here?

HighCommander4 commented 2 years ago

The Language Server Protocol currently only accepts a single position as input for the go-to-definition request, not a range.

Assuming you've selected the range BandList in the forward direction, such that your cursor is between the t and >, it's this cursor position that's sent.

Clangd then needs to decide whether to use the > or the t for targeting. It tries the character to the right of the cursor first, i.e. the >, which is associated with the class template, so it navigates to the class template.

I agree this behaviour is suboptimal. Hopefully, LSP can add support for sending the entire range in https://github.com/microsoft/language-server-protocol/issues/1029.

sam-mccall commented 2 years ago

I don't think MS is likely to fix this. I think we should probably add an extension client capability to control the behavior and set it in the VSCode plugin. For ibeam-editors bias-toward-identifier is better than bias-right.

Last time i thought about this i got stuck on the idea that this might not be fixed for an editor (e.g. vim mode for VSCode) or even vary over time (modal editing). But this is probably perfect-as-enemy-of-the-good.

HighCommander4 commented 2 years ago

I don't think MS is likely to fix this.

I'm all for making the clangd-only fix you describe, but I'm also curious why you don't think this will be fixed in LSP. Range inputs seem useful independent of this specific issue; are you expecting resistance to them for a particular reason, rather than just being a matter of someone doing the work of a fleshed-out proposal with spec and vscode client changes?

juxeii commented 2 years ago

I am wondering how CLion does this, since AFAIK they also use clang. There, the Go to Definition works as intuitevly expected.

wangyu- commented 2 years ago

I have encountered similiar problems.

Thanks for @HighCommander4 's explanation, I have found the reason and known how to a workaround it.

Assuming you've selected the range BandList in the forward direction, such that your cursor is between the t and >, it's this cursor position that's sent.

Clangd then needs to decide whether to use the > or the t for targeting. It tries the character to the right of the cursor first, i.e. the >, which is associated with the class template, so it navigates to the class template.

For me, the "go to definition" doesn't work correctly since I have a habit of double clicking on the BandList, then the cursor is automatically placed at the end of BandList. For my case, the workaround is just to put the cursor in the middle of BandList and don't double click.