Re: [PATCH] D71414: [clangd] Introduce codeblocks

[Yvan Roux via cfe-commits]

Thanks Kadir, bots are back to green :)

On Mon, 16 Dec 2019 at 09:18, Kadir Çetinkaya  wrote:
>
> sent out 0f959c87cc7867beb67bfab2d5e3cf90708b2f98
>
> On Mon, Dec 16, 2019 at 9:08 AM Yvan Roux  wrote:
>>
>> Hi, it is still broken on AArch64 bots, logs are available here:
>>
>> http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/21251/steps/ninja%20check%201/logs/stdio
>>
>> Thanks,
>> Yvan
>>
>> On Sat, 14 Dec 2019 at 16:15, Nico Weber via Phabricator via
>> cfe-commits  wrote:
>> >
>> > thakis added inline comments.
>> >
>> >
>> > 
>> > Comment at: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp:126
>> > +test
>> > +```
>> > +bar)md");
>> > 
>> > Older (but still supported) gccs can't handle multiline raw strings in 
>> > macro arguments, see e.g. 
>> > http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/21230/steps/ninja%20check%201/logs/stdio
>> >
>> > I fixed this for you in 687e98d294c4f77e. It's been broken for 3 days, 
>> > please watch bots and your inbox after committing.
>> >
>> >
>> > Repository:
>> >   rG LLVM Github Monorepo
>> >
>> > CHANGES SINCE LAST ACTION
>> >   https://reviews.llvm.org/D71414/new/
>> >
>> > https://reviews.llvm.org/D71414
>> >
>> >
>> >
>> > ___
>> > cfe-commits mailing list
>> > cfe-commits@lists.llvm.org
>> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D71414: [clangd] Introduce codeblocks

[Kadir Çetinkaya via cfe-commits]

sent out 0f959c87cc7867beb67bfab2d5e3cf90708b2f98

On Mon, Dec 16, 2019 at 9:08 AM Yvan Roux  wrote:

> Hi, it is still broken on AArch64 bots, logs are available here:
>
>
> http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/21251/steps/ninja%20check%201/logs/stdio
>
> Thanks,
> Yvan
>
> On Sat, 14 Dec 2019 at 16:15, Nico Weber via Phabricator via
> cfe-commits  wrote:
> >
> > thakis added inline comments.
> >
> >
> > 
> > Comment at:
> clang-tools-extra/clangd/unittests/FormattedStringTests.cpp:126
> > +test
> > +```
> > +bar)md");
> > 
> > Older (but still supported) gccs can't handle multiline raw strings in
> macro arguments, see e.g.
> http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/21230/steps/ninja%20check%201/logs/stdio
> >
> > I fixed this for you in 687e98d294c4f77e. It's been broken for 3 days,
> please watch bots and your inbox after committing.
> >
> >
> > Repository:
> >   rG LLVM Github Monorepo
> >
> > CHANGES SINCE LAST ACTION
> >   https://reviews.llvm.org/D71414/new/
> >
> > https://reviews.llvm.org/D71414
> >
> >
> >
> > ___
> > cfe-commits mailing list
> > cfe-commits@lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D71414: [clangd] Introduce codeblocks

[Kadir Çetinkaya via cfe-commits]

I was actually watching for buildbots(sent out
6b8ff5e43b405d255259196b6a53a3b5671aa5c7 for fixing some breakages for
example) but somehow this breakage mail didn't arrive. buildbots were in
bad shape on friday it might've skipped because of that :/

Thanks for fixing this.

On Sat, Dec 14, 2019 at 4:15 PM Nico Weber via Phabricator <
revi...@reviews.llvm.org> wrote:

> thakis added inline comments.
>
>
> 
> Comment at: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp:126
> +test
> +```
> +bar)md");
> 
> Older (but still supported) gccs can't handle multiline raw strings in
> macro arguments, see e.g.
> http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/21230/steps/ninja%20check%201/logs/stdio
>
> I fixed this for you in 687e98d294c4f77e. It's been broken for 3 days,
> please watch bots and your inbox after committing.
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D71414/new/
>
> https://reviews.llvm.org/D71414
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D71414: [clangd] Introduce codeblocks

[Yvan Roux via cfe-commits]

Hi, it is still broken on AArch64 bots, logs are available here:

http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/21251/steps/ninja%20check%201/logs/stdio

Thanks,
Yvan

On Sat, 14 Dec 2019 at 16:15, Nico Weber via Phabricator via
cfe-commits  wrote:
>
> thakis added inline comments.
>
>
> 
> Comment at: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp:126
> +test
> +```
> +bar)md");
> 
> Older (but still supported) gccs can't handle multiline raw strings in macro 
> arguments, see e.g. 
> http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/21230/steps/ninja%20check%201/logs/stdio
>
> I fixed this for you in 687e98d294c4f77e. It's been broken for 3 days, please 
> watch bots and your inbox after committing.
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D71414/new/
>
> https://reviews.llvm.org/D71414
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71414: [clangd] Introduce codeblocks

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp:126
+test
+```
+bar)md");

Older (but still supported) gccs can't handle multiline raw strings in macro 
arguments, see e.g. 
http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/21230/steps/ninja%20check%201/logs/stdio

I fixed this for you in 687e98d294c4f77e. It's been broken for 3 days, please 
watch bots and your inbox after committing.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71414/new/

https://reviews.llvm.org/D71414



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71414: [clangd] Introduce codeblocks

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7c13fe8a6a64: [clangd] Introduce codeblocks (authored by 
kadircet).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71414/new/

https://reviews.llvm.org/D71414

Files:
  clang-tools-extra/clangd/FormattedString.cpp
  clang-tools-extra/clangd/FormattedString.h
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/FormattedStringTests.cpp

Index: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
===
--- clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
+++ clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
@@ -54,6 +54,28 @@
   P = Paragraph();
   P.appendCode("`foo`");
   EXPECT_EQ(P.asMarkdown(), "` ``foo`` `");
+
+  // Code blocks might need more than 3 backticks.
+  Document D;
+  D.addCodeBlock("foobarbaz `\nqux");
+  EXPECT_EQ(D.asMarkdown(), "```cpp\n"
+"foobarbaz `\nqux\n"
+"```");
+  D = Document();
+  D.addCodeBlock("foobarbaz ``\nqux");
+  EXPECT_THAT(D.asMarkdown(), "```cpp\n"
+  "foobarbaz ``\nqux\n"
+  "```");
+  D = Document();
+  D.addCodeBlock("foobarbaz ```\nqux");
+  EXPECT_EQ(D.asMarkdown(), "cpp\n"
+"foobarbaz ```\nqux\n"
+"");
+  D = Document();
+  D.addCodeBlock("foobarbaz ` `` ```  `\nqux");
+  EXPECT_EQ(D.asMarkdown(), "`cpp\n"
+"foobarbaz ` `` ```  `\nqux\n"
+"`");
 }
 
 TEST(Paragraph, SeparationOfChunks) {
@@ -96,9 +118,18 @@
 TEST(Document, Separators) {
   Document D;
   D.addParagraph().appendText("foo");
+  D.addCodeBlock("test");
   D.addParagraph().appendText("bar");
-  EXPECT_EQ(D.asMarkdown(), "foo\nbar");
-  EXPECT_EQ(D.asPlainText(), "foo\nbar");
+  EXPECT_EQ(D.asMarkdown(), R"md(foo
+```cpp
+test
+```
+bar)md");
+  EXPECT_EQ(D.asPlainText(), R"pt(foo
+
+test
+
+bar)pt");
 }
 
 TEST(Document, Spacer) {
@@ -110,6 +141,38 @@
   EXPECT_EQ(D.asPlainText(), "foo\n\nbar");
 }
 
+TEST(CodeBlock, Render) {
+  Document D;
+  // Code blocks preserves any extra spaces.
+  D.addCodeBlock("foo\n  bar\n  baz");
+  EXPECT_EQ(D.asMarkdown(), R"md(```cpp
+foo
+  bar
+  baz
+```)md");
+  EXPECT_EQ(D.asPlainText(), R"pt(foo
+  bar
+  baz)pt");
+  D.addCodeBlock("foo");
+  EXPECT_EQ(D.asMarkdown(), R"md(```cpp
+foo
+  bar
+  baz
+```
+```cpp
+foo
+```)md");
+  // FIXME: we shouldn't have 2 empty lines in between. A solution might be
+  // having a `verticalMargin` method for blocks, and let container insert new
+  // lines according to that before/after blocks.
+  EXPECT_EQ(D.asPlainText(), R"pt(foo
+  bar
+  baz
+
+
+foo)pt");
+}
+
 } // namespace
 } // namespace markup
 } // namespace clangd
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -95,7 +95,7 @@
 }
 
 void printParams(llvm::raw_ostream ,
-const std::vector ) {
+ const std::vector ) {
   for (size_t I = 0, E = Params.size(); I != E; ++I) {
 if (I)
   OS << ", ";
@@ -456,12 +456,11 @@
   P.appendCode(llvm::StringRef(*NamespaceScope).drop_back(2));
   }
 
-  Output.addSpacer();
   if (!Definition.empty()) {
-Output.addParagraph().appendCode(Definition);
+Output.addCodeBlock(Definition);
   } else {
 // Builtin types
-Output.addParagraph().appendCode(Name);
+Output.addCodeBlock(Name);
   }
 
   if (!Documentation.empty())
Index: clang-tools-extra/clangd/FormattedString.h
===
--- clang-tools-extra/clangd/FormattedString.h
+++ clang-tools-extra/clangd/FormattedString.h
@@ -70,6 +70,9 @@
   Paragraph ();
   /// Inserts a vertical space into the document.
   void addSpacer();
+  /// Adds a block of code. This translates to a ``` block in markdown. In plain
+  /// text representation, the code block will be surrounded by newlines.
+  void addCodeBlock(std::string Code, std::string Language = "cpp");
 
   std::string asMarkdown() const;
   std::string asPlainText() const;
Index: clang-tools-extra/clangd/FormattedString.cpp
===
--- clang-tools-extra/clangd/FormattedString.cpp
+++ clang-tools-extra/clangd/FormattedString.cpp
@@ -72,10 +72,10 @@
   return "`" + std::move(R) + "`";
 }
 
-/// Render \p Input as markdown code block with a specified \p Language. The
-/// result is surrounded by >= 3 backticks. Although markdown also allows to use
-/// '~' for code blocks, they are never used.
-std::string renderCodeBlock(llvm::StringRef Input, llvm::StringRef Language) {
+/// Get marker required for \p Input to 

[PATCH] D71414: [clangd] Introduce codeblocks

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 60768 tests passed, 0 failed 
and 726 were skipped.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
console-log.txt 
,
 CMakeCache.txt 
,
 test-results.xml 
,
 diff.json 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71414/new/

https://reviews.llvm.org/D71414



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71414: [clangd] Introduce codeblocks

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/FormattedString.cpp:138
+// No need to pad from previous blocks, as they should end with a new line.
+OS << renderCodeBlock(Contents, Language) << '\n';
+  }

sammccall wrote:
> can you inline renderCodeBlock here, or modify it to just compute the marker?
> It doesn't make sense to have this function just to delegate to one with a 
> different interface (and a copy)
changed it to only return `Marker`



Comment at: clang-tools-extra/clangd/FormattedString.cpp:142
+  void renderPlainText(llvm::raw_ostream ) const override {
+// In plaintext we want one empty line before and after codeblocks.
+OS << '\n' << Contents << "\n\n";

sammccall wrote:
> This means if we have two consecutive code blocks, they'll be separated by 
> two blank lines...
> 
> worth at least a fixme.
> 
> (I guess the "right thing" here is to have `virtual unsigned verticalMargin() 
> { return 0; }`, and have the document renderer insert 
> max(first.verticalMargin(),second.verticalMargin()) newlines in between). 
> Probably not worth it
adding a testcase and a fixme


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71414/new/

https://reviews.llvm.org/D71414



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71414: [clangd] Introduce codeblocks

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 233606.
kadircet marked 4 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71414/new/

https://reviews.llvm.org/D71414

Files:
  clang-tools-extra/clangd/FormattedString.cpp
  clang-tools-extra/clangd/FormattedString.h
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/FormattedStringTests.cpp

Index: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
===
--- clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
+++ clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
@@ -54,6 +54,28 @@
   P = Paragraph();
   P.appendCode("`foo`");
   EXPECT_EQ(P.asMarkdown(), "` ``foo`` `");
+
+  // Code blocks might need more than 3 backticks.
+  Document D;
+  D.addCodeBlock("foobarbaz `\nqux");
+  EXPECT_EQ(D.asMarkdown(), "```cpp\n"
+"foobarbaz `\nqux\n"
+"```");
+  D = Document();
+  D.addCodeBlock("foobarbaz ``\nqux");
+  EXPECT_THAT(D.asMarkdown(), "```cpp\n"
+  "foobarbaz ``\nqux\n"
+  "```");
+  D = Document();
+  D.addCodeBlock("foobarbaz ```\nqux");
+  EXPECT_EQ(D.asMarkdown(), "cpp\n"
+"foobarbaz ```\nqux\n"
+"");
+  D = Document();
+  D.addCodeBlock("foobarbaz ` `` ```  `\nqux");
+  EXPECT_EQ(D.asMarkdown(), "`cpp\n"
+"foobarbaz ` `` ```  `\nqux\n"
+"`");
 }
 
 TEST(Paragraph, SeparationOfChunks) {
@@ -96,9 +118,18 @@
 TEST(Document, Separators) {
   Document D;
   D.addParagraph().appendText("foo");
+  D.addCodeBlock("test");
   D.addParagraph().appendText("bar");
-  EXPECT_EQ(D.asMarkdown(), "foo\nbar");
-  EXPECT_EQ(D.asPlainText(), "foo\nbar");
+  EXPECT_EQ(D.asMarkdown(), R"md(foo
+```cpp
+test
+```
+bar)md");
+  EXPECT_EQ(D.asPlainText(), R"pt(foo
+
+test
+
+bar)pt");
 }
 
 TEST(Document, Spacer) {
@@ -110,6 +141,38 @@
   EXPECT_EQ(D.asPlainText(), "foo\n\nbar");
 }
 
+TEST(CodeBlock, Render) {
+  Document D;
+  // Code blocks preserves any extra spaces.
+  D.addCodeBlock("foo\n  bar\n  baz");
+  EXPECT_EQ(D.asMarkdown(), R"md(```cpp
+foo
+  bar
+  baz
+```)md");
+  EXPECT_EQ(D.asPlainText(), R"pt(foo
+  bar
+  baz)pt");
+  D.addCodeBlock("foo");
+  EXPECT_EQ(D.asMarkdown(), R"md(```cpp
+foo
+  bar
+  baz
+```
+```cpp
+foo
+```)md");
+  // FIXME: we shouldn't have 2 empty lines in between. A solution might be
+  // having a `verticalMargin` method for blocks, and let container insert new
+  // lines according to that before/after blocks.
+  EXPECT_EQ(D.asPlainText(), R"pt(foo
+  bar
+  baz
+
+
+foo)pt");
+}
+
 } // namespace
 } // namespace markup
 } // namespace clangd
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -95,7 +95,7 @@
 }
 
 void printParams(llvm::raw_ostream ,
-const std::vector ) {
+ const std::vector ) {
   for (size_t I = 0, E = Params.size(); I != E; ++I) {
 if (I)
   OS << ", ";
@@ -456,12 +456,11 @@
   P.appendCode(llvm::StringRef(*NamespaceScope).drop_back(2));
   }
 
-  Output.addSpacer();
   if (!Definition.empty()) {
-Output.addParagraph().appendCode(Definition);
+Output.addCodeBlock(Definition);
   } else {
 // Builtin types
-Output.addParagraph().appendCode(Name);
+Output.addCodeBlock(Name);
   }
 
   if (!Documentation.empty())
Index: clang-tools-extra/clangd/FormattedString.h
===
--- clang-tools-extra/clangd/FormattedString.h
+++ clang-tools-extra/clangd/FormattedString.h
@@ -70,6 +70,9 @@
   Paragraph ();
   /// Inserts a vertical space into the document.
   void addSpacer();
+  /// Adds a block of code. This translates to a ``` block in markdown. In plain
+  /// text representation, the code block will be surrounded by newlines.
+  void addCodeBlock(std::string Code, std::string Language = "cpp");
 
   std::string asMarkdown() const;
   std::string asPlainText() const;
Index: clang-tools-extra/clangd/FormattedString.cpp
===
--- clang-tools-extra/clangd/FormattedString.cpp
+++ clang-tools-extra/clangd/FormattedString.cpp
@@ -72,10 +72,10 @@
   return "`" + std::move(R) + "`";
 }
 
-/// Render \p Input as markdown code block with a specified \p Language. The
-/// result is surrounded by >= 3 backticks. Although markdown also allows to use
-/// '~' for code blocks, they are never used.
-std::string renderCodeBlock(llvm::StringRef Input, llvm::StringRef Language) {
+/// Get marker required for \p Input to represent a markdown codeblock. 

[PATCH] D71414: [clangd] Introduce codeblocks

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Otherwise LG


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71414/new/

https://reviews.llvm.org/D71414



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71414: [clangd] Introduce codeblocks

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/FormattedString.cpp:138
+// No need to pad from previous blocks, as they should end with a new line.
+OS << renderCodeBlock(Contents, Language) << '\n';
+  }

can you inline renderCodeBlock here, or modify it to just compute the marker?
It doesn't make sense to have this function just to delegate to one with a 
different interface (and a copy)



Comment at: clang-tools-extra/clangd/FormattedString.cpp:142
+  void renderPlainText(llvm::raw_ostream ) const override {
+// In plaintext we want one empty line before and after codeblocks.
+OS << '\n' << Contents << "\n\n";

This means if we have two consecutive code blocks, they'll be separated by two 
blank lines...

worth at least a fixme.

(I guess the "right thing" here is to have `virtual unsigned verticalMargin() { 
return 0; }`, and have the document renderer insert 
max(first.verticalMargin(),second.verticalMargin()) newlines in between). 
Probably not worth it


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71414/new/

https://reviews.llvm.org/D71414



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71414: [clangd] Introduce codeblocks

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
console-log.txt 
,
 diff.json 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71414/new/

https://reviews.llvm.org/D71414



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71414: [clangd] Introduce codeblocks

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

Follow-up to the patch D71248 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71414

Files:
  clang-tools-extra/clangd/FormattedString.cpp
  clang-tools-extra/clangd/FormattedString.h
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/FormattedStringTests.cpp

Index: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
===
--- clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
+++ clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
@@ -54,6 +54,28 @@
   P = Paragraph();
   P.appendCode("`foo`");
   EXPECT_EQ(P.asMarkdown(), "` ``foo`` `");
+
+  // Code blocks might need more than 3 backticks.
+  Document D;
+  D.addCodeBlock("foobarbaz `\nqux");
+  EXPECT_EQ(D.asMarkdown(), "```cpp\n"
+"foobarbaz `\nqux\n"
+"```");
+  D = Document();
+  D.addCodeBlock("foobarbaz ``\nqux");
+  EXPECT_THAT(D.asMarkdown(), "```cpp\n"
+  "foobarbaz ``\nqux\n"
+  "```");
+  D = Document();
+  D.addCodeBlock("foobarbaz ```\nqux");
+  EXPECT_EQ(D.asMarkdown(), "cpp\n"
+"foobarbaz ```\nqux\n"
+"");
+  D = Document();
+  D.addCodeBlock("foobarbaz ` `` ```  `\nqux");
+  EXPECT_EQ(D.asMarkdown(), "`cpp\n"
+"foobarbaz ` `` ```  `\nqux\n"
+"`");
 }
 
 TEST(Paragraph, SeparationOfChunks) {
@@ -96,9 +118,18 @@
 TEST(Document, Separators) {
   Document D;
   D.addParagraph().appendText("foo");
+  D.addCodeBlock("test");
   D.addParagraph().appendText("bar");
-  EXPECT_EQ(D.asMarkdown(), "foo\nbar");
-  EXPECT_EQ(D.asPlainText(), "foo\nbar");
+  EXPECT_EQ(D.asMarkdown(), R"md(foo
+```cpp
+test
+```
+bar)md");
+  EXPECT_EQ(D.asPlainText(), R"pt(foo
+
+test
+
+bar)pt");
 }
 
 TEST(Document, Spacer) {
@@ -110,6 +141,20 @@
   EXPECT_EQ(D.asPlainText(), "foo\n\nbar");
 }
 
+TEST(CodeBlock, Render) {
+  Document D;
+  // Code blocks preserves any extra spaces.
+  D.addCodeBlock("foo\n  bar\n  baz");
+  EXPECT_EQ(D.asMarkdown(), R"md(```cpp
+foo
+  bar
+  baz
+```)md");
+  EXPECT_EQ(D.asPlainText(), R"pt(foo
+  bar
+  baz)pt");
+}
+
 } // namespace
 } // namespace markup
 } // namespace clangd
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -95,7 +95,7 @@
 }
 
 void printParams(llvm::raw_ostream ,
-const std::vector ) {
+ const std::vector ) {
   for (size_t I = 0, E = Params.size(); I != E; ++I) {
 if (I)
   OS << ", ";
@@ -456,12 +456,11 @@
   P.appendCode(llvm::StringRef(*NamespaceScope).drop_back(2));
   }
 
-  Output.addSpacer();
   if (!Definition.empty()) {
-Output.addParagraph().appendCode(Definition);
+Output.addCodeBlock(Definition);
   } else {
 // Builtin types
-Output.addParagraph().appendCode(Name);
+Output.addCodeBlock(Name);
   }
 
   if (!Documentation.empty())
Index: clang-tools-extra/clangd/FormattedString.h
===
--- clang-tools-extra/clangd/FormattedString.h
+++ clang-tools-extra/clangd/FormattedString.h
@@ -70,6 +70,9 @@
   Paragraph ();
   /// Inserts a vertical space into the document.
   void addSpacer();
+  /// Adds a block of code. This translates to a ``` block in markdown. In plain
+  /// text representation, the code block will be surrounded by newlines.
+  void addCodeBlock(std::string Code, std::string Language = "cpp");
 
   std::string asMarkdown() const;
   std::string asPlainText() const;
Index: clang-tools-extra/clangd/FormattedString.cpp
===
--- clang-tools-extra/clangd/FormattedString.cpp
+++ clang-tools-extra/clangd/FormattedString.cpp
@@ -131,6 +131,25 @@
   void renderPlainText(llvm::raw_ostream ) const override { OS << '\n'; }
 };
 
+class CodeBlock : public Block {
+public:
+  void renderMarkdown(llvm::raw_ostream ) const override {
+// No need to pad from previous blocks, as they should end with a new line.
+OS << renderCodeBlock(Contents, Language) << '\n';
+  }
+
+  void renderPlainText(llvm::raw_ostream ) const override {
+// In plaintext we want one empty line before and after codeblocks.
+OS << '\n' << Contents << "\n\n";
+  }
+
+  CodeBlock(std::string Contents, std::string Language)
+  : Contents(std::move(Contents)), Language(std::move(Language)) {}
+
+private:
+  std::string Contents;
+  std::string Language;
+};
 } // namespace