[clangd] Preserve line break when rendering text chunks of markdown

[ilya-biryukov]

Summary

Fixes https://github.com/clangd/clangd/issues/95

[sammccall]

Added a bit of a braindump to https://github.com/clangd/clangd/issues/95

I think always forcing a break is maybe at least as bad as forcing no break, and we should aim to be cleverer.
Some (relatively) simple punctuation/capitalization/line-length based heuristics may get us a long way here - is it ok to punt on this for a while?

This change would be consistent with how we handle plaintext, but including the breaks seems less problematic there: it's not clear editors are supposed to rewrap plaintext if we unwrap it, whereas markdown is very clear about that.

[ilya-biryukov]

The heuristics for properly rendering the comments are probably a good way to go.
I'd say this change is still a step in the right direction - text blocks in formatted strings should be properly escaped to avoid being interpreted like markdown constructs.
Any structure we want to have should be explicit.

However, landing this without the aforementioned heuristics means changing our behavior and since some folks like the current one better, I'll just keep this change in a limbo until we get to those heuristics.

clang-tools-extra/clangd/FormattedString.cpp

 
 namespace {
 /// Escape a markdown text block. Ensures the punctuation will not introduce
 /// any of the markdown constructs.
 static std::string renderText(llvm::StringRef Input) {
   // Escaping ASCII punctiation ensures we can't start a markdown construct.
   constexpr llvm::StringLiteral Punctuation =
       R"txt(!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~)txt";
-
   std::string R;
-  for (size_t From = 0; From < Input.size();) {
-    size_t Next = Input.find_first_of(Punctuation, From);
-    R += Input.substr(From, Next - From);
-    if (Next == llvm::StringRef::npos)
-      break;
+  for (char C : Input) {
+    if (Punctuation.contains(C)) {
+      // Has to be escaped.
       R += "\\";
-    R += Input[Next];
-
-    From = Next + 1;
+    } else if (C == '\n') {
+      // Putting two spaces at the end of the line preserves line breaks.
+      R += "  ";
+    }
+    R += C;
   }
   return R;
 }
 
 /// Renders \p Input as an inline block of code in markdown. The returned value
 /// is surrounded by backticks and the inner contents are properly escaped.
 static std::string renderInlineBlock(llvm::StringRef Input) {
   std::string R;

clang-tools-extra/clangd/unittests/FormattedStringTests.cpp

TEST(FormattedString, MarkdownWhitespace) {
   S = FormattedString();
   S.appendText("foo ");
   S.appendInlineCode("bar");
   EXPECT_EQ(S.renderAsMarkdown(), "foo `bar`");
 
   S = FormattedString();
   S.appendText("foo\n");
   S.appendInlineCode("bar");
-  EXPECT_EQ(S.renderAsMarkdown(), "foo\n`bar`");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo  \n`bar`");
 
   S = FormattedString();
   S.appendInlineCode("foo");
   S.appendText(" bar");
   EXPECT_EQ(S.renderAsMarkdown(), "`foo` bar");
 
   S = FormattedString();
   S.appendText("foo");
   S.appendCodeBlock("bar");
   S.appendText("baz");
   EXPECT_EQ(S.renderAsMarkdown(), "foo\n```cpp\nbar\n```\nbaz");
 }
 
+TEST(FormattedString, MarkdownLineBreaks) {
+  FormattedString S;
+  S.appendText("foo\nbar\nbaz");
+  // To preserve line breaks, two spaces are added at the end of each line.
+  EXPECT_EQ(S.renderAsMarkdown(), "foo  \nbar  \nbaz");
+}
 
 } // namespace
 } // namespace clangd
 } // namespace clang