[PATCH] D66637: [clangd] Support multifile edits as output of Tweaks
Sam McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 23 05:48:22 PDT 2019
sammccall added inline comments.
================
Comment at: clang-tools-extra/clangd/SourceCode.h:41
+/// Represents a set of edits generated for a single file.
+struct Edit {
+ /// SHA1 hash of the file contents for the edits generated below. Clients
----------------
Hmm, it is also attractive to make this something like
```
struct Edit {
string CodeBefore;
tooling::Replacements Reps;
string codeAfter();
std::vector<TextEdit> asTextEdits();
bool canApplyTo(StringRef CodeBefore); // does line-ending normalization
}
```
if we were worried about abuse, we could make CodeBefore private
================
Comment at: clang-tools-extra/clangd/SourceCode.h:43
+ /// SHA1 hash of the file contents for the edits generated below. Clients
+ /// should verify contents they see are the same as this one before applying
+ /// edits.
----------------
, if possible.
(Sometimes we're just sending these over the wire)
================
Comment at: clang-tools-extra/clangd/SourceCode.h:46
+ std::array<uint8_t, 20> ContentDigests;
+ tooling::Replacements Reps;
+ llvm::Optional<std::vector<TextEdit>> Edits;
----------------
document that these are equivalent.
Why is Edits optional?
================
Comment at: clang-tools-extra/clangd/SourceCode.h:46
+ std::array<uint8_t, 20> ContentDigests;
+ tooling::Replacements Reps;
+ llvm::Optional<std::vector<TextEdit>> Edits;
----------------
sammccall wrote:
> document that these are equivalent.
> Why is Edits optional?
nit: reps -> replacements
ContentDigests -> ContentDigest
================
Comment at: clang-tools-extra/clangd/SourceCode.h:49
+
+ static Edit generateEdit(llvm::StringRef Code, tooling::Replacements Reps) {
+ Edit E;
----------------
make this a constructor?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66637/new/
https://reviews.llvm.org/D66637
More information about the cfe-commits
mailing list