[PATCH] D98505: [clangd] Propagate data in diagnostics

Kadir Cetinkaya via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 24 08:55:44 PDT 2021


kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/Diagnostics.h:58
+  /// If true, Clangd will populate the data field in LSP diagnostic
+  /// representation. This is used to prevent extra data transfer with old
+  /// clients that doesn't support data field.
----------------
sammccall wrote:
> Second sentence is confusing because of inverted sense. And really I don't think the reason is that we don't want to send extra data, but rather the fear that old clients will choke on it.
> 
> If we're *not* afraid of that, i.e. we think they'll just drop it on the floor, then I don't think we should bother to feature-detect it just so *we* can drop it on the floor.
> Not sure how you feel about this, but it's pretty tempting to me...
as discussed offline dropping this completely. since we don't really guard against adding "extra" properties to objects, and the worst that could happen is clangd won't surface a particular tweak on clients without support.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98505



More information about the cfe-commits mailing list