[PATCH] D65483: [clang-doc] Add link to source code in file definitions
This revision was automatically updated to reflect the committed changes.
Closed by commit rL368460: [clang-format] Add link to source code in file
definitions (authored by DiegoAstiazaran, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D65483?vs=214004&id=214404#toc
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65483/new/
https://reviews.llvm.org/D65483
Files:
clang-tools-extra/trunk/clang-doc/BitcodeReader.cpp
clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp
clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
clang-tools-extra/trunk/clang-doc/Mapper.cpp
clang-tools-extra/trunk/clang-doc/Mapper.h
clang-tools-extra/trunk/clang-doc/Representation.cpp
clang-tools-extra/trunk/clang-doc/Representation.h
clang-tools-extra/trunk/clang-doc/Serialize.cpp
clang-tools-extra/trunk/clang-doc/Serialize.h
clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp
clang-tools-extra/trunk/docs/clang-doc.rst
clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp
clang-tools-extra/trunk/unittests/clang-doc/SerializeTest.cpp
Index: clang-tools-extra/trunk/docs/clang-doc.rst
===
--- clang-tools-extra/trunk/docs/clang-doc.rst
+++ clang-tools-extra/trunk/docs/clang-doc.rst
@@ -66,7 +66,7 @@
.. code-block:: console
- $ clang-doc --help
+ $ clang-doc --help
USAGE: clang-doc [options] [... ]
OPTIONS:
@@ -79,17 +79,27 @@
clang-doc options:
--doxygen - Use only doxygen-style comments to generate docs.
--dump - Dump intermediate results to bitcode file.
--extra-arg=- Additional argument to append to the compiler command line
--extra-arg-before= - Additional argument to prepend to the compiler command line
---format= - Format for outputted docs.
- =yaml- Documentation in YAML format.
- =md - Documentation in MD format.
- =html- Documentation in HTML format.
--output= - Directory for outputting generated files.
--p=- Build path
---public - Document only public declarations.
---stylesheets= - CSS stylesheets to extend the default styles.
-
-``stylesheets`` should only be used if ``format`` is set to ``html``.
+--doxygen - Use only doxygen-style comments to generate docs.
+--extra-arg=- Additional argument to append to the compiler command line
+--extra-arg-before= - Additional argument to prepend to the compiler command line
+--format=- Format for outputted docs.
+ =yaml - Documentation in YAML format.
+ =md - Documentation in MD format.
+ =html - Documentation in HTML format.
+--ignore-map-errors - Continue if files are not mapped correctly.
+--output= - Directory for outputting generated files.
+-p= - Build path
+--public- Document only public declarations.
+--repository= -
+ URL of repository that hosts code.
+ Used for links to definition locations.
+--source-root= -
+ Directory where processed files are stored.
+ Links to definition locations will only be
+ generated if the file is in this dir.
+--stylesheets= - CSS stylesheets to extend the default styles.
+
+The following flags shoud only be used if ``format`` is set to ``html``:
+- ``repository``
+- ``source-root``
+- ``stylesheets``
Index: clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp
===
--- clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp
+++ clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp
@@ -77,10 +77,13 @@
{// 0. Fixed-size integer (line number)
llvm::BitCodeAbbrevOp(llvm::BitCodeAbbrevOp::Fixed,
BitCodeConstants::LineNumberSize),
- // 1. Fixed-size integer (length of the following string (filename))
+ // 1. Boolean (IsFileInRootDir)
+ llvm::BitCodeAbbrevOp(llvm::BitCodeAbbrevOp::Fixed,
+ BitCodeConstants::BoolSize),
+ // 2. Fixed-size integer (length of the following string (filename))
llvm::BitCodeAbbrevOp(llvm::BitCodeAbbrevOp::Fixed,
BitCodeConstants::StringLengthSize),
- // 2. The string blob
+ // 3. The string blob
llvm::BitCodeAbbrevOp(llvm::BitCodeAbbrevOp::Blob)});
}
@@ -316,6 +319,7 @@
// FIXME: Assert that the line number is of the appropriate size.
Record.push_back(L
[PATCH] D65483: [clang-doc] Add link to source code in file definitions
juliehockett accepted this revision. juliehockett added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65483/new/ https://reviews.llvm.org/D65483 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D65483: [clang-doc] Add link to source code in file definitions
DiegoAstiazaran updated this revision to Diff 214004.
DiegoAstiazaran added a comment.
Rebase to master
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65483/new/
https://reviews.llvm.org/D65483
Files:
clang-tools-extra/clang-doc/BitcodeReader.cpp
clang-tools-extra/clang-doc/BitcodeWriter.cpp
clang-tools-extra/clang-doc/HTMLGenerator.cpp
clang-tools-extra/clang-doc/Mapper.cpp
clang-tools-extra/clang-doc/Mapper.h
clang-tools-extra/clang-doc/Representation.cpp
clang-tools-extra/clang-doc/Representation.h
clang-tools-extra/clang-doc/Serialize.cpp
clang-tools-extra/clang-doc/Serialize.h
clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
clang-tools-extra/docs/clang-doc.rst
clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -37,7 +37,7 @@
template bool mapDecl(const T *D) {
auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
+ /*File=*/"test.cpp", true, Public);
if (I.first)
EmittedInfos.emplace_back(std::move(I.first));
if (I.second)
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
@@ -23,9 +23,9 @@
}
ClangDocContext
-getClangDocContext(std::vector UserStylesheets = {}) {
- ClangDocContext CDCtx;
- CDCtx.UserStylesheets = {UserStylesheets.begin(), UserStylesheets.end()};
+getClangDocContext(std::vector UserStylesheets = {},
+ StringRef RepositoryUrl = "") {
+ ClangDocContext CDCtx{{}, {}, {}, {}, RepositoryUrl, UserStylesheets, {}};
CDCtx.UserStylesheets.insert(
CDCtx.UserStylesheets.begin(),
"../share/clang/clang-doc-default-stylesheet.css");
@@ -127,7 +127,7 @@
I.Path = "X/Y/Z";
I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
- I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+ I.DefLoc = Location(10, llvm::SmallString<16>{"dir/test.cpp"}, true);
I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
SmallString<16> PathTo;
@@ -147,7 +147,7 @@
assert(G);
std::string Buffer;
llvm::raw_string_ostream Actual(Buffer);
- ClangDocContext CDCtx = getClangDocContext();
+ ClangDocContext CDCtx = getClangDocContext({}, "http://www.repository.com";);
auto Err = G->generateDocForInfo(&I, Actual, CDCtx);
assert(!Err);
std::string Expected = R"raw(
@@ -194,7 +194,12 @@
class r
- Defined at line 10 of test.cpp
+
+Defined at line
+http://www.repository.com/dir/test.cpp#10";>10
+ of file
+http://www.repository.com/dir/test.cpp";>test.cpp
+
Inherits from
F
@@ -232,7 +237,7 @@
I.Name = "f";
I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
- I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+ I.DefLoc = Location(10, llvm::SmallString<16>{"dir/test.cpp"}, false);
I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
SmallString<16> PathTo;
@@ -246,7 +251,7 @@
assert(G);
std::string Buffer;
llvm::raw_string_ostream Actual(Buffer);
- ClangDocContext CDCtx = getClangDocContext();
+ ClangDocContext CDCtx = getClangDocContext({}, "https://www.repository.com";);
auto Err = G->generateDocForInfo(&I, Actual, CDCtx);
assert(!Err);
std::string Expected = R"raw(
@@ -263,7 +268,7 @@
int
P)
- Defined at line 10 of test.cpp
+ Defined at line 10 of file dir/test.cpp
)raw";
@@ -275,7 +280,7 @@
I.Name = "e";
I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
- I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+ I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"}, true);
I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
I.Members.emplace_back("X");
@@ -285,7 +290,7 @@
assert(G);
std::string Buffer;
llvm::raw_string_ostream Actual(Buffer);
- ClangDocContext CDCtx = getClangDocContext();
+ ClangDocContext CDCtx = getClangDocContext({}, "www.repository.com");
auto Err = G->generateDocForInfo(&I, Actual, CDCtx);
assert(!Err);
std::string Expected = R"raw(
@@ -299,7 +304,12 @@
X
- Defined at line 10 of test.cpp
+
+Defined at line
+https://www.repository.com/test.cpp#10";>10
+ of file
+https://www.repository.com/test.cpp";>test.cpp
+
)raw";
@@ -368,7 +378,7 @@
f
void f(int I, int J)
- Defined at line 10 of test.cpp
+ Defined at line 10 of file test.cpp
Brief description.
Index:
[PATCH] D65483: [clang-doc] Add link to source code in file definitions
DiegoAstiazaran updated this revision to Diff 214000.
DiegoAstiazaran marked 2 inline comments as done.
DiegoAstiazaran added a comment.
Add comments.
Move definition of ClangDocContext constructor to .cpp file.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65483/new/
https://reviews.llvm.org/D65483
Files:
clang-tools-extra/clang-doc/BitcodeReader.cpp
clang-tools-extra/clang-doc/BitcodeWriter.cpp
clang-tools-extra/clang-doc/HTMLGenerator.cpp
clang-tools-extra/clang-doc/Mapper.cpp
clang-tools-extra/clang-doc/Mapper.h
clang-tools-extra/clang-doc/Representation.cpp
clang-tools-extra/clang-doc/Representation.h
clang-tools-extra/clang-doc/Serialize.cpp
clang-tools-extra/clang-doc/Serialize.h
clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
clang-tools-extra/docs/clang-doc.rst
clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -37,7 +37,7 @@
template bool mapDecl(const T *D) {
auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
+ /*File=*/"test.cpp", true, Public);
if (I.first)
EmittedInfos.emplace_back(std::move(I.first));
if (I.second)
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
@@ -22,9 +22,9 @@
}
ClangDocContext
-getClangDocContext(std::vector UserStylesheets = {}) {
- ClangDocContext CDCtx;
- CDCtx.UserStylesheets = {UserStylesheets.begin(), UserStylesheets.end()};
+getClangDocContext(std::vector UserStylesheets = {},
+ StringRef RepositoryUrl = "") {
+ ClangDocContext CDCtx{{}, {}, {}, {}, RepositoryUrl, UserStylesheets, {}};
CDCtx.UserStylesheets.insert(
CDCtx.UserStylesheets.begin(),
"../share/clang/clang-doc-default-stylesheet.css");
@@ -90,7 +90,7 @@
I.Path = "X/Y/Z";
I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
- I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+ I.DefLoc = Location(10, llvm::SmallString<16>{"dir/test.cpp"}, true);
I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
SmallString<16> PathTo;
@@ -110,7 +110,7 @@
assert(G);
std::string Buffer;
llvm::raw_string_ostream Actual(Buffer);
- ClangDocContext CDCtx = getClangDocContext();
+ ClangDocContext CDCtx = getClangDocContext({}, "http://www.repository.com";);
auto Err = G->generateDocForInfo(&I, Actual, CDCtx);
assert(!Err);
std::string Expected = R"raw(
@@ -121,7 +121,12 @@
class r
- Defined at line 10 of test.cpp
+
+Defined at line
+http://www.repository.com/dir/test.cpp#10";>10
+ of file
+http://www.repository.com/dir/test.cpp";>test.cpp
+
Inherits from
F
@@ -159,7 +164,7 @@
I.Name = "f";
I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
- I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+ I.DefLoc = Location(10, llvm::SmallString<16>{"dir/test.cpp"}, false);
I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
SmallString<16> PathTo;
@@ -173,7 +178,7 @@
assert(G);
std::string Buffer;
llvm::raw_string_ostream Actual(Buffer);
- ClangDocContext CDCtx = getClangDocContext();
+ ClangDocContext CDCtx = getClangDocContext({}, "https://www.repository.com";);
auto Err = G->generateDocForInfo(&I, Actual, CDCtx);
assert(!Err);
std::string Expected = R"raw(
@@ -190,7 +195,7 @@
int
P)
- Defined at line 10 of test.cpp
+ Defined at line 10 of file dir/test.cpp
)raw";
@@ -202,7 +207,7 @@
I.Name = "e";
I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
- I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+ I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"}, true);
I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
I.Members.emplace_back("X");
@@ -212,7 +217,7 @@
assert(G);
std::string Buffer;
llvm::raw_string_ostream Actual(Buffer);
- ClangDocContext CDCtx = getClangDocContext();
+ ClangDocContext CDCtx = getClangDocContext({}, "www.repository.com");
auto Err = G->generateDocForInfo(&I, Actual, CDCtx);
assert(!Err);
std::string Expected = R"raw(
@@ -226,7 +231,12 @@
X
- Defined at line 10 of test.cpp
+
+Defined at line
+https://www.repository.com/test.cpp#10";>10
+ of file
+https://www.repository.com/test.cpp";>test.cpp
+
)raw";
@@ -295,7 +305,7 @@
f
void f(int I, int J)
- Defined
[PATCH] D65483: [clang-doc] Add link to source code in file definitions
juliehockett added inline comments.
Comment at: clang-tools-extra/clang-doc/Representation.h:385
+SourceRoot(SourceRoot), UserStylesheets(UserStylesheets),
+JsScripts(JsScripts) {
+if (!RepositoryUrl.empty()) {
Move to .cpp file (now that it has a non-trivial implementation)
Comment at: clang-tools-extra/clang-doc/Representation.h:396
+ std::string OutDirectory; // Directory for outputting generated files.
+ std::string SourceRoot; //
+ llvm::Optional RepositoryUrl;
Comment
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65483/new/
https://reviews.llvm.org/D65483
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D65483: [clang-doc] Add link to source code in file definitions
DiegoAstiazaran updated this revision to Diff 213776.
DiegoAstiazaran added a comment.
Remove unnecessary type casting.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65483/new/
https://reviews.llvm.org/D65483
Files:
clang-tools-extra/clang-doc/BitcodeReader.cpp
clang-tools-extra/clang-doc/BitcodeWriter.cpp
clang-tools-extra/clang-doc/HTMLGenerator.cpp
clang-tools-extra/clang-doc/Mapper.cpp
clang-tools-extra/clang-doc/Mapper.h
clang-tools-extra/clang-doc/Representation.h
clang-tools-extra/clang-doc/Serialize.cpp
clang-tools-extra/clang-doc/Serialize.h
clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
clang-tools-extra/docs/clang-doc.rst
clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -37,7 +37,7 @@
template bool mapDecl(const T *D) {
auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
+ /*File=*/"test.cpp", true, Public);
if (I.first)
EmittedInfos.emplace_back(std::move(I.first));
if (I.second)
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
@@ -22,9 +22,9 @@
}
ClangDocContext
-getClangDocContext(std::vector UserStylesheets = {}) {
- ClangDocContext CDCtx;
- CDCtx.UserStylesheets = {UserStylesheets.begin(), UserStylesheets.end()};
+getClangDocContext(std::vector UserStylesheets = {},
+ StringRef RepositoryUrl = "") {
+ ClangDocContext CDCtx{{}, {}, {}, {}, RepositoryUrl, UserStylesheets, {}};
CDCtx.UserStylesheets.insert(
CDCtx.UserStylesheets.begin(),
"../share/clang/clang-doc-default-stylesheet.css");
@@ -90,7 +90,7 @@
I.Path = "X/Y/Z";
I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
- I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+ I.DefLoc = Location(10, llvm::SmallString<16>{"dir/test.cpp"}, true);
I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
SmallString<16> PathTo;
@@ -110,7 +110,8 @@
assert(G);
std::string Buffer;
llvm::raw_string_ostream Actual(Buffer);
- ClangDocContext CDCtx = getClangDocContext();
+ ClangDocContext CDCtx =
+ getClangDocContext({}, "http://www.repository.com";);
auto Err = G->generateDocForInfo(&I, Actual, CDCtx);
assert(!Err);
std::string Expected = R"raw(
@@ -121,7 +122,12 @@
class r
- Defined at line 10 of test.cpp
+
+Defined at line
+http://www.repository.com/dir/test.cpp#10";>10
+ of file
+http://www.repository.com/dir/test.cpp";>test.cpp
+
Inherits from
F
@@ -159,7 +165,7 @@
I.Name = "f";
I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
- I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+ I.DefLoc = Location(10, llvm::SmallString<16>{"dir/test.cpp"}, false);
I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
SmallString<16> PathTo;
@@ -173,7 +179,8 @@
assert(G);
std::string Buffer;
llvm::raw_string_ostream Actual(Buffer);
- ClangDocContext CDCtx = getClangDocContext();
+ ClangDocContext CDCtx =
+ getClangDocContext({}, "https://www.repository.com";);
auto Err = G->generateDocForInfo(&I, Actual, CDCtx);
assert(!Err);
std::string Expected = R"raw(
@@ -190,7 +197,7 @@
int
P)
- Defined at line 10 of test.cpp
+ Defined at line 10 of file dir/test.cpp
)raw";
@@ -202,7 +209,7 @@
I.Name = "e";
I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
- I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+ I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"}, true);
I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
I.Members.emplace_back("X");
@@ -212,7 +219,7 @@
assert(G);
std::string Buffer;
llvm::raw_string_ostream Actual(Buffer);
- ClangDocContext CDCtx = getClangDocContext();
+ ClangDocContext CDCtx = getClangDocContext({}, "www.repository.com");
auto Err = G->generateDocForInfo(&I, Actual, CDCtx);
assert(!Err);
std::string Expected = R"raw(
@@ -226,7 +233,12 @@
X
- Defined at line 10 of test.cpp
+
+Defined at line
+https://www.repository.com/test.cpp#10";>10
+ of file
+https://www.repository.com/test.cpp";>test.cpp
+
)raw";
@@ -295,7 +307,7 @@
f
void f(int I, int J)
- Defined at line 10 of test.cpp
+ Defined at line 10 of file test.cpp
Brief description.
Index: clang-tools-extra/doc
[PATCH] D65483: [clang-doc] Add link to source code in file definitions
DiegoAstiazaran added inline comments. Comment at: clang-tools-extra/docs/clang-doc.rst:93 +--public- Document only public declarations. +--repository= - + URL of repository that hosts code. juliehockett wrote: > Formatting here is a bit weird Do you mean the almost empty line that's only "-"? This is because the description of the flag starts with \n. ```R"( URL of repository that hosts code. Used for links to definition locations.)"``` I used the same format used by [[ https://clang.llvm.org/extra/clang-tidy/ | clang-tidy ]], it has this format for all the multi-line flag descriptions. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65483/new/ https://reviews.llvm.org/D65483 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D65483: [clang-doc] Add link to source code in file definitions
DiegoAstiazaran updated this revision to Diff 213772.
DiegoAstiazaran marked 7 inline comments as done.
DiegoAstiazaran edited the summary of this revision.
DiegoAstiazaran added a comment.
Add comments.
Change name of flag; `root-directory` -> `--source-root`
Moved fixing of repository link to ClangDocContext constructor and add test
where the prefix is omitted.
RepositoryLink in ClangDocContext struct and writeFileDefinition function is
now llvm::Optional<>
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65483/new/
https://reviews.llvm.org/D65483
Files:
clang-tools-extra/clang-doc/BitcodeReader.cpp
clang-tools-extra/clang-doc/BitcodeWriter.cpp
clang-tools-extra/clang-doc/HTMLGenerator.cpp
clang-tools-extra/clang-doc/Mapper.cpp
clang-tools-extra/clang-doc/Mapper.h
clang-tools-extra/clang-doc/Representation.h
clang-tools-extra/clang-doc/Serialize.cpp
clang-tools-extra/clang-doc/Serialize.h
clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
clang-tools-extra/docs/clang-doc.rst
clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -37,7 +37,7 @@
template bool mapDecl(const T *D) {
auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
+ /*File=*/"test.cpp", true, Public);
if (I.first)
EmittedInfos.emplace_back(std::move(I.first));
if (I.second)
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
@@ -22,9 +22,9 @@
}
ClangDocContext
-getClangDocContext(std::vector UserStylesheets = {}) {
- ClangDocContext CDCtx;
- CDCtx.UserStylesheets = {UserStylesheets.begin(), UserStylesheets.end()};
+getClangDocContext(std::vector UserStylesheets = {},
+ StringRef RepositoryUrl = "") {
+ ClangDocContext CDCtx{{}, {}, {}, {}, RepositoryUrl, UserStylesheets, {}};
CDCtx.UserStylesheets.insert(
CDCtx.UserStylesheets.begin(),
"../share/clang/clang-doc-default-stylesheet.css");
@@ -90,7 +90,7 @@
I.Path = "X/Y/Z";
I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
- I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+ I.DefLoc = Location(10, llvm::SmallString<16>{"dir/test.cpp"}, true);
I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
SmallString<16> PathTo;
@@ -110,7 +110,8 @@
assert(G);
std::string Buffer;
llvm::raw_string_ostream Actual(Buffer);
- ClangDocContext CDCtx = getClangDocContext();
+ ClangDocContext CDCtx =
+ getClangDocContext({}, StringRef{"http://www.repository.com"});
auto Err = G->generateDocForInfo(&I, Actual, CDCtx);
assert(!Err);
std::string Expected = R"raw(
@@ -121,7 +122,12 @@
class r
- Defined at line 10 of test.cpp
+
+Defined at line
+http://www.repository.com/dir/test.cpp#10";>10
+ of file
+http://www.repository.com/dir/test.cpp";>test.cpp
+
Inherits from
F
@@ -159,7 +165,7 @@
I.Name = "f";
I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
- I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+ I.DefLoc = Location(10, llvm::SmallString<16>{"dir/test.cpp"}, false);
I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
SmallString<16> PathTo;
@@ -173,7 +179,8 @@
assert(G);
std::string Buffer;
llvm::raw_string_ostream Actual(Buffer);
- ClangDocContext CDCtx = getClangDocContext();
+ ClangDocContext CDCtx =
+ getClangDocContext({}, StringRef{"https://www.repository.com"});
auto Err = G->generateDocForInfo(&I, Actual, CDCtx);
assert(!Err);
std::string Expected = R"raw(
@@ -190,7 +197,7 @@
int
P)
- Defined at line 10 of test.cpp
+ Defined at line 10 of file dir/test.cpp
)raw";
@@ -202,7 +209,7 @@
I.Name = "e";
I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
- I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+ I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"}, true);
I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
I.Members.emplace_back("X");
@@ -212,7 +219,7 @@
assert(G);
std::string Buffer;
llvm::raw_string_ostream Actual(Buffer);
- ClangDocContext CDCtx = getClangDocContext();
+ ClangDocContext CDCtx = getClangDocContext({}, "www.repository.com");
auto Err = G->generateDocForInfo(&I, Actual, CDCtx);
assert(!Err);
std::string Expected = R"raw(
@@ -226,7 +233,12 @@
X
- Defined at l
[PATCH] D65483: [clang-doc] Add link to source code in file definitions
juliehockett added inline comments.
Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:377-378
+ "href", (FileURL + "#" + std::to_string(L.LineNumber)).str());
+ Node->Children.emplace_back(std::move(LocNumberNode));
+ Node->Children.emplace_back(llvm::make_unique(" of file "));
+ auto LocFileNode = llvm::make_unique(
DiegoAstiazaran wrote:
> jakehehrlich wrote:
> > Can you put these in the link so that the link is larger than a single
> > number? e.g. "303 of file foo.c" should be linkified rather than just "303"
> > which is how I read the code now.
> "303" is linkified to the specific line in the source code but also "foo.c"
> is linkified to the top of the source code page.
> This is how it is done in Doxygen documentation.
> I think it would look weird to have " of file " linkified. But talking to
> @phosek, we agreed that it would be better to remove the whole "Defined at
> ..." and make the info name linkified to the definition, with a tooltip that
> shows "foo.c:303".
> So I think we could leave it like this for now and later, in another patch
> (this will require some CSS), do what I just mentioned. What do you think?
While the tooltip idea seems interesting, I'm inclined to keep the "Defined at"
portion and an actual link with the text for now. I'd want to see an example of
what the tooltip option would look like before going with it.
For now, the separate line number and file linkification SGTM.
Comment at: clang-tools-extra/clang-doc/Mapper.cpp:103-104
+ llvm::SmallString<128> Prefix(RootDir);
+ if (!llvm::sys::path::is_separator(Prefix.back()))
+Prefix += llvm::sys::path::get_separator();
+ llvm::sys::path::replace_path_prefix(File, Prefix, "");
DiegoAstiazaran wrote:
> jakehehrlich wrote:
> > Do you actually need to do this? Feels like a bug in replace_path_prefix
> > per its own documentation if you do.
> `replace_path_prefix` simply calls substr() on the path so if the prefix does
> not include the separator at the end, the resulting path will have it at the
> beginning.
Leave a comment to that effect, then.
Comment at: clang-tools-extra/clang-doc/Mapper.cpp:40
+ bool IsFileInRootDir;
+ auto File = getFile(D, D->getASTContext(), CDCtx.RootDir, IsFileInRootDir);
+ auto I = serialize::emitInfo(D, getComment(D, D->getASTContext()),
Don't use auto here, since it's not immediately obvious what the type is.
Comment at: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp:228
+ RepositoryUrl.find("https://";) != 0)
+RepositoryUrl.insert(0, "http://";);
+
DiegoAstiazaran wrote:
> jakehehrlich wrote:
> > 1) Do we need to add this for the user?
> > 2) Can we use https by default if we need this?
> Not required but I consider it'd be nice to have it.
> You're right, https should be the default.
This is fine, but please add a test case for the when the user omits the prefix.
Comment at: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp:76
+static llvm::cl::opt
+RootDirectory("root-directory", llvm::cl::desc(R"(
+Directory where processed files are stored.
Can we call this `--source-root`?
Comment at: clang-tools-extra/docs/clang-doc.rst:93
+--public- Document only public declarations.
+--repository= -
+ URL of repository that hosts code.
Formatting here is a bit weird
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65483/new/
https://reviews.llvm.org/D65483
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D65483: [clang-doc] Add link to source code in file definitions
DiegoAstiazaran added a subscriber: phosek.
DiegoAstiazaran added inline comments.
Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:377-378
+ "href", (FileURL + "#" + std::to_string(L.LineNumber)).str());
+ Node->Children.emplace_back(std::move(LocNumberNode));
+ Node->Children.emplace_back(llvm::make_unique(" of file "));
+ auto LocFileNode = llvm::make_unique(
jakehehrlich wrote:
> Can you put these in the link so that the link is larger than a single
> number? e.g. "303 of file foo.c" should be linkified rather than just "303"
> which is how I read the code now.
"303" is linkified to the specific line in the source code but also "foo.c" is
linkified to the top of the source code page.
This is how it is done in Doxygen documentation.
I think it would look weird to have " of file " linkified. But talking to
@phosek, we agreed that it would be better to remove the whole "Defined at ..."
and make the info name linkified to the definition, with a tooltip that shows
"foo.c:303".
So I think we could leave it like this for now and later, in another patch
(this will require some CSS), do what I just mentioned. What do you think?
Comment at: clang-tools-extra/clang-doc/Mapper.cpp:103-104
+ llvm::SmallString<128> Prefix(RootDir);
+ if (!llvm::sys::path::is_separator(Prefix.back()))
+Prefix += llvm::sys::path::get_separator();
+ llvm::sys::path::replace_path_prefix(File, Prefix, "");
jakehehrlich wrote:
> Do you actually need to do this? Feels like a bug in replace_path_prefix per
> its own documentation if you do.
`replace_path_prefix` simply calls substr() on the path so if the prefix does
not include the separator at the end, the resulting path will have it at the
beginning.
Comment at: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp:228
+ RepositoryUrl.find("https://";) != 0)
+RepositoryUrl.insert(0, "http://";);
+
jakehehrlich wrote:
> 1) Do we need to add this for the user?
> 2) Can we use https by default if we need this?
Not required but I consider it'd be nice to have it.
You're right, https should be the default.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65483/new/
https://reviews.llvm.org/D65483
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D65483: [clang-doc] Add link to source code in file definitions
DiegoAstiazaran updated this revision to Diff 213169.
DiegoAstiazaran marked 7 inline comments as done.
DiegoAstiazaran added a comment.
Change http to https as the default fix to the repository link.
Add new flags to clang-doc documentation.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65483/new/
https://reviews.llvm.org/D65483
Files:
clang-tools-extra/clang-doc/BitcodeReader.cpp
clang-tools-extra/clang-doc/BitcodeWriter.cpp
clang-tools-extra/clang-doc/HTMLGenerator.cpp
clang-tools-extra/clang-doc/Mapper.cpp
clang-tools-extra/clang-doc/Mapper.h
clang-tools-extra/clang-doc/Representation.h
clang-tools-extra/clang-doc/Serialize.cpp
clang-tools-extra/clang-doc/Serialize.h
clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
clang-tools-extra/docs/clang-doc.rst
clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -37,7 +37,7 @@
template bool mapDecl(const T *D) {
auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
+ /*File=*/"test.cpp", true, Public);
if (I.first)
EmittedInfos.emplace_back(std::move(I.first));
if (I.second)
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
@@ -22,12 +22,14 @@
}
ClangDocContext
-getClangDocContext(std::vector UserStylesheets = {}) {
+getClangDocContext(std::vector UserStylesheets = {},
+ StringRef RepositoryUrl = "") {
ClangDocContext CDCtx;
CDCtx.UserStylesheets = {UserStylesheets.begin(), UserStylesheets.end()};
CDCtx.UserStylesheets.insert(
CDCtx.UserStylesheets.begin(),
"../share/clang/clang-doc-default-stylesheet.css");
+ CDCtx.RepositoryUrl = RepositoryUrl;
return CDCtx;
}
@@ -87,7 +89,7 @@
I.Path = "X/Y/Z";
I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
- I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+ I.DefLoc = Location(10, llvm::SmallString<16>{"dir/test.cpp"}, true);
I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
SmallString<16> PathTo;
@@ -107,7 +109,7 @@
assert(G);
std::string Buffer;
llvm::raw_string_ostream Actual(Buffer);
- ClangDocContext CDCtx = getClangDocContext();
+ ClangDocContext CDCtx = getClangDocContext({}, "https://www.repository.com";);
auto Err = G->generateDocForInfo(&I, Actual, CDCtx);
assert(!Err);
std::string Expected = R"raw(
@@ -116,7 +118,12 @@
class r
- Defined at line 10 of test.cpp
+
+Defined at line
+https://www.repository.com/dir/test.cpp#10";>10
+ of file
+https://www.repository.com/dir/test.cpp";>test.cpp
+
Inherits from
F
@@ -154,7 +161,7 @@
I.Name = "f";
I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
- I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+ I.DefLoc = Location(10, llvm::SmallString<16>{"dir/test.cpp"}, false);
I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
SmallString<16> PathTo;
@@ -168,7 +175,7 @@
assert(G);
std::string Buffer;
llvm::raw_string_ostream Actual(Buffer);
- ClangDocContext CDCtx = getClangDocContext();
+ ClangDocContext CDCtx = getClangDocContext({}, "https://www.repository.com";);
auto Err = G->generateDocForInfo(&I, Actual, CDCtx);
assert(!Err);
std::string Expected = R"raw(
@@ -183,7 +190,7 @@
int
P)
- Defined at line 10 of test.cpp
+ Defined at line 10 of file dir/test.cpp
)raw";
@@ -217,7 +224,7 @@
X
- Defined at line 10 of test.cpp
+ Defined at line 10 of file test.cpp
)raw";
@@ -284,7 +291,7 @@
f
void f(int I, int J)
- Defined at line 10 of test.cpp
+ Defined at line 10 of file test.cpp
Brief description.
Index: clang-tools-extra/docs/clang-doc.rst
===
--- clang-tools-extra/docs/clang-doc.rst
+++ clang-tools-extra/docs/clang-doc.rst
@@ -79,17 +79,27 @@
clang-doc options:
--doxygen - Use only doxygen-style comments to generate docs.
--dump - Dump intermediate results to bitcode file.
--extra-arg=- Additional argument to append to the compiler command line
--extra-arg-before= - Additional argument to prepend to the compiler command line
---format= - Format for outputted docs.
- =yaml- Documentation in YAML format.
- =md
[PATCH] D65483: [clang-doc] Add link to source code in file definitions
jakehehrlich added inline comments.
Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:376
+ LocNumberNode->Attributes.try_emplace(
+ "href", (FileURL + "#" + std::to_string(L.LineNumber)).str());
+ Node->Children.emplace_back(std::move(LocNumberNode));
Add a comment here that this is the github/googlesource notation for this that
way in the future people arren't confused when it doesn't work for other
hosting pages
Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:377-378
+ "href", (FileURL + "#" + std::to_string(L.LineNumber)).str());
+ Node->Children.emplace_back(std::move(LocNumberNode));
+ Node->Children.emplace_back(llvm::make_unique(" of file "));
+ auto LocFileNode = llvm::make_unique(
Can you put these in the link so that the link is larger than a single number?
e.g. "303 of file foo.c" should be linkified rather than just "303" which is
how I read the code now.
Comment at: clang-tools-extra/clang-doc/Mapper.cpp:103-104
+ llvm::SmallString<128> Prefix(RootDir);
+ if (!llvm::sys::path::is_separator(Prefix.back()))
+Prefix += llvm::sys::path::get_separator();
+ llvm::sys::path::replace_path_prefix(File, Prefix, "");
Do you actually need to do this? Feels like a bug in replace_path_prefix per
its own documentation if you do.
Comment at: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp:228
+ RepositoryUrl.find("https://";) != 0)
+RepositoryUrl.insert(0, "http://";);
+
1) Do we need to add this for the user?
2) Can we use https by default if we need this?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65483/new/
https://reviews.llvm.org/D65483
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D65483: [clang-doc] Add link to source code in file definitions
Eugene.Zelenko added a comment. It'll be reasonable to mention new command-line arguments in documentation and Release Notes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65483/new/ https://reviews.llvm.org/D65483 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
