[clangd] Change line break behaviour for hoverinfo

[lolleko]

Summary

parseDocumentation retains hard line breaks and removes soft line breaks inside documentation comments.
Wether a line break is hard or soft is determined by the following rules (some of which have been discussed in https://github.com/clangd/clangd/issues/95):

• Hard Markdown line breaks (https://github.github.com/gfm/#hard-line-breaks) are retained
• Line breaks that are preceded by a punctuation are retained
• Line breaks that are followed by "interesting characters" (e.g. Markdown syntax, doxygen commands) are retained
• All other line breaks are removed

Since this is my first contribution feel free to be extra thorough.

Related issue: https://github.com/clangd/clangd/issues/95

[sammccall]

I understand the main goal here is preserving intended line breaks in documentation comments? Thanks for tackling this problem!

A design goal here is that the markup objects are the source of truth about the document structure. So if we've decided we want a comment to turn into multiple lines, we should create multiple Paragraph objects, this makes it easy to adjust the rendering strategy and isolate differences between text/markdown.

I think this means this splitting needs to happen on the way in instead of on the way out. In Hover.cpp:

if (!Documentation.empty())
  Output.addParagraph().appendText(Documentation);

would be come something like

parseDocumentationComment(Output, Documentation);

which would ultimately add a paragraph to Output for each line detected in Documentation.

Even though markdown doc comments in cpp are still rare, I removed the markdown escaping

Please let's leave this out, at least from this patch, if it's not the primary goal. This is a complicated tradeoff, there's plenty of reasonably common patterns where we should be escaping, such as vector<string>. Moreover if we're treating punctuation in the comments as formatting, we should likely also be doing so in plain-text mode.

FYI D75687 (less unneccesary escaping in markdown) is also in flight and likely to touch some of the same code.

clang-tools-extra/clangd/FormattedString.cpp

103

We can separate concerns more clearly by not having this function care about markdown.
Can it have a signature like:

• vector<StringRef> splitLines(StringRef Text) (which wouldn't normalize whitespace within the lines)
• vector<string> splitLines(StringRef Text) which would

and then continue to have line rendering handled elsewhere?

[lolleko]

Addressed review.

Paragraphs (lines separated by a double line break \n\n) are currently not retained. Because the current implementation of markup::Paragraph only appends a single white space after it's contents (https://github.com/llvm/llvm-project/blob/e26e9ba288ce156b851504ebbb7d8a775572957c/clang-tools-extra/clangd/FormattedString.cpp#L368).
I think this is semantically wrong because in natural language aswell as markup languages like md, html, ... a paragraph should be followed by a blank line.
But since a lot of code relies on that single white space paragraph, this should be addressed in a different patch.

[sammccall]

Thanks, this fits in a lot better!
I think there's some more details that could be handled, e.g. if a line is much shorter than the max line length, then that probably implies it's a "real" break. But if we get the code structure right, these should be easy to plug in later.

Because the current implementation of markup::Paragraph only appends a single white space after it's contents. I think this is semantically wrong because in natural language aswell as markup languages like md, html, ... a paragraph should be followed by a blank line

Yeah, we can look at this again, but we shouldn't do so in this patch. You're right on the semantics, but real implementations (particularly VSCode) use a distasteful amount of whitespace around HTML paragraphs vs other UI elements and don't give us any control over the CSS. (Similarly rendering paragraph breaks as \n\n in plaintext feels much too heavyweight in terminal-based editors in my experience)

BTW, it's really useful to upload diffs with full file context, I think -U99999 will do this, though I tend to use arc diff which always includes it.

clang-tools-extra/clangd/Hover.cpp

524 ↗

(On Diff #251334)

I think we're going to end up using this elsewhere, e.g. in code completion.
Fine to keep it in Hover for now, but please expose it from hover.h and test it directly (rather than via HoverInfo::present())

This will also allow moving this implementation to the end of the file, rather than interleaving it with unrelated parts of hover logic.

527 ↗

(On Diff #251334)

nit: input should be StringRef since you never mutate it

529 ↗

(On Diff #251334)

nit: we'd generally use static/anon-namespace functions rather than lambdas for helpers when we don't need to capture much stuff.

583 ↗

(On Diff #251334)

I think this could be easier to read by separating the strategy from some of the details.

From my reading of the code, the strategy is:

• examine the raw lines from the input to determine where true paragraph breaks occur. (In general, based on the end of the previous line and the start of the new line)
• form each groups of raw lines into a paragraph (by trimming whitespace and \, dropping blank lines, and joining the results with spaces)

Given this, I think we could extract a couple of functions like:

• bool breakBetween(StringRef Before, StringRef After)
• std::string joinRawLines(ArrayRef<StringRef>)

and the outer loop becomes simpler, like:

SmallVector<StringRef, N> RawLines;
llvm::SplitString(Input, RawLines, "\n");
ArrayRef Remaining = RawLines;
while (!Remaining.empty()) {
  int Para = 1;
  while (Para < Remaining.size() && !breakBetween(Remaining[Para-1], Remaining[Para]))
    Para++;
  std::string ParaText = joinRawLines(Remaining.take_front(Para));
  if (!ParaText.empty())
    Output.addParagraph().appendText(std::move(ParaText));
  Remaining = Remaining.drop_front(Para);
}

I think this signature for breakBetween in terms of stringrefs would result in the lambdas you have here being a bit more readable, and the main is clearer for not dealing directly with string, whitespace, etc.

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

1886

@kadircet this feels like a case where having a debug output mode like [Para:foo][Para:bar] would be useful!

[lolleko]

Adressed 2nd Review

The problem with the changes you proposed is that it doesn't handle paragraphs (\n\n).
I think the conditionals required to make that work wouldn't be much cleaner than the current solution.

And I do think parahraphs in comments should be retained. I agree that not every hard line break should be a pargraph.
But actual parahraphs should be retained e.g.:

	/**
	 * Condition for calling UpdateOverlaps() to initialize overlap state when loaded in during level streaming.
	 * If set to 'UseConfigDefault', the default specified in ini (displayed in 'DefaultUpdateOverlapsMethodDuringLevelStreaming') will be used.
	 * If overlaps are not initialized, this actor and attached components will not have an initial state of what objects are touching it,
	 * and overlap events may only come in once one of those objects update overlaps themselves (for example when moving).
	 * However if an object touching it *does* initialize state, both objects will know about their touching state with each other.
	 * This can be a potentially large performance savings during level streaming, and is safe if the object and others initially
	 * overlapping it do not need the overlap state because they will not trigger overlap notifications.
	 * 
	 * Note that if 'bGenerateOverlapEventsDuringLevelStreaming' is true, overlaps are always updated in this case, but that flag
	 * determines whether the Begin/End overlap events are triggered.
	 * 
	 * @see bGenerateOverlapEventsDuringLevelStreaming, DefaultUpdateOverlapsMethodDuringLevelStreaming, GetUpdateOverlapsMethodDuringLevelStreaming()
	 */

I think once proper paragraphs are in. The linebreak parsing could be rewritten into 2 steps:

1. Split on \n\n to extract actual paragraphs.
2. Use the solution you proposed to split and join new lines.

[sammccall]

Thanks for this! Mostly just some cleanups/comment nits left.

The substantive change is around the \ and at end-of-line. It looks like the goal is on improving the parsing of markdown-formatted comments, but we need to focus as well on non-markdown-formatted comments as these are the majority case. (Most of your patch improves these a lot!)

If you don't have commit access yet, I'm happy to land this for you, just let me know when ready.
(A couple of patches like this are good grounds to apply for commit access, you can just link to the review threads)

clang-tools-extra/clangd/Hover.cpp

530 ↗

(On Diff #252006)

We can make use of some StringRef helpers to make this more readable.
I think this is equivalent (to the whole function body):

return Str.substr(LineBreakIndex + 1).drop_while(isWhitespace).startswith("\n");

534 ↗

(On Diff #252006)

I'm not altogether happy with these rules: many comments are not markdown, and these don't seem very safe for non-markdown comments.

Comments ending in a trailing slash may be explicitly indicating a soft line break e.g.

// Example usage:
// my_prog --some_flag = foo \
//    --remote_database=/path/to/whatever \
//    input.txt

and trailing whitespace is often just sloppiness.

I did a search over a large corpus of open-source C++.

• for //.*\ \ $: Of a random 10 results, none appear to be markdown.
• for //.*\\$: Of a random 10 results, none were markdown. (4 were explicit soft breaks, 4 were ascii art, 2 were soft breaks in commented-out macro definitions)

535 ↗

(On Diff #252006)

What about:

StringRef Prev = Str.substr(0, LineBreakIndex);
return Prev.endswith("\\") || Prev.endswith("  ")

(this seems more readable, and fixes the bug if LineBreakIndex == 1 and the string is {backslash, newline, ...}.)

544 ↗

(On Diff #252006)

nit: Punctuation.contains(...)

733 ↗

(On Diff #252006)

nit: please put the doc on the declaration rather than the definition.

(I think the previous name was more appropriate to the signature, but up to you)

750 ↗

(On Diff #252006)

rather than repeating the code twice, please just use if isParagraph() || isHard(), with the comment that we may treat paragraph differently

751 ↗

(On Diff #252006)

nit: llvm tends to use "// FIXME: "

751 ↗

(On Diff #252006)

please soften this to "maybe".
(Because we need to consider the padding/css issues of doing so, whether a hard break vs double break reflects different intents or just different conventions in different codebases, and whether we have a reasonable way to implement this - paragraphs have leading as well as trailing padding, so it gets complicated fast).

and rephrase to "should distinguish between line breaks and paragraphs".
("Double linebreak" is confusing as it doesn't mean anything at the markup::Document level, and markdown paragraph isn't the same concept as a plaintext blank line, even if they are both encoded as "\n\n")

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

1891

please add tests starting/ending with newline as these are special cases in the code

1943

please drop these comments, as they don't seem to be true (unless assuming the content is markdown, which it usually isn't)

I think we can drop most of the tests too (for specific characters after newline), as we already have escaping tests and these don't actually test different linebreaking behaviour.

[lolleko]

This comment was removed by lolleko.

[lolleko]

This comment was removed by lolleko.

[lolleko]

Final Cleanup
Removed markdown linebreak parsing for now.

No I don't have commit rights yet. Feel fre to merge this for me.

Thanks for the thorough review.

clang-tools-extra/clangd/FormattedString.cpp

 #include <string>
 #include <vector>
 
 namespace clang {
 namespace clangd {
 namespace markup {
 
 namespace {
-/// Escape a markdown text block. Ensures the punctuation will not introduce
-/// any of the markdown constructs.
-std::string renderText(llvm::StringRef Input) {
-  // Escaping ASCII punctuation ensures we can't start a markdown construct.
-  constexpr llvm::StringLiteral Punctuation =
-      R"txt(!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~)txt";
+bool isCharLineBreak(llvm::StringRef Str, size_t LineBreakIndex) {
+  return Str[LineBreakIndex] == '\n';
+}
+
+bool isCharParagraphLineBreak(llvm::StringRef Str, size_t LineBreakIndex) {
+  if (!isCharLineBreak(Str, LineBreakIndex) ||
+      LineBreakIndex + 1 >= Str.size()) {
+    return false;
+  }
+  auto NextNonSpaceCharIndex = Str.find_first_not_of(' ', LineBreakIndex + 1);
+
+  return NextNonSpaceCharIndex != llvm::StringRef::npos &&
+         Str[NextNonSpaceCharIndex] == '\n';
+}
+
+bool isCharMardkownLineBreak(llvm::StringRef Str, size_t LineBreakIndex) {
+  if (!isCharLineBreak(Str, LineBreakIndex) || LineBreakIndex <= 2) {
+    return false;
+  }
+
+  if (Str[LineBreakIndex - 1] == '\\') {
+    return true;
+  }
+
+  if ((Str[LineBreakIndex - 1] == ' ' && Str[LineBreakIndex - 2] == ' ')) {
+    return true;
+  }
+
+  return false;
+}
+
+bool isCharPunctuationLineBreak(llvm::StringRef Str, size_t LineBreakIndex) {
+  if (!isCharLineBreak(Str, LineBreakIndex) || LineBreakIndex <= 1) {
+    return false;
+  }
+
+  constexpr llvm::StringLiteral Punctuation = R"txt(.:,;!?)txt";
+
+  return Punctuation.find(Str[LineBreakIndex - 1]) != llvm::StringRef::npos;
+}
+
+bool isCharFollowedByHardLineBreakIndicator(llvm::StringRef Str,
+                                            size_t LineBreakIndex) {
+  if (!isCharLineBreak(Str, LineBreakIndex)) {
+    return false;
+  }
+
+  // '-'/'*' md list, '@'/'\' documentation command, '>' md blockquote,
+  // '#' headings, '`' code blocks
+  constexpr llvm::StringLiteral LinbreakIdenticators = R"txt(-*@\>#`)txt";
+
+  auto NextNonSpaceCharIndex = Str.find_first_not_of(' ', LineBreakIndex + 1);
+
+  if (NextNonSpaceCharIndex == llvm::StringRef::npos) {
+
+    return false;
+  }
+
+  auto FollowedBySingleCharIndicator =
+      LinbreakIdenticators.find(Str[NextNonSpaceCharIndex]) !=
+      llvm::StringRef::npos;
+
+  auto FollowedByNumberedListIndicator =
+      llvm::isDigit(Str[NextNonSpaceCharIndex]) &&
+      NextNonSpaceCharIndex + 1 < Str.size() &&
+      (Str[NextNonSpaceCharIndex + 1] == '.' ||
+       Str[NextNonSpaceCharIndex + 1] == ')');
+
+  return FollowedBySingleCharIndicator || FollowedByNumberedListIndicator;
+}
+
+// Tries to estimate which linebreak is meant to be soft and which is meant to
+// be hard. Soft linebreaks are removed. Hard linebreaks are retained or
+// converted to markdown linebreaks if \p Markdown is set.
+std::string convertLineBreaks(llvm::StringRef Input, bool Markdown = false) {
+  Input = Input.trim();
+
+  constexpr auto WhiteSpaceChars = "\t\n\v\f\r ";
 
   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;
-    R += "\\";
-    R += Input[Next];
+  R.reserve(Input.size());
 
-    From = Next + 1;
+  std::string HardLinebreak = "\n";
+  if (Markdown) {
+    HardLinebreak = "  \n";
+  }
+
+  for (size_t CharIndex = 0; CharIndex < Input.size();) {
+    if (isCharLineBreak(Input, CharIndex)) {
+      // Trim spaces infront of linebreak
+      R.erase(R.find_last_not_of(WhiteSpaceChars) + 1);
+      if (isCharParagraphLineBreak(Input, CharIndex)) {
+        R += "\n\n";
+      } else if (Markdown && isCharMardkownLineBreak(Input, CharIndex)) {
+        if (Input[CharIndex - 1] == '\\') {
+          R += '\n';
+        } else {
+          R += HardLinebreak;
+        }
+      } else if (isCharPunctuationLineBreak(Input, CharIndex)) {
+        R += HardLinebreak;
+      } else if (isCharFollowedByHardLineBreakIndicator(Input, CharIndex)) {
+        R += HardLinebreak;
+      } else {
+        // Omit line break -> replace with space
+        R += ' ';
+      }
+      CharIndex++;
+      // After a linebreak always remove spaces to avoid 4 space markdown code
+      // blocks, also skip all additional linebreaks since they have no effect
+      CharIndex = Input.find_first_not_of(WhiteSpaceChars, CharIndex);
+    } else {
+      // copy the rest of the string
+      R += Input[CharIndex];
+      CharIndex++;
+    }
   }
   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.
 std::string renderInlineBlock(llvm::StringRef Input) {
   std::string R;
for (char C : Input) {
     MaxBackticks = std::max(MaxBackticks, Backticks);
     Backticks = 0;
   }
   MaxBackticks = std::max(Backticks, MaxBackticks);
   // Use the corresponding number of backticks to start and end a code block.
   return std::string(/*Repeat=*/std::max(3u, MaxBackticks + 1), '`');
 }
 
-// Trims the input and concatenates whitespace blocks into a single ` `.
-std::string canonicalizeSpaces(std::string Input) {
-  // Goes over the string and preserves only a single ` ` for any whitespace
-  // chunks, the rest is moved to the end of the string and dropped in the end.
-  auto WritePtr = Input.begin();
-  llvm::SmallVector<llvm::StringRef, 4> Words;
-  llvm::SplitString(Input, Words);
-  if (Words.empty())
-    return "";
-  // Go over each word and add it to the string.
-  for (llvm::StringRef Word : Words) {
-    if (WritePtr > Input.begin())
-      *WritePtr++ = ' '; // Separate from previous block.
-    llvm::for_each(Word, [&WritePtr](const char C) { *WritePtr++ = C; });
-  }
-  // Get rid of extra spaces.
-  Input.resize(WritePtr - Input.begin());
-  return Input;
-}
-
 std::string renderBlocks(llvm::ArrayRef<std::unique_ptr<Block>> Children,
                          void (Block::*RenderFunc)(llvm::raw_ostream &) const) {
   std::string R;
   llvm::raw_string_ostream OS(R);
 
   // Trim rulers.
   Children = Children.drop_while(
       [](const std::unique_ptr<Block> &C) { return C->isRuler(); });

 }
 
 void Paragraph::renderMarkdown(llvm::raw_ostream &OS) const {
   llvm::StringRef Sep = "";
   for (auto &C : Chunks) {
     OS << Sep;
     switch (C.Kind) {
     case Chunk::PlainText:
-      OS << renderText(C.Contents);
+      OS << convertLineBreaks(C.Contents, true);
       break;
     case Chunk::InlineCode:
       OS << renderInlineBlock(C.Contents);
       break;
     }
     Sep = " ";
   }
   // Paragraphs are translated into markdown lines, not markdown paragraphs.
   // Therefore it only has a single linebreak afterwards.
   // VSCode requires two spaces at the end of line to start a new one.
   OS << "  \n";
 }
 
 void Paragraph::renderPlainText(llvm::raw_ostream &OS) const {
   llvm::StringRef Sep = "";
   for (auto &C : Chunks) {
-    OS << Sep << C.Contents;
+    OS << Sep << convertLineBreaks(C.Contents);
     Sep = " ";
   }
   OS << '\n';
 }
 
 void BulletList::renderMarkdown(llvm::raw_ostream &OS) const {
   for (auto &D : Items) {
     // Instead of doing this we might prefer passing Indent to children to get
     // rid of the copies, if it turns out to be a bottleneck.
     OS << "- " << indentLines(D.asMarkdown()) << '\n';
   }
   // We need a new line after list to terminate it in markdown.
   OS << '\n';
 }
 
 void BulletList::renderPlainText(llvm::raw_ostream &OS) const {
   for (auto &D : Items) {
     // Instead of doing this we might prefer passing Indent to children to get
     // rid of the copies, if it turns out to be a bottleneck.
     OS << "- " << indentLines(D.asPlainText()) << '\n';
   }
 }
 
 Paragraph &Paragraph::appendText(std::string Text) {
-  Text = canonicalizeSpaces(std::move(Text));
+  Text = llvm::StringRef(Text).trim().str();
   if (Text.empty())
     return *this;
   Chunks.emplace_back();
   Chunk &C = Chunks.back();
   C.Contents = std::move(Text);
   C.Kind = Chunk::PlainText;
   return *this;
 }
 
 Paragraph &Paragraph::appendCode(std::string Code) {
-  Code = canonicalizeSpaces(std::move(Code));
+  Code = llvm::StringRef(Code).trim().str();
   if (Code.empty())
     return *this;
   Chunks.emplace_back();
   Chunk &C = Chunks.back();
   C.Contents = std::move(Code);
   C.Kind = Chunk::InlineCode;
   return *this;
 }

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

 namespace clang {
 namespace clangd {
 namespace markup {
 namespace {
 
 TEST(Render, Escaping) {
   // Check some ASCII punctuation
   Paragraph P;
-  P.appendText("*!`");
-  EXPECT_EQ(P.asMarkdown(), "\\*\\!\\`");
-
-  // Check all ASCII punctuation.
-  P = Paragraph();
-  std::string Punctuation = R"txt(!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~)txt";
-  // Same text, with each character escaped.
-  std::string EscapedPunctuation;
-  EscapedPunctuation.reserve(2 * Punctuation.size());
-  for (char C : Punctuation)
-    EscapedPunctuation += std::string("\\") + C;
-  P.appendText(Punctuation);
-  EXPECT_EQ(P.asMarkdown(), EscapedPunctuation);
 
   // In code blocks we don't need to escape ASCII punctuation.
   P = Paragraph();
   P.appendCode("* foo !+ bar * baz");
   EXPECT_EQ(P.asMarkdown(), "`* foo !+ bar * baz`");
 
   // But we have to escape the backticks.
   P = Paragraph();
TEST(Paragraph, SeparationOfChunks) {
   EXPECT_EQ(P.asPlainText(), "after foobar");
 
   P.appendText("bat");
   EXPECT_EQ(P.asMarkdown(), "after `foobar` bat");
   EXPECT_EQ(P.asPlainText(), "after foobar bat");
 }
 
 TEST(Paragraph, ExtraSpaces) {
-  // Make sure spaces inside chunks are dropped.
+  // Make sure spaces after linebreaks are dropped.
   Paragraph P;
   P.appendText("foo\n   \t   baz");
   P.appendCode(" bar\n");
   EXPECT_EQ(P.asMarkdown(), "foo baz `bar`");
   EXPECT_EQ(P.asPlainText(), "foo baz bar");
 }
 
-TEST(Paragraph, NewLines) {
+TEST(Paragraph, LineBreakTrim) {
   // New lines before and after chunks are dropped.
   Paragraph P;
   P.appendText(" \n foo\nbar\n ");
   P.appendCode(" \n foo\nbar \n ");
-  EXPECT_EQ(P.asMarkdown(), "foo bar `foo bar`");
+  EXPECT_EQ(P.asMarkdown(), "foo bar `foo\nbar`");
   EXPECT_EQ(P.asPlainText(), "foo bar foo bar");
 }
 
+TEST(Paragraph, LineBreakConversionParagraphs) {
+  // Paragraphs are eindicated by double linebreaks
+  // additional linebreaks and spaces should be dropped
+  constexpr auto Expected = "foo\n\nbar";
+
+  Paragraph P1;
+  P1.appendText("foo\n\n\n\nbar");
+  EXPECT_EQ(P1.asMarkdown(), Expected);
+  EXPECT_EQ(P1.asPlainText(), Expected);
+
+  Paragraph P2;
+  P2.appendText("foo\n\n\n\tbar");
+  EXPECT_EQ(P2.asMarkdown(), Expected);
+  EXPECT_EQ(P2.asPlainText(), Expected);
+
+  Paragraph P3;
+  P3.appendText("foo\n\n\n bar");
+  EXPECT_EQ(P3.asMarkdown(), Expected);
+  EXPECT_EQ(P3.asPlainText(), Expected);
+}
+
+TEST(Paragraph, LineBreakConversionPunctuation) {
+  // Keep linebreaks after punctuation
+  constexpr llvm::StringLiteral Punctuation = R"txt(.:,;!?)txt";
+
+  for (auto Char : Punctuation) {
+    const auto TestString = std::string("foo") + Char + "\nbar";
+    const auto MarkdownExpected = std::string("foo") + Char + "  \nbar";
+
+    Paragraph P;
+    P.appendText(TestString);
+    EXPECT_EQ(P.asMarkdown(), MarkdownExpected);
+    EXPECT_EQ(P.asPlainText(), TestString);
+  }
+}
+
+TEST(Paragraph, LineBreakConversionIndicator) {
+  // Keep linebreaks before interesting characters
+  constexpr llvm::StringLiteral LinbreakIdenticators = R"txt(-*@\>#`)txt";
+
+  for (auto Char : LinbreakIdenticators) {
+    const auto TestString = std::string("foo\n") + Char + "bar";
+    const auto MarkdownExpected = std::string("foo  \n") + Char + "bar";
+
+    Paragraph P;
+    P.appendText(TestString);
+    EXPECT_EQ(P.asMarkdown(), MarkdownExpected);
+    EXPECT_EQ(P.asPlainText(), TestString);
+  }
+}
+
+TEST(Paragraph, LineBreakConversionMarkdownLineBreaks) {
+  // Keep Markdown linebreaks
+  Paragraph P1;
+  P1.appendText("foo  \nbar");
+  EXPECT_EQ(P1.asMarkdown(), "foo  \nbar");
+  EXPECT_EQ(P1.asPlainText(), "foo bar");
+
+  Paragraph P2;
+  P2.appendText("foo    \nbar");
+  EXPECT_EQ(P2.asMarkdown(), "foo  \nbar");
+  EXPECT_EQ(P2.asPlainText(), "foo bar");
+
+  Paragraph P3;
+  P3.appendText("foo\\\nbar");
+  EXPECT_EQ(P3.asMarkdown(), "foo\\\nbar");
+  EXPECT_EQ(P3.asPlainText(), "foo\\ bar");
+}
+
+TEST(Paragraph, LineBreakConversionRemoveSoftLineBreaks) {
+  // Remove soft linebreaks with no interesting context
+  Paragraph P1;
+  P1.appendText("foo\nbar");
+  EXPECT_EQ(P1.asMarkdown(), "foo bar");
+  EXPECT_EQ(P1.asPlainText(), "foo bar");
+}
+
 TEST(Document, Separators) {
   Document D;
   D.addParagraph().appendText("foo");
   D.addCodeBlock("test");
   D.addParagraph().appendText("bar");
 
   const char ExpectedMarkdown[] = R"md(foo  
 ```cpp

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

def)",
 
   for (const auto &C : Cases) {
     HoverInfo HI;
     C.Builder(HI);
     EXPECT_EQ(HI.present().asPlainText(), C.ExpectedRender);
   }
 }
 
 // This is a separate test as headings don't create any differences in plaintext
 // mode.
 TEST(Hover, PresentHeadings) {
   HoverInfo HI;
   HI.Kind = index::SymbolKind::Variable;
   HI.Name = "foo";
 
   EXPECT_EQ(HI.present().asMarkdown(), "### variable `foo`");
 }
 
 // This is a separate test as rulers behave differently in markdown vs
 // plaintext.
 TEST(Hover, PresentRulers) {
   HoverInfo HI;
   HI.Kind = index::SymbolKind::Variable;
   HI.Name = "foo";
   HI.Value = "val";
   HI.Definition = "def";
 
   llvm::StringRef ExpectedMarkdown = R"md(### variable `foo`  
 
 ---
-Value \= `val`  
+Value = `val`  
 
 ---
 ```cpp
 def
 ```)md";
   EXPECT_EQ(HI.present().asMarkdown(), ExpectedMarkdown);
 
   llvm::StringRef ExpectedPlaintext = R"pt(variable foo
 
 Value = val
 
 def)pt";
   EXPECT_EQ(HI.present().asPlainText(), ExpectedPlaintext);
 }
 } // namespace
 } // namespace clangd
 } // namespace clang