sammccall added a comment.
In D97738#2597296 <https://reviews.llvm.org/D97738#2597296>, @kadircet wrote:
> as discussed offline, i am hesitant about `validateEdits` layering at the
> moment. but i suppose the best thing to do is move it into clangdserver and
> expose a structrual api, as you proposed, which can be done independently.
Yup. It's not great, but it doesn't seem terrible or dangerous or anything.
(We're not opening any new interfaces to support this, getDraft() is called in
lots of places)
In the same bucket I think is sorting out TextEdits vs Replacements in a few
interfaces etc.
================
Comment at: clang-tools-extra/clangd/ClangdServer.h:266
+ /// If \p Rng is non-null, formats only that region.
+ void formatFile(PathRef File, const Range *Rng,
Callback<tooling::Replacements> CB);
----------------
kadircet wrote:
> nit: Optional<Range> instead of a pointer?
Haha, I had this, then thought "but we pass things in by reference anyway".
Except... we don't, in ClangdServer. And I'm not sure why.
Anyway, done.
================
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);
----------------
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!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97738/new/
https://reviews.llvm.org/D97738
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
