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<TextNode>(" of file "));
+ auto LocFileNode = llvm::make_unique<TagNode>(
----------------
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
