[clang-tools-extra] b194e7d - [clangd] Change line break behaviour for hoverinfo
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 24 04:41:44 PDT 2020
Author: Lorenz Junglas
Date: 2020-03-24T12:41:08+01:00
New Revision: b194e7d6313be3b6e6db6e2d617a76c6dde2651b
URL: https://github.com/llvm/llvm-project/commit/b194e7d6313be3b6e6db6e2d617a76c6dde2651b
DIFF: https://github.com/llvm/llvm-project/commit/b194e7d6313be3b6e6db6e2d617a76c6dde2651b.diff
LOG: [clangd] Change line break behaviour for hoverinfo
`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):
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
Related issue: https://github.com/clangd/clangd/issues/95
Differential Revision: https://reviews.llvm.org/D76094
Added:
Modified:
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/Hover.h
clang-tools-extra/clangd/unittests/HoverTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp
index 5c1288c14b58..1d41f0a3e046 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -520,6 +520,49 @@ llvm::Optional<HoverInfo> getHoverContents(const Expr *E, ParsedAST &AST) {
}
return llvm::None;
}
+
+bool isParagraphLineBreak(llvm::StringRef Str, size_t LineBreakIndex) {
+ 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.contains(Str[LineBreakIndex - 1]);
+};
+
+bool 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;
+};
+
+bool isHardLineBreak(llvm::StringRef Str, size_t LineBreakIndex) {
+ return isPunctuationLineBreak(Str, LineBreakIndex) ||
+ isFollowedByHardLineBreakIndicator(Str, LineBreakIndex);
+}
+
} // namespace
llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
@@ -652,7 +695,7 @@ markup::Document HoverInfo::present() const {
}
if (!Documentation.empty())
- Output.addParagraph().appendText(Documentation);
+ parseDocumentation(Documentation, Output);
if (!Definition.empty()) {
Output.addRuler();
@@ -675,6 +718,45 @@ markup::Document HoverInfo::present() const {
return Output;
}
+void parseDocumentation(llvm::StringRef Input, markup::Document &Output) {
+
+ constexpr auto WhiteSpaceChars = "\t\n\v\f\r ";
+
+ auto TrimmedInput = Input.trim();
+
+ std::string CurrentLine;
+
+ for (size_t CharIndex = 0; CharIndex < TrimmedInput.size();) {
+ if (TrimmedInput[CharIndex] == '\n') {
+ // Trim whitespace infront of linebreak
+ const auto LastNonSpaceCharIndex =
+ CurrentLine.find_last_not_of(WhiteSpaceChars) + 1;
+ CurrentLine.erase(LastNonSpaceCharIndex);
+
+ if (isParagraphLineBreak(TrimmedInput, CharIndex) ||
+ isHardLineBreak(TrimmedInput, CharIndex)) {
+ // FIXME: maybe distinguish between line breaks and paragraphs
+ 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);
+ }
+}
+
llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,
const HoverInfo::Param &P) {
std::vector<llvm::StringRef> Output;
diff --git a/clang-tools-extra/clangd/Hover.h b/clang-tools-extra/clangd/Hover.h
index 40a10ff6a63f..ef3bd9f22d95 100644
--- a/clang-tools-extra/clangd/Hover.h
+++ b/clang-tools-extra/clangd/Hover.h
@@ -74,6 +74,10 @@ struct HoverInfo {
/// Produce a user-readable information.
markup::Document present() const;
};
+
+// Try to infer structure of a documentation comment (e.g. line breaks).
+void parseDocumentation(llvm::StringRef Input, markup::Document &Output);
+
llvm::raw_ostream &operator<<(llvm::raw_ostream &, const HoverInfo::Param &);
inline bool operator==(const HoverInfo::Param &LHS,
const HoverInfo::Param &RHS) {
diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index c243346a73f6..593fb16e2194 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1883,6 +1883,71 @@ def)",
}
}
+TEST(Hover, DocCommentLineBreakConversion) {
+ struct Case {
+ llvm::StringRef Documentation;
+ 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",
+ },
+ {
+ "foo\n\n\n\tbar",
+ "foo \nbar",
+ "foo\nbar",
+ },
+ {
+ "foo\n\n\n bar",
+ "foo \nbar",
+ "foo\nbar",
+ },
+ {
+ "foo.\nbar",
+ "foo. \nbar",
+ "foo.\nbar",
+ },
+ {
+ "foo\n*bar",
+ "foo \n\\*bar",
+ "foo\n*bar",
+ },
+ {
+ "foo\nbar",
+ "foo bar",
+ "foo bar",
+ }};
+
+ for (const auto &C : Cases) {
+ markup::Document Output;
+ parseDocumentation(C.Documentation, Output);
+
+ EXPECT_EQ(Output.asMarkdown(), C.ExpectedRenderMarkdown);
+ EXPECT_EQ(Output.asPlainText(), C.ExpectedRenderPlainText);
+ }
+}
+
// This is a separate test as headings don't create any
diff erences in plaintext
// mode.
TEST(Hover, PresentHeadings) {
More information about the cfe-commits
mailing list