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 @@ -520,6 +520,101 @@ } return llvm::None; } + +// Parses in documentation comments and tries to preserve the markup in the +// comment. +// Currently only linebreaks are handled. +void parseDocumentation(std::string Input, markup::Document &Output) { + + auto 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'; + }; + + auto IsMardkownLineBreak = [](llvm::StringRef Str, size_t LineBreakIndex) { + return LineBreakIndex > 1 && + (Str[LineBreakIndex - 1] == '\\' || + (Str[LineBreakIndex - 1] == ' ' && Str[LineBreakIndex - 2] == ' ')); + }; + + auto 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; + }; + + auto IsFollowedByHardLineBreakIndicator = [](llvm::StringRef Str, + size_t LineBreakIndex) { + // '-'/'*' 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; + }; + + constexpr auto WhiteSpaceChars = "\t\n\v\f\r "; + + auto TrimmedInput = llvm::StringRef(Input).trim(); + + std::string CurrentLine; + + for (size_t CharIndex = 0; CharIndex < TrimmedInput.size();) { + if (TrimmedInput[CharIndex] == '\n') { + // Trim whitespace infront of linebreak, also get rif of markdown + // linebreak character `\` + CurrentLine.erase( + CurrentLine.find_last_not_of(std::string(WhiteSpaceChars) + "\\") + + 1); + + if (IsParagraphLineBreak(TrimmedInput, CharIndex)) { + // TODO this should add an actual paragraph (double linebreak) + Output.addParagraph().appendText(CurrentLine); + CurrentLine = ""; + } else if (IsMardkownLineBreak(TrimmedInput, CharIndex) || + IsPunctuationLineBreak(TrimmedInput, CharIndex) || + IsFollowedByHardLineBreakIndicator(TrimmedInput, CharIndex)) { + Output.addParagraph().appendText(CurrentLine); + CurrentLine = ""; + } else { + // Ommit linebreak + CurrentLine += ' '; + } + + 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 = TrimmedInput.find_first_not_of(WhiteSpaceChars, CharIndex); + } else { + CurrentLine += TrimmedInput[CharIndex]; + CharIndex++; + } + } + if (!CurrentLine.empty()) { + Output.addParagraph().appendText(CurrentLine); + } +} } // namespace llvm::Optional getHover(ParsedAST &AST, Position Pos, @@ -652,7 +747,7 @@ } if (!Documentation.empty()) - Output.addParagraph().appendText(Documentation); + parseDocumentation(Documentation, Output); if (!Definition.empty()) { Output.addRuler(); 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 @@ -1883,6 +1883,118 @@ } } +TEST(Hover, LineBreakConversionDocs) { + struct Case { + const std::function Builder; + llvm::StringRef ExpectedRenderMarkdown; + llvm::StringRef ExpectedRenderPlainText; + } Cases[] = {{ + [](HoverInfo &HI) { HI.Documentation = "foo\n\n\nbar"; }, + "foo \nbar", + "foo\nbar", + }, + { + [](HoverInfo &HI) { HI.Documentation = "foo\n\n\n\tbar"; }, + "foo \nbar", + "foo\nbar", + }, + { + [](HoverInfo &HI) { HI.Documentation = "foo\n\n\n bar"; }, + "foo \nbar", + "foo\nbar", + }, + { + [](HoverInfo &HI) { HI.Documentation = "foo.\nbar"; }, + "foo. \nbar", + "foo.\nbar", + }, + { + [](HoverInfo &HI) { HI.Documentation = "foo:\nbar"; }, + "foo: \nbar", + "foo:\nbar", + }, + { + [](HoverInfo &HI) { HI.Documentation = "foo,\nbar"; }, + "foo, \nbar", + "foo,\nbar", + }, + { + [](HoverInfo &HI) { HI.Documentation = "foo;\nbar"; }, + "foo; \nbar", + "foo;\nbar", + }, + { + [](HoverInfo &HI) { HI.Documentation = "foo!\nbar"; }, + "foo! \nbar", + "foo!\nbar", + }, + { + [](HoverInfo &HI) { HI.Documentation = "foo?\nbar"; }, + "foo? \nbar", + "foo?\nbar", + }, + { + [](HoverInfo &HI) { HI.Documentation = "foo\n-bar"; }, + "foo \n-bar", + "foo\n-bar", + }, + { + [](HoverInfo &HI) { HI.Documentation = "foo\n*bar"; }, + // TODO `*` should probably not be escaped after line break + "foo \n\\*bar", + "foo\n*bar", + }, + { + [](HoverInfo &HI) { HI.Documentation = "foo\n@bar"; }, + "foo \n@bar", + "foo\n@bar", + }, + { + [](HoverInfo &HI) { HI.Documentation = "foo\n\\bar"; }, + // TODO `\` should probably not be escaped after line break + "foo \n\\\\bar", + "foo\n\\bar", + }, + { + [](HoverInfo &HI) { HI.Documentation = "foo\n>bar"; }, + // TODO `>` should probably not be escaped after line break + "foo \n\\>bar", + "foo\n>bar", + }, + { + [](HoverInfo &HI) { HI.Documentation = "foo\n#bar"; }, + "foo \n#bar", + "foo\n#bar", + }, + { + [](HoverInfo &HI) { HI.Documentation = "foo \nbar"; }, + "foo \nbar", + "foo\nbar", + }, + { + [](HoverInfo &HI) { HI.Documentation = "foo \nbar"; }, + "foo \nbar", + "foo\nbar", + }, + { + [](HoverInfo &HI) { HI.Documentation = "foo\\\nbar"; }, + "foo \nbar", + "foo\nbar", + }, + { + [](HoverInfo &HI) { HI.Documentation = "foo\nbar"; }, + "foo bar", + "foo bar", + }}; + + for (const auto &C : Cases) { + HoverInfo HI; + C.Builder(HI); + EXPECT_EQ(HI.present().asMarkdown().erase(0, 12), C.ExpectedRenderMarkdown); + EXPECT_EQ(HI.present().asPlainText(), C.ExpectedRenderPlainText); + } +} + // This is a separate test as headings don't create any differences in plaintext // mode. TEST(Hover, PresentHeadings) {