lolleko updated this revision to Diff 252159.
lolleko added a comment.
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.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76094/new/
https://reviews.llvm.org/D76094
Files:
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/Hover.h
clang-tools-extra/clangd/unittests/HoverTests.cpp
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ 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);
+ parseDocumentation(C.Documentation, Output);
EXPECT_EQ(Output.asMarkdown(), C.ExpectedRenderMarkdown);
EXPECT_EQ(Output.asPlainText(), C.ExpectedRenderPlainText);
Index: clang-tools-extra/clangd/Hover.h
===================================================================
--- clang-tools-extra/clangd/Hover.h
+++ clang-tools-extra/clangd/Hover.h
@@ -75,7 +75,7 @@
markup::Document present() const;
};
-void parseLineBreaks(llvm::StringRef Input, markup::Document &Output);
+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,
Index: clang-tools-extra/clangd/Hover.cpp
===================================================================
--- clang-tools-extra/clangd/Hover.cpp
+++ 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);
+ parseDocumentation(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 parseDocumentation(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 {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
