kbobyrev added inline comments.
================ Comment at: clang-tools-extra/clangd/FindSymbols.cpp:281 + Range.startCharacter = Symbol.range.start.character; + Range.endLine = Symbol.range.end.line; + Range.endCharacter = Symbol.range.end.character; ---------------- sammccall wrote: > kbobyrev wrote: > > sammccall wrote: > > > How useful are folding ranges when symbols start/end on the same line? Do > > > we really want to emit them? > > I think they are useful from the LSP perspective, if characters didn't > > matter in the ranges they wouldn't be included in the protocol at all and > > there wouldn't be any way for clients to specify their preferences. > > > > I don't think it's a common use case, but I do think it's a totally valid > > one. Maybe those should be the first candidates for excluding when there is > > a limit and maybe we could completely cut them for performance. But from > > the LSP perspective it seems logical to have those. > > > > What do you think? > Well, I don't think "the protocol allows regions within a line, therefore we > must emit them" is a great argument. You mention a valid use case, but you > haven't *described* any use cases - what specific C++ intra-line region is > the user likely to fold in a way that's useful? > > I think that it's hard to point to concrete benefits, but the costs are clear > enough: > - responses will be (much) bigger, which we know some clients don't deal > well with > - clients are not likely to do any smart filtering, so I think this will > lead to performance/ux problems in practice (especially if most > implementations only support folding blocks etc) > - it makes decisions on what folding regions to emit harder, e.g. should we > fold function calls, not fold them, or fold them only if start/end are on > different lines? > - we need two different modes, to handle clients that support line-folds vs > clients that support char-folds. (If we only emit multi-line folds we could > also only emit the inner (or the outer) fold for a pair of lines, and the > result would be suitable for both types of editors) > > > when there is a limit > It's pretty important that this feature behaves consistently, for it to be > useful. If we're going to only-sometimes enable folding of certain > constructs, it better be for a reason that's pretty obvious to the user (I > believe single vs multiline qualifies, but "the document complexity exceeds > X" certainly doesn't.) For this reason I don't think we should implement that > capabilities feature (or should do it in some totally naive and unobtrusive > way, like truncation) As discussed online, we should emit single-line ranges. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82436/new/ https://reviews.llvm.org/D82436 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
