[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