[PATCH] D137909: [Support] Allow complex names for annotation points and ranges via ${}

[Tom Praschan via Phabricator via llvm-commits]

tom-anders added inline comments.


================
Comment at: clang-tools-extra/clangd/unittests/Annotations.h:30
 
+  struct Position : clangd::Position {
+    llvm::Optional<llvm::StringRef> Payload;
----------------
tom-anders wrote:
> I thought about just using `std::pair<clangd::Position, llvm::Optional<llvm::StringRef>> instead, but that is pretty verbose and unreadable here IMO. Another nice thing about subclassing here is that we can keep the API stable and don't have to introduce  an additional `pointWithPayload()` getter like in the base class.
Ok the part about not needing API changes was a total lie, I had a typo so that ninja would only build clang but not clangd tests - I now kept the struct but added the same `withPayload` methods like in llvm::Annotations


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137909