Skip to content

[clangd] Improve Markup Rendering #140498

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions clang-tools-extra/clangd/ClangdLSPServer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1262,11 +1262,9 @@ void ClangdLSPServer::onHover(const TextDocumentPositionParams &Params,
R.contents.kind = HoverContentFormat;
R.range = (*H)->SymRange;
switch (HoverContentFormat) {
case MarkupKind::PlainText:
R.contents.value = (*H)->present().asPlainText();
return Reply(std::move(R));
case MarkupKind::Markdown:
R.contents.value = (*H)->present().asMarkdown();
case MarkupKind::PlainText:
R.contents.value = (*H)->present(HoverContentFormat);
return Reply(std::move(R));
};
llvm_unreachable("unhandled MarkupKind");
Expand Down
6 changes: 5 additions & 1 deletion clang-tools-extra/clangd/CodeComplete.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,11 @@ MarkupContent renderDoc(const markup::Document &Doc, MarkupKind Kind) {
Result.value.append(Doc.asPlainText());
break;
case MarkupKind::Markdown:
Result.value.append(Doc.asMarkdown());
if (Config::current().Documentation.CommentFormat ==
Config::CommentFormatPolicy::PlainText)
Result.value.append(Doc.asEscapedMarkdown());
else
Result.value.append(Doc.asMarkdown());
break;
}
return Result;
Expand Down
13 changes: 13 additions & 0 deletions clang-tools-extra/clangd/Config.h
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,19 @@ struct Config {
/// Controls highlighting modifiers that are disabled.
std::vector<std::string> DisabledModifiers;
} SemanticTokens;

enum class CommentFormatPolicy {
/// Treat comments as plain text.
PlainText,
/// Treat comments as Markdown.
Markdown,
/// Treat comments as doxygen.
Doxygen,
};

struct {
CommentFormatPolicy CommentFormat = CommentFormatPolicy::PlainText;
} Documentation;
};

} // namespace clangd
Expand Down
16 changes: 16 additions & 0 deletions clang-tools-extra/clangd/ConfigCompile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ struct FragmentCompiler {
compile(std::move(F.InlayHints));
compile(std::move(F.SemanticTokens));
compile(std::move(F.Style));
compile(std::move(F.Documentation));
}

void compile(Fragment::IfBlock &&F) {
Expand Down Expand Up @@ -760,6 +761,21 @@ struct FragmentCompiler {
}
}

void compile(Fragment::DocumentationBlock &&F) {
if (F.CommentFormat) {
if (auto Val =
compileEnum<Config::CommentFormatPolicy>("CommentFormat",
*F.CommentFormat)
.map("Plaintext", Config::CommentFormatPolicy::PlainText)
.map("Markdown", Config::CommentFormatPolicy::Markdown)
.map("Doxygen", Config::CommentFormatPolicy::Doxygen)
.value())
Out.Apply.push_back([Val](const Params &, Config &C) {
C.Documentation.CommentFormat = *Val;
});
}
}

constexpr static llvm::SourceMgr::DiagKind Error = llvm::SourceMgr::DK_Error;
constexpr static llvm::SourceMgr::DiagKind Warning =
llvm::SourceMgr::DK_Warning;
Expand Down
11 changes: 11 additions & 0 deletions clang-tools-extra/clangd/ConfigFragment.h
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,17 @@ struct Fragment {
std::vector<Located<std::string>> DisabledModifiers;
};
SemanticTokensBlock SemanticTokens;

/// Configures documentation style and behaviour.
struct DocumentationBlock {
/// Specifies the format of comments in the code.
/// Valid values are enum Config::CommentFormatPolicy values:
/// - Plaintext: Treat comments as plain text.
/// - Markdown: Treat comments as Markdown.
/// - Doxygen: Treat comments as doxygen.
std::optional<Located<std::string>> CommentFormat;
};
DocumentationBlock Documentation;
};

} // namespace config
Expand Down
10 changes: 10 additions & 0 deletions clang-tools-extra/clangd/ConfigYAML.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ class Parser {
Dict.handle("Hover", [&](Node &N) { parse(F.Hover, N); });
Dict.handle("InlayHints", [&](Node &N) { parse(F.InlayHints, N); });
Dict.handle("SemanticTokens", [&](Node &N) { parse(F.SemanticTokens, N); });
Dict.handle("Documentation", [&](Node &N) { parse(F.Documentation, N); });
Dict.parse(N);
return !(N.failed() || HadError);
}
Expand Down Expand Up @@ -299,6 +300,15 @@ class Parser {
Dict.parse(N);
}

void parse(Fragment::DocumentationBlock &F, Node &N) {
DictParser Dict("Documentation", this);
Dict.handle("CommentFormat", [&](Node &N) {
if (auto Value = scalarValue(N, "CommentFormat"))
F.CommentFormat = *Value;
});
Dict.parse(N);
}

// Helper for parsing mapping nodes (dictionaries).
// We don't use YamlIO as we want to control over unknown keys.
class DictParser {
Expand Down
119 changes: 49 additions & 70 deletions clang-tools-extra/clangd/Hover.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "Headers.h"
#include "IncludeCleaner.h"
#include "ParsedAST.h"
#include "Protocol.h"
#include "Selection.h"
#include "SourceCode.h"
#include "clang-include-cleaner/Analysis.h"
Expand Down Expand Up @@ -960,42 +961,6 @@ std::optional<HoverInfo> getHoverContents(const Attr *A, ParsedAST &AST) {
return HI;
}

bool isParagraphBreak(llvm::StringRef Rest) {
return Rest.ltrim(" \t").starts_with("\n");
}

bool punctuationIndicatesLineBreak(llvm::StringRef Line) {
constexpr llvm::StringLiteral Punctuation = R"txt(.:,;!?)txt";

Line = Line.rtrim();
return !Line.empty() && Punctuation.contains(Line.back());
}

bool isHardLineBreakIndicator(llvm::StringRef Rest) {
// '-'/'*' md list, '@'/'\' documentation command, '>' md blockquote,
// '#' headings, '`' code blocks
constexpr llvm::StringLiteral LinebreakIndicators = R"txt(-*@\>#`)txt";

Rest = Rest.ltrim(" \t");
if (Rest.empty())
return false;

if (LinebreakIndicators.contains(Rest.front()))
return true;

if (llvm::isDigit(Rest.front())) {
llvm::StringRef AfterDigit = Rest.drop_while(llvm::isDigit);
if (AfterDigit.starts_with(".") || AfterDigit.starts_with(")"))
return true;
}
return false;
}

bool isHardLineBreakAfter(llvm::StringRef Line, llvm::StringRef Rest) {
// Should we also consider whether Line is short?
return punctuationIndicatesLineBreak(Line) || isHardLineBreakIndicator(Rest);
}

void addLayoutInfo(const NamedDecl &ND, HoverInfo &HI) {
if (ND.isInvalidDecl())
return;
Expand Down Expand Up @@ -1571,6 +1536,26 @@ markup::Document HoverInfo::present() const {
return Output;
}

std::string HoverInfo::present(MarkupKind Kind) const {
if (Kind == MarkupKind::Markdown) {
const Config &Cfg = Config::current();
if ((Cfg.Documentation.CommentFormat ==
Config::CommentFormatPolicy::Markdown) ||
(Cfg.Documentation.CommentFormat ==
Config::CommentFormatPolicy::Doxygen))
// If the user prefers Markdown, we use the present() method to generate
// the Markdown output.
return present().asMarkdown();
if (Cfg.Documentation.CommentFormat ==
Config::CommentFormatPolicy::PlainText)
// If the user prefers plain text, we use the present() method to generate
// the plain text output.
return present().asEscapedMarkdown();
}

return present().asPlainText();
}

// If the backtick at `Offset` starts a probable quoted range, return the range
// (including the quotes).
std::optional<llvm::StringRef> getBacktickQuoteRange(llvm::StringRef Line,
Expand All @@ -1584,9 +1569,14 @@ std::optional<llvm::StringRef> getBacktickQuoteRange(llvm::StringRef Line,
return std::nullopt;

// The quoted string must be nonempty and usually has no leading/trailing ws.
auto Next = Line.find('`', Offset + 1);
auto Next = Line.find_first_of("`\n", Offset + 1);
if (Next == llvm::StringRef::npos)
return std::nullopt;

// There should be no newline in the quoted string.
if (Line[Next] == '\n')
return std::nullopt;

llvm::StringRef Contents = Line.slice(Offset + 1, Next);
if (Contents.empty() || isWhitespace(Contents.front()) ||
isWhitespace(Contents.back()))
Expand All @@ -1601,51 +1591,40 @@ std::optional<llvm::StringRef> getBacktickQuoteRange(llvm::StringRef Line,
return Line.slice(Offset, Next + 1);
}

void parseDocumentationLine(llvm::StringRef Line, markup::Paragraph &Out) {
void parseDocumentationParagraph(llvm::StringRef Text, markup::Paragraph &Out) {
// Probably this is appendText(Line), but scan for something interesting.
for (unsigned I = 0; I < Line.size(); ++I) {
switch (Line[I]) {
for (unsigned I = 0; I < Text.size(); ++I) {
switch (Text[I]) {
case '`':
if (auto Range = getBacktickQuoteRange(Line, I)) {
Out.appendText(Line.substr(0, I));
if (auto Range = getBacktickQuoteRange(Text, I)) {
Out.appendText(Text.substr(0, I));
Out.appendCode(Range->trim("`"), /*Preserve=*/true);
return parseDocumentationLine(Line.substr(I + Range->size()), Out);
return parseDocumentationParagraph(Text.substr(I + Range->size()), Out);
}
break;
}
}
Out.appendText(Line).appendSpace();
Out.appendText(Text);
}

void parseDocumentation(llvm::StringRef Input, markup::Document &Output) {
std::vector<llvm::StringRef> ParagraphLines;
auto FlushParagraph = [&] {
if (ParagraphLines.empty())
return;
auto &P = Output.addParagraph();
for (llvm::StringRef Line : ParagraphLines)
parseDocumentationLine(Line, P);
ParagraphLines.clear();
};

llvm::StringRef Line, Rest;
for (std::tie(Line, Rest) = Input.split('\n');
!(Line.empty() && Rest.empty());
std::tie(Line, Rest) = Rest.split('\n')) {

// After a linebreak remove spaces to avoid 4 space markdown code blocks.
// FIXME: make FlushParagraph handle this.
Line = Line.ltrim();
if (!Line.empty())
ParagraphLines.push_back(Line);

if (isParagraphBreak(Rest) || isHardLineBreakAfter(Line, Rest)) {
FlushParagraph();
}
// A documentation string is treated as a sequence of paragraphs,
// where the paragraphs are seperated by at least one empty line
// (meaning 2 consecutive newline characters).
// Possible leading empty lines (introduced by an odd number > 1 of
// empty lines between 2 paragraphs) will be removed later in the Markup
// renderer.
llvm::StringRef Paragraph, Rest;
for (std::tie(Paragraph, Rest) = Input.split("\n\n");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this always work as expected when there's an even number of line breaks between paragraphs? It's not obvious from the code.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it will still work.
In case of an even number of line breaks > 2 the if condition in the loop will evaluate to false and the additional empty lines will be ignored.
I added some comments about the logic and some new test cases to show it.

!(Paragraph.empty() && Rest.empty());
std::tie(Paragraph, Rest) = Rest.split("\n\n")) {

// The Paragraph will be empty if there is an even number of newline
// characters between two paragraphs, so we skip it.
if (!Paragraph.empty())
parseDocumentationParagraph(Paragraph, Output.addParagraph());
}
FlushParagraph();
}

llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,
const HoverInfo::PrintedType &T) {
OS << T.Type;
Expand Down
2 changes: 2 additions & 0 deletions clang-tools-extra/clangd/Hover.h
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,8 @@ struct HoverInfo {

/// Produce a user-readable information.
markup::Document present() const;

std::string present(MarkupKind Kind) const;
};

inline bool operator==(const HoverInfo::PrintedType &LHS,
Expand Down
Loading
Loading