[PATCH] D44247: [clangd] Use identifier range as the definition range.

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Mar 8 08:21:41 PST 2018

sammccall added a comment.

In https://reviews.llvm.org/D44247#1031366, @sammccall wrote:

> In https://reviews.llvm.org/D44247#1031345, @malaperle wrote:
> > I was going to change the symbol index to do the opposite :) The range of definitions including the bodies of functions, etc is used in a "peek definition" feature by several LSP clients. So for example in VSCode, you can hold Ctrl and hover on a function call and see its definition in a popup. There was some discussion about this in https://reviews.llvm.org/D35894
> Interesting - that seemed like a more natural interpretation of LSP to me.
>  Others talked me down, motivated (I think) by nicer behavior of jump-to-definition... will bring it up again :)

Man, this seemed compelling to me, but there's a little bit of wiggle room in the spec (what's the "definition location"), so we looked at the MS language servers...
... and both their TS and C++ implementations return the range of the name only, despite that (IMO) being a weird interpretation of the spec.
As for VSCode:

- the "ctrl-to-hover" behavior starts at beginning of the line containing the range, and has a heuristic for when to stop (even if you return the whole definition range, I think). So whole-range is a bit better here (particularly when type/template is on a separate line) but actually still not great.
- "peek definition" shows the selected range in the middle of a block, with the range highlighted. Having the whole code highlighted actually looks kinda bad :/
- "go to definition" puts your cursor at the start of the range, and the identifier seems much better here.

So I *want* to agree, but we'll be fighting the other language servers and editors (and @ioeric, @ilya-biryukov who think we'd be breaking more important workflows)... I think we're actually better off just returning the name.

  rCTE Clang Tools Extra


More information about the cfe-commits mailing list