paulkirth added a comment.
Can you document the new directory structure with a small example? It would
also be good to document in the code that some of these changes were driven by
file corruption, and if possible point to a bug on github.
================
Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:890-892
+ // TODO If there are multiple Infos for this file name (for example,
+ // template specializations), this will generate multiple complete web
pages
+ // (with <DOCTYPE> and <title>, etc.) concatenated together. This generator
----------------
So does this patch make HTML unusable? Currently, if there is a conflict, the
file gets replaced, right?
I read this as we're going from a situation where we lost data, but still had
valid HTML pages to one where we have complete, but incorrect HTML, is that
correct?
Also, why does that happen under template specialization? USRs are SHA1 hashes
of the symbol. I would expect that hashing the symbol would be unambiguous and
unique, given C++'s ODR.
That last point made me wonder if an alternate way to handle this without using
the hash would be to use the mangled names?
================
Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:860
+
+ // Collect all output by file name and create the nexessary directories.
+ llvm::StringMap<std::vector<doc::Info *>> FileToInfos;
----------------
Nit: necessary
================
Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:893
+ // (with <DOCTYPE> and <title>, etc.) concatenated together. This generator
+ // needs some refactoring to be able to output the headers separatelky from
+ // the contents.
----------------
Nit: separately
================
Comment at: clang-tools-extra/clang-doc/MDGenerator.cpp:382
+
+ // Collect all output by file name and create the nexessary directories.
+ llvm::StringMap<std::vector<doc::Info *>> FileToInfos;
----------------
nit: necessary
================
Comment at: clang-tools-extra/clang-doc/MDGenerator.cpp:422-435
+llvm::Error MDGenerator::createResources(ClangDocContext &CDCtx) {
+ // Write an all_files.md
+ auto Err = serializeIndex(CDCtx);
+ if (Err)
+ return Err;
+
+ // Generate the index page.
----------------
Why did this move? Nothing has changed in its implementation, right?
================
Comment at: clang-tools-extra/test/clang-doc/single-file-public.cpp:6
// RUN: clang-doc --doxygen --public --executor=standalone -p %t %t/test.cpp
-output=%t/docs
-// RUN: cat %t/docs/GlobalNamespace/Record.yaml | FileCheck %s
--check-prefix=CHECK
+// RUN: cat %t/docs/`ls %t/docs | grep -v index.yaml` | FileCheck %s
--check-prefix=CHECK
// RUN: rm -rf %t
----------------
Its OK to use `ls` and `grep` in the runline, but you cannot use the backtick
syntax. It will cause problems on other platforms and I can't find any other
usage of that in our codebase. TBH I'm a bit surprised this works, since `lit`
has some interesting behavior around RUN.
I think you can just use `cat < ls %t/docs | grep -v index.yaml` instead. I
haven't tried it, but that is certainly a more typical method of doing this in
a lit test. Using `find` may also work, but I only found one use in
`llvm/test/ExecutionEngine/MCJIT/load-object-a.ll`.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138073/new/
https://reviews.llvm.org/D138073
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
