[PATCH] D98414: [clangd] Turn off implicit cancellation based on client capabilities

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clang-tools-extra/clangd/ClangdServer.h:153
+    /// to cancel. Clients that always cancel stale requests should clear this.
+    bool ImplicitCancellation = true;
+
----------------
kadircet wrote:
> this makes sense as is, but i wonder if we should lift this to LSPServer instead.
> 
> 3.17 specs also introduce `retryOnContentModified`, which is supposed to be a list of RPC names. so if we decide to take that into account, rather than deciding on what's "transient" ourselves, having all of this hardcoded in clangdserver would be limiting. 
> 
> We can accept a cancellation policy on all of the ClangdServer endpoints, have some sensible defaults in clangdlspserver, enable those to be overwritten by client specs on initialize. (or if we don't want to make them part of the signature i suppose we can make use of config/context, but ... yikes). WDYT?
I thought about this, but I don't think it's better...

 - it's complicated and boilerplatey with the method names, way out of proportion to the value here
 - I think "will the client take care of cancellation" is actually the more pertinent question rather than "will the client retry if we cancel"
 - we want to disable this (entirely) in C++ embedders, and a boolean option is the simplest way to do that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98414/new/

https://reviews.llvm.org/D98414