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. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44247 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
