[PATCH] D58547: [clangd] Introduce intermediate representation of formatted text

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

As discussed, it probably makes sense to split into two patches, adding this abstraction and using it.
(All together is also fine but should have the tests for the new behavior, which probably means renderForTests() too)

Generally looks good. I think we'll run into the limitations as soon as we want to add lists, but that's OK.



================
Comment at: clang-tools-extra/clangd/FormattedString.cpp:71
+  // start and end the code block with more.
+  unsigned MaxBackticks = 0;
+  for (llvm::StringRef Left = Input;;) {
----------------
```
backticks = "```"
while (Input.contains(backticks))
  backticks.push_back("`")
```

I don't think we care about DOS here?


================
Comment at: clang-tools-extra/clangd/FormattedString.cpp:90
+void FormattedString::appendText(std::string Text) {
+  if (Chunks.empty() || Chunks.back().Kind != ChunkKind::PlainText) {
+    Chunk C;
----------------
comment for this merge?


================
Comment at: clang-tools-extra/clangd/FormattedString.cpp:107
+void FormattedString::appendInlineCode(std::string Code) {
+  assert(!llvm::StringRef(Code).contains("\n"));
+
----------------
I'm not sure we're realistically going to be able to enforce this very well, we're going to use this for types. Maybe translate into ' ' instead? (or at least in production mode)


================
Comment at: clang-tools-extra/clangd/FormattedString.cpp:140
+
+static bool IsWhitespace(char C) {
+  return llvm::StringLiteral(" \t\n").contains(C);
----------------
(we could also use the version in CharInfo)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58547