[PATCH] D65003: [clang-doc] Add index in each info html file

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran abandoned this revision.
DiegoAstiazaran added a comment.

D65690  replaces this revision.


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

https://reviews.llvm.org/D65003



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

[PATCH] D65003: [clang-doc] Add index in each info html file

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

I think everything but the implementation of `genIndex` being confusing looks 
good to me. I haven't actually groked how that code works yet other than the 
fact that it generates the index tree that I expect and all the surrounding 
code looks good to me. I understand that some of the trickiness here comes from 
the fact that you're building from a list of values but trying to generate the 
tree structure from that list which is hard. I think we can structure that code 
better; lets see if we can't device a better algorithm.




Comment at: clang-tools-extra/clang-doc/Generators.cpp:16
 
+Index Generator::genIndex(const std::vector> &Infos) {
+  Index Idx;

Please document this function with more internal comments. I have no idea 
what's going on here and the shifting of 'I' is super confusing to me.



Comment at: clang-tools-extra/clang-doc/Generators.cpp:20
+Index *I = &Idx;
+for (auto R = Info->Namespace.rbegin(), E = Info->Namespace.rend(); R != E;
+ ++R) {

Use llvm::reverse https://llvm.org/doxygen/STLExtras_8h.html


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

https://reviews.llvm.org/D65003



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

[PATCH] D65003: [clang-doc] Add index in each info html file

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran added inline comments.



Comment at: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp:337-346
+
+  A
+  B
+  C
+
+  D
+  E

juliehockett wrote:
> The indentation here seems a bit off
Fixed by D65005.


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

https://reviews.llvm.org/D65003



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

[PATCH] D65003: [clang-doc] Add index in each info html file

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran updated this revision to Diff 211866.
DiegoAstiazaran marked 3 inline comments as done.
DiegoAstiazaran added a comment.

Rebase and add comments.


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

https://reviews.llvm.org/D65003

Files:
  clang-tools-extra/clang-doc/Generators.cpp
  clang-tools-extra/clang-doc/Generators.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/unittests/clang-doc/CMakeLists.txt
  clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
  clang-tools-extra/unittests/clang-doc/ClangDocTest.h
  clang-tools-extra/unittests/clang-doc/GeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
@@ -9,6 +9,7 @@
 #include "ClangDocTest.h"
 #include "Generators.h"
 #include "Representation.h"
+#include "Serialize.h"
 #include "gtest/gtest.h"
 
 namespace clang {
@@ -315,5 +316,79 @@
   EXPECT_EQ(Expected, Actual.str());
 }
 
+TEST(HTMLGeneratorTest, emitIndexHTML) {
+  RecordInfo I;
+  I.Path = "";
+  ClangDocContext CDCtx;
+  std::vector> Infos;
+  Infos.emplace_back(llvm::make_unique());
+  Info *InfoA = Infos.back().get();
+  InfoA->Name = "A";
+  InfoA->USR = serialize::hashUSR("1");
+  Infos.emplace_back(llvm::make_unique());
+  Info *InfoC = Infos.back().get();
+  InfoC->Name = "C";
+  InfoC->USR = serialize::hashUSR("3");
+  Reference RefB = Reference("B");
+  RefB.USR = serialize::hashUSR("2");
+  InfoC->Namespace = {std::move(RefB)};
+  Infos.emplace_back(llvm::make_unique());
+  Info *InfoD = Infos.back().get();
+  InfoD->Name = "D";
+  InfoD->USR = serialize::hashUSR("4");
+  Infos.emplace_back(llvm::make_unique());
+  Info *InfoF = Infos.back().get();
+  InfoF->Name = "F";
+  InfoF->USR = serialize::hashUSR("6");
+  Reference RefD = Reference("D");
+  RefD.USR = serialize::hashUSR("4");
+  Reference RefE = Reference("E");
+  RefE.USR = serialize::hashUSR("5");
+  InfoF->Namespace = {std::move(RefE), std::move(RefD)};
+  CDCtx.Idx = Generator::genIndex(Infos);
+
+  auto G = getHTMLGenerator();
+  assert(G);
+  std::string Buffer;
+  llvm::raw_string_ostream Actual(Buffer);
+  auto Err = G->generateDocForInfo(&I, Actual, CDCtx);
+  assert(!Err);
+  std::string Expected = R"raw(
+
+struct 
+
+  
+A
+  
+  
+B
+
+  
+C
+  
+
+  
+  
+D
+
+  
+E
+
+  
+F
+  
+
+  
+
+  
+
+
+  struct 
+
+)raw";
+
+  EXPECT_EQ(Expected, Actual.str());
+}
+
 } // namespace doc
 } // namespace clang
Index: clang-tools-extra/unittests/clang-doc/GeneratorTest.cpp
===
--- /dev/null
+++ clang-tools-extra/unittests/clang-doc/GeneratorTest.cpp
@@ -0,0 +1,70 @@
+//===-- clang-doc/GeneratorTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ClangDocTest.h"
+#include "Generators.h"
+#include "Representation.h"
+#include "Serialize.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace doc {
+
+TEST(GeneratorTest, emitIndex) {
+  std::vector> Infos;
+  Infos.emplace_back(llvm::make_unique());
+  Info *InfoA = Infos.back().get();
+  InfoA->Name = "A";
+  InfoA->USR = serialize::hashUSR("1");
+  Infos.emplace_back(llvm::make_unique());
+  Info *InfoC = Infos.back().get();
+  InfoC->Name = "C";
+  InfoC->USR = serialize::hashUSR("3");
+  Reference RefB = Reference("B");
+  RefB.USR = serialize::hashUSR("2");
+  InfoC->Namespace = {std::move(RefB)};
+  Infos.emplace_back(llvm::make_unique());
+  Info *InfoD = Infos.back().get();
+  InfoD->Name = "D";
+  InfoD->USR = serialize::hashUSR("4");
+  Infos.emplace_back(llvm::make_unique());
+  Info *InfoF = Infos.back().get();
+  InfoF->Name = "F";
+  InfoF->USR = serialize::hashUSR("6");
+  Reference RefD = Reference("D");
+  RefD.USR = serialize::hashUSR("4");
+  Reference RefE = Reference("E");
+  RefE.USR = serialize::hashUSR("5");
+  InfoF->Namespace = {std::move(RefE), std::move(RefD)};
+  Index Idx = Generator::genIndex(Infos);
+
+  Index ExpectedIdx;
+  Index IndexA;
+  IndexA.Name = "A";
+  ExpectedIdx.Children.emplace_back(std::move(IndexA));
+  Index IndexB;
+  IndexB.Name = "B";
+  Index IndexC;
+  IndexC.Name = "C";
+  IndexB.Children.emplace_back(std::move(IndexC));
+  ExpectedIdx.Children.emplace_back(std::mov

[PATCH] D65003: [clang-doc] Add index in each info html file

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/clang-doc/Generators.h:28
 
+  static Index genIndex(const std::vector> &Infos);
+

Add a comment here indicating that this should be created before calling any 
`generateDocForInfo`s. Also add a fix-it note to the following effect:

`FIXME: This currently needs to be run before generating any individual 
documentation pages, since the content it generates is directly included in 
every page. A better design would be to lazily include it in the individual 
documentation pages, in which case this could be run in parallel with calls to 
generateDocForInfo().`



Comment at: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp:337-346
+
+  A
+  B
+  C
+
+  D
+  E

The indentation here seems a bit off


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

https://reviews.llvm.org/D65003



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

[PATCH] D65003: [clang-doc] Add index in each info html file

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran created this revision.
DiegoAstiazaran added reviewers: juliehockett, jakehehrlich, lebedev.ri.
DiegoAstiazaran added a project: clang-tools-extra.
Herald added subscribers: kadircet, arphaman, mgrang, mgorny.

An index structure is created while reducing the infos. This is then passed to 
the HTML generator so it's included in each file created.
Paths in the index are fixed (relative to the current file) for each file.
Index is currently rendered on top of the info content, this will be fixed 
later with CSS.


https://reviews.llvm.org/D65003

Files:
  clang-tools-extra/clang-doc/Generators.cpp
  clang-tools-extra/clang-doc/Generators.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/unittests/clang-doc/CMakeLists.txt
  clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
  clang-tools-extra/unittests/clang-doc/ClangDocTest.h
  clang-tools-extra/unittests/clang-doc/GeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -40,7 +40,7 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  auto Err = G->generateDocForInfo(&I, Actual);
+  auto Err = G->generateDocForInfo(&I, Actual, ClangDocContext());
   assert(!Err);
   std::string Expected =
   R"raw(---
@@ -94,7 +94,7 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  auto Err = G->generateDocForInfo(&I, Actual);
+  auto Err = G->generateDocForInfo(&I, Actual, ClangDocContext());
   assert(!Err);
   std::string Expected =
   R"raw(---
@@ -158,7 +158,7 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  auto Err = G->generateDocForInfo(&I, Actual);
+  auto Err = G->generateDocForInfo(&I, Actual, ClangDocContext());
   assert(!Err);
   std::string Expected =
   R"raw(---
@@ -206,7 +206,7 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  auto Err = G->generateDocForInfo(&I, Actual);
+  auto Err = G->generateDocForInfo(&I, Actual, ClangDocContext());
   assert(!Err);
   std::string Expected =
   R"raw(---
@@ -343,7 +343,7 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  auto Err = G->generateDocForInfo(&I, Actual);
+  auto Err = G->generateDocForInfo(&I, Actual, ClangDocContext());
   assert(!Err);
   std::string Expected =
   R"raw(---
Index: clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp
@@ -38,7 +38,7 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  auto Err = G->generateDocForInfo(&I, Actual);
+  auto Err = G->generateDocForInfo(&I, Actual, ClangDocContext());
   assert(!Err);
   std::string Expected = R"raw(# namespace Namespace
 
@@ -101,7 +101,7 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  auto Err = G->generateDocForInfo(&I, Actual);
+  auto Err = G->generateDocForInfo(&I, Actual, ClangDocContext());
   assert(!Err);
   std::string Expected = R"raw(# class r
 
@@ -162,7 +162,7 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  auto Err = G->generateDocForInfo(&I, Actual);
+  auto Err = G->generateDocForInfo(&I, Actual, ClangDocContext());
   assert(!Err);
   std::string Expected = R"raw(### f
 
@@ -190,7 +190,7 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  auto Err = G->generateDocForInfo(&I, Actual);
+  auto Err = G->generateDocForInfo(&I, Actual, ClangDocContext());
   assert(!Err);
   std::string Expected = R"raw(| enum class e |
 
@@ -320,7 +320,7 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  auto Err = G->generateDocForInfo(&I, Actual);
+  auto Err = G->generateDocForInfo(&I, Actual, ClangDocContext());
   assert(!Err);
   std::string Expected =
   R"raw(### f
Index: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
@@ -9,6 +9,7 @@
 #include "ClangDocTest.h"
 #include "Gener