kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land.
thanks, lgtm! ================ Comment at: clang-tools-extra/clangd/ClangdServer.h:153 + /// to cancel. Clients that always cancel stale requests should clear this. + bool ImplicitCancellation = true; + ---------------- sammccall wrote: > 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. > we want to disable this (entirely) in C++ embedders, and a boolean option is > the simplest way to do that. i think it should be okay for embedders to specify what behavior they want per action, but i agree with the other two points. thanks for the explanation! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98414/new/ https://reviews.llvm.org/D98414 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
