[PATCH] D97738: [clangd] Move DraftStore from ClangdLSPServer into ClangdServer.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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/DraftStore.cpp:66
     // We treat versions as opaque, but the protocol says they increase.
-    if (*Version <= D.Version)
-      log("File version went from {0} to {1}", D.Version, Version);
-    D.Version = *Version;
+    if (SpecifiedVersion.compare_numeric(D.Version) <= 0)
+      log("File version went from {0} to {1}", D.Version, SpecifiedVersion);
----------------
sammccall wrote:
> kadircet wrote:
> > why not a not_equals instead? we are going to override the version anyway
> I don't really understand the suggestion.
> 
> The purpose here is to notice and log when client-specified versions go backwards.
> This is a protocol violation, which won't cause problems for clangd but may indicate something broken going on.
> 
> We don't want to log when versions go forwards!
ah nvm, i was reading this wrong. i thought it was logging when versions go "forward"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97738