[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/FormattedString.cpp:95
+// Concatanates whitespace blocks into a single ` `.
+std::string canonicalizeSpaces(std::string Input) {
+  // Goes over the string and preserves only a single ` ` for any whitespace
----------------
sammccall wrote:
> is this equivalent to
> ```
> SmallVector<StringRef> Words;
> llvm::SplitString(Input, Words);
> return llvm::join(Input, " ");
> ```
> ?
> 
> (That would trim leading/trailing whitespace, but I suspect we actually want that)
yes that's exactly what we want, thanks!
not just joining them though, since we could still merge them in-place.


================
Comment at: clang-tools-extra/clangd/FormattedString.h:30
+  virtual std::string renderAsPlainText() const = 0;
+
+  virtual ~RenderableBlock() = default;
----------------
sammccall wrote:
> renderfortests has gone. What's the plan for testing Hover rendering? (i.e. HoverInfo -> Document)
> I guess asserting the plain text and markdown might be enough...
> 
> (Did we remove *all* the tests for `present()`in D70911, and I didn't notice? We should restore some based on synthetic HoverInfo)
yes we I was planning to check using the plaintext output with some "synthetic" hoverinfo. Currently only lit tests are checking for the behavior of present.
there's also changes to hover.cpp in this patch to preserve current output. happy to add some tests before moving forward, but didn't want to add any as they
will change drastically with the new rendering layout.


================
Comment at: clang-tools-extra/clangd/FormattedString.h:35
+/// Represents parts of the markup that can contain strings, like inline code,
+/// code block or plain text.
+/// Only CodeBlocks guarantees conservation of new lines within text. One must
----------------
sammccall wrote:
> A code block doesn't go in a paragraph, as it's a block itself.
> 
> "A paragraph contains a logical line of rich text. It may be wrapped for display"
sending out codeblocks in a new patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71248