Skip to content

Commit b194e7d

Browse files
lollekosam-mccall
authored andcommitted
[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 clangd/clangd#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: clangd/clangd#95 Differential Revision: https://reviews.llvm.org/D76094
1 parent 68687e7 commit b194e7d

File tree

3 files changed

+152
-1
lines changed

3 files changed

+152
-1
lines changed

clang-tools-extra/clangd/Hover.cpp

Lines changed: 83 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,49 @@ llvm::Optional<HoverInfo> getHoverContents(const Expr *E, ParsedAST &AST) {
520520
}
521521
return llvm::None;
522522
}
523+
524+
bool isParagraphLineBreak(llvm::StringRef Str, size_t LineBreakIndex) {
525+
return Str.substr(LineBreakIndex + 1)
526+
.drop_while([](auto C) { return C == ' ' || C == '\t'; })
527+
.startswith("\n");
528+
};
529+
530+
bool isPunctuationLineBreak(llvm::StringRef Str, size_t LineBreakIndex) {
531+
constexpr llvm::StringLiteral Punctuation = R"txt(.:,;!?)txt";
532+
533+
return LineBreakIndex > 0 && Punctuation.contains(Str[LineBreakIndex - 1]);
534+
};
535+
536+
bool isFollowedByHardLineBreakIndicator(llvm::StringRef Str,
537+
size_t LineBreakIndex) {
538+
// '-'/'*' md list, '@'/'\' documentation command, '>' md blockquote,
539+
// '#' headings, '`' code blocks
540+
constexpr llvm::StringLiteral LinbreakIdenticators = R"txt(-*@\>#`)txt";
541+
542+
auto NextNonSpaceCharIndex = Str.find_first_not_of(' ', LineBreakIndex + 1);
543+
544+
if (NextNonSpaceCharIndex == llvm::StringRef::npos) {
545+
return false;
546+
}
547+
548+
auto FollowedBySingleCharIndicator =
549+
LinbreakIdenticators.find(Str[NextNonSpaceCharIndex]) !=
550+
llvm::StringRef::npos;
551+
552+
auto FollowedByNumberedListIndicator =
553+
llvm::isDigit(Str[NextNonSpaceCharIndex]) &&
554+
NextNonSpaceCharIndex + 1 < Str.size() &&
555+
(Str[NextNonSpaceCharIndex + 1] == '.' ||
556+
Str[NextNonSpaceCharIndex + 1] == ')');
557+
558+
return FollowedBySingleCharIndicator || FollowedByNumberedListIndicator;
559+
};
560+
561+
bool isHardLineBreak(llvm::StringRef Str, size_t LineBreakIndex) {
562+
return isPunctuationLineBreak(Str, LineBreakIndex) ||
563+
isFollowedByHardLineBreakIndicator(Str, LineBreakIndex);
564+
}
565+
523566
} // namespace
524567

525568
llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
@@ -652,7 +695,7 @@ markup::Document HoverInfo::present() const {
652695
}
653696

654697
if (!Documentation.empty())
655-
Output.addParagraph().appendText(Documentation);
698+
parseDocumentation(Documentation, Output);
656699

657700
if (!Definition.empty()) {
658701
Output.addRuler();
@@ -675,6 +718,45 @@ markup::Document HoverInfo::present() const {
675718
return Output;
676719
}
677720

721+
void parseDocumentation(llvm::StringRef Input, markup::Document &Output) {
722+
723+
constexpr auto WhiteSpaceChars = "\t\n\v\f\r ";
724+
725+
auto TrimmedInput = Input.trim();
726+
727+
std::string CurrentLine;
728+
729+
for (size_t CharIndex = 0; CharIndex < TrimmedInput.size();) {
730+
if (TrimmedInput[CharIndex] == '\n') {
731+
// Trim whitespace infront of linebreak
732+
const auto LastNonSpaceCharIndex =
733+
CurrentLine.find_last_not_of(WhiteSpaceChars) + 1;
734+
CurrentLine.erase(LastNonSpaceCharIndex);
735+
736+
if (isParagraphLineBreak(TrimmedInput, CharIndex) ||
737+
isHardLineBreak(TrimmedInput, CharIndex)) {
738+
// FIXME: maybe distinguish between line breaks and paragraphs
739+
Output.addParagraph().appendText(CurrentLine);
740+
CurrentLine = "";
741+
} else {
742+
// Ommit linebreak
743+
CurrentLine += ' ';
744+
}
745+
746+
CharIndex++;
747+
// After a linebreak always remove spaces to avoid 4 space markdown code
748+
// blocks, also skip all additional linebreaks since they have no effect
749+
CharIndex = TrimmedInput.find_first_not_of(WhiteSpaceChars, CharIndex);
750+
} else {
751+
CurrentLine += TrimmedInput[CharIndex];
752+
CharIndex++;
753+
}
754+
}
755+
if (!CurrentLine.empty()) {
756+
Output.addParagraph().appendText(CurrentLine);
757+
}
758+
}
759+
678760
llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,
679761
const HoverInfo::Param &P) {
680762
std::vector<llvm::StringRef> Output;

clang-tools-extra/clangd/Hover.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,10 @@ struct HoverInfo {
7474
/// Produce a user-readable information.
7575
markup::Document present() const;
7676
};
77+
78+
// Try to infer structure of a documentation comment (e.g. line breaks).
79+
void parseDocumentation(llvm::StringRef Input, markup::Document &Output);
80+
7781
llvm::raw_ostream &operator<<(llvm::raw_ostream &, const HoverInfo::Param &);
7882
inline bool operator==(const HoverInfo::Param &LHS,
7983
const HoverInfo::Param &RHS) {

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

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1883,6 +1883,71 @@ def)",
18831883
}
18841884
}
18851885

1886+
TEST(Hover, DocCommentLineBreakConversion) {
1887+
struct Case {
1888+
llvm::StringRef Documentation;
1889+
llvm::StringRef ExpectedRenderMarkdown;
1890+
llvm::StringRef ExpectedRenderPlainText;
1891+
} Cases[] = {{
1892+
" \n foo\nbar",
1893+
"foo bar",
1894+
"foo bar",
1895+
},
1896+
{
1897+
"foo\nbar \n ",
1898+
"foo bar",
1899+
"foo bar",
1900+
},
1901+
{
1902+
"foo \nbar",
1903+
"foo bar",
1904+
"foo bar",
1905+
},
1906+
{
1907+
"foo \nbar",
1908+
"foo bar",
1909+
"foo bar",
1910+
},
1911+
{
1912+
"foo\n\n\nbar",
1913+
"foo \nbar",
1914+
"foo\nbar",
1915+
},
1916+
{
1917+
"foo\n\n\n\tbar",
1918+
"foo \nbar",
1919+
"foo\nbar",
1920+
},
1921+
{
1922+
"foo\n\n\n bar",
1923+
"foo \nbar",
1924+
"foo\nbar",
1925+
},
1926+
{
1927+
"foo.\nbar",
1928+
"foo. \nbar",
1929+
"foo.\nbar",
1930+
},
1931+
{
1932+
"foo\n*bar",
1933+
"foo \n\\*bar",
1934+
"foo\n*bar",
1935+
},
1936+
{
1937+
"foo\nbar",
1938+
"foo bar",
1939+
"foo bar",
1940+
}};
1941+
1942+
for (const auto &C : Cases) {
1943+
markup::Document Output;
1944+
parseDocumentation(C.Documentation, Output);
1945+
1946+
EXPECT_EQ(Output.asMarkdown(), C.ExpectedRenderMarkdown);
1947+
EXPECT_EQ(Output.asPlainText(), C.ExpectedRenderPlainText);
1948+
}
1949+
}
1950+
18861951
// This is a separate test as headings don't create any differences in plaintext
18871952
// mode.
18881953
TEST(Hover, PresentHeadings) {

0 commit comments

Comments
 (0)