diff --git a/clang-tools-extra/clangd/Hover.h b/clang-tools-extra/clangd/Hover.h --- a/clang-tools-extra/clangd/Hover.h +++ b/clang-tools-extra/clangd/Hover.h @@ -75,7 +75,7 @@ markup::Document present() const; }; -void parseLineBreaks(llvm::StringRef Input, markup::Document &Output); +void parseDoucmentation(llvm::StringRef Input, markup::Document &Output); llvm::raw_ostream &operator<<(llvm::raw_ostream &, const HoverInfo::Param &); inline bool operator==(const HoverInfo::Param &LHS, diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp --- a/clang-tools-extra/clangd/Hover.cpp +++ b/clang-tools-extra/clangd/Hover.cpp @@ -522,26 +522,15 @@ } bool isParagraphLineBreak(llvm::StringRef Str, size_t LineBreakIndex) { - if (LineBreakIndex + 1 >= Str.size()) { - return false; - } - auto NextNonSpaceCharIndex = Str.find_first_not_of(' ', LineBreakIndex + 1); - - return NextNonSpaceCharIndex != llvm::StringRef::npos && - Str[NextNonSpaceCharIndex] == '\n'; -}; - -bool isMardkownLineBreak(llvm::StringRef Str, size_t LineBreakIndex) { - return LineBreakIndex > 1 && - (Str[LineBreakIndex - 1] == '\\' || - (Str[LineBreakIndex - 1] == ' ' && Str[LineBreakIndex - 2] == ' ')); + return Str.substr(LineBreakIndex + 1) + .drop_while([](auto C) { return C == ' ' || C == '\t'; }) + .startswith("\n"); }; bool isPunctuationLineBreak(llvm::StringRef Str, size_t LineBreakIndex) { constexpr llvm::StringLiteral Punctuation = R"txt(.:,;!?)txt"; - return LineBreakIndex > 0 && - Punctuation.find(Str[LineBreakIndex - 1]) != llvm::StringRef::npos; + return LineBreakIndex > 0 && Punctuation.contains(Str[LineBreakIndex - 1]); }; bool isFollowedByHardLineBreakIndicator(llvm::StringRef Str, @@ -570,8 +559,7 @@ }; bool isHardLineBreak(llvm::StringRef Str, size_t LineBreakIndex) { - return isMardkownLineBreak(Str, LineBreakIndex) || - isPunctuationLineBreak(Str, LineBreakIndex) || + return isPunctuationLineBreak(Str, LineBreakIndex) || isFollowedByHardLineBreakIndicator(Str, LineBreakIndex); } @@ -707,7 +695,7 @@ } if (!Documentation.empty()) - parseLineBreaks(Documentation, Output); + parseDoucmentation(Documentation, Output); if (!Definition.empty()) { Output.addRuler(); @@ -730,8 +718,7 @@ return Output; } -// Tries to retain hard line breaks and drops soft line breaks. -void parseLineBreaks(llvm::StringRef Input, markup::Document &Output) { +void parseDoucmentation(llvm::StringRef Input, markup::Document &Output) { constexpr auto WhiteSpaceChars = "\t\n\v\f\r "; @@ -741,17 +728,14 @@ for (size_t CharIndex = 0; CharIndex < TrimmedInput.size();) { if (TrimmedInput[CharIndex] == '\n') { - // Trim whitespace infront of linebreak, also get rif of markdown - // linebreak character `\` + // Trim whitespace infront of linebreak const auto LastNonSpaceCharIndex = - CurrentLine.find_last_not_of(std::string(WhiteSpaceChars) + "\\") + 1; + CurrentLine.find_last_not_of(WhiteSpaceChars) + 1; CurrentLine.erase(LastNonSpaceCharIndex); - if (isParagraphLineBreak(TrimmedInput, CharIndex)) { - // TODO this should add an actual paragraph (double linebreak) - Output.addParagraph().appendText(CurrentLine); - CurrentLine = ""; - } else if (isHardLineBreak(TrimmedInput, CharIndex)) { + if (isParagraphLineBreak(TrimmedInput, CharIndex) || + isHardLineBreak(TrimmedInput, CharIndex)) { + // FIXME: maybe distinguish between line breaks and paragraphs Output.addParagraph().appendText(CurrentLine); CurrentLine = ""; } else { diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp --- a/clang-tools-extra/clangd/unittests/HoverTests.cpp +++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp @@ -1889,6 +1889,26 @@ llvm::StringRef ExpectedRenderMarkdown; llvm::StringRef ExpectedRenderPlainText; } Cases[] = {{ + " \n foo\nbar", + "foo bar", + "foo bar", + }, + { + "foo\nbar \n ", + "foo bar", + "foo bar", + }, + { + "foo \nbar", + "foo bar", + "foo bar", + }, + { + "foo \nbar", + "foo bar", + "foo bar", + }, + { "foo\n\n\nbar", "foo \nbar", "foo\nbar", @@ -1908,79 +1928,11 @@ "foo. \nbar", "foo.\nbar", }, - { - "foo:\nbar", - "foo: \nbar", - "foo:\nbar", - }, - { - "foo,\nbar", - "foo, \nbar", - "foo,\nbar", - }, - { - "foo;\nbar", - "foo; \nbar", - "foo;\nbar", - }, - { - "foo!\nbar", - "foo! \nbar", - "foo!\nbar", - }, - { - "foo?\nbar", - "foo? \nbar", - "foo?\nbar", - }, - { - "foo\n-bar", - "foo \n-bar", - "foo\n-bar", - }, { "foo\n*bar", - // TODO `*` should probably not be escaped after line break "foo \n\\*bar", "foo\n*bar", }, - { - "foo\n@bar", - "foo \n@bar", - "foo\n@bar", - }, - { - "foo\n\\bar", - // TODO `\` should probably not be escaped after line break - "foo \n\\\\bar", - "foo\n\\bar", - }, - { - "foo\n>bar", - // TODO `>` should probably not be escaped after line break - "foo \n\\>bar", - "foo\n>bar", - }, - { - "foo\n#bar", - "foo \n#bar", - "foo\n#bar", - }, - { - "foo \nbar", - "foo \nbar", - "foo\nbar", - }, - { - "foo \nbar", - "foo \nbar", - "foo\nbar", - }, - { - "foo\\\nbar", - "foo \nbar", - "foo\nbar", - }, { "foo\nbar", "foo bar", @@ -1989,7 +1941,7 @@ for (const auto &C : Cases) { markup::Document Output; - parseLineBreaks(C.Documentation, Output); + parseDoucmentation(C.Documentation, Output); EXPECT_EQ(Output.asMarkdown(), C.ExpectedRenderMarkdown); EXPECT_EQ(Output.asPlainText(), C.ExpectedRenderPlainText);