Please use GitHub pull requests for new patches. Avoid migrating existing patches. Phabricator shutdown timeline
Changeset View
Standalone View
docs/CMakeLists.txt
| Show All 16 Lines | if (LLVM_ENABLE_DOXYGEN) | ||||
| else() | else() | ||||
| set(enable_searchengine "NO") | set(enable_searchengine "NO") | ||||
| set(searchengine_url "") | set(searchengine_url "") | ||||
| set(enable_server_based_search "NO") | set(enable_server_based_search "NO") | ||||
| set(enable_external_search "NO") | set(enable_external_search "NO") | ||||
| set(extra_search_mappings "") | set(extra_search_mappings "") | ||||
| endif() | endif() | ||||
| # If asked, configure doxygen for the creation of a Qt Compressed Help file. | |||||
| option(LLVM_ENABLE_DOXYGEN_QT_HELP | |||||
gottesmm: I don't think we are doing banner comments in CMake. Double check there is precedent for this. | |||||
Not Done ReplyInline Actionsphabricator turned my # into a 1 since it thought that it was a list. That should be: "# If asked, configure doxygen for the creation of a Qt Compressed Help file." gottesmm: phabricator turned my # into a 1 since it thought that it was a list. That should be:
"# If… | |||||
| "Generate a Qt Compressed Help file." OFF) | |||||
| if (LLVM_ENABLE_DOXYGEN_QT_HELP) | |||||
Not Done ReplyInline ActionsCould you change this variable name to LLVM_ENABLE_DOXYGEN_QT_HELP. gottesmm: Could you change this variable name to LLVM_ENABLE_DOXYGEN_QT_HELP. | |||||
| set(LLVM_DOXYGEN_QCH_FILENAME "org.llvm.qch" CACHE STRING | |||||
| "Filename of the Qt Compressed help file") | |||||
| set(LLVM_DOXYGEN_QHP_NAMESPACE "org.llvm" CACHE STRING | |||||
| "Namespace under which the intermediate Qt Help Project file lives") | |||||
| set(LLVM_DOXYGEN_QHP_CUST_FILTER_NAME "${PACKAGE_STRING}" CACHE STRING | |||||
| "See http://qt-project.org/doc/qt-4.8/qthelpproject.html#custom-filters") | |||||
| set(LLVM_DOXYGEN_QHP_CUST_FILTER_ATTRS "${PACKAGE_NAME},${PACKAGE_VERSION}" CACHE STRING | |||||
| "See http://qt-project.org/doc/qt-4.8/qthelpproject.html#filter-attributes") | |||||
| find_program(LLVM_DOXYGEN_QHELPGENERATOR_PATH qhelpgenerator | |||||
| DOC "Path to the qhelpgenerator binary") | |||||
| if (LLVM_DOXYGEN_QHELPGENERATOR_PATH) | |||||
Not Done ReplyInline ActionsYou should check if we successfully find the program and error via message(FATAL_ERROR ...). If they asked us to generate this and we can't find the program it seems to me that that should be a hard error. gottesmm: You should check if we successfully find the program and error via message(FATAL_ERROR ...). If… | |||||
gottesmmUnsubmitted Not Done ReplyInline ActionsOne last nit. In general in LLVM we prefer to do this sort of thing like this: if (NOT LLVM_DOXYGEN_QHELPGENERATOR_PATH) message(FATAL_ERROR "Failed to find qhelpgenerator binary") endif() set(llvm_doxygen_generate_qhp "YES") But I will just fix it since it is such a nit. gottesmm: One last nit. In general in LLVM we prefer to do this sort of thing like this:
if (NOT… | |||||
| set(llvm_doxygen_generate_qhp "YES") | |||||
| set(llvm_doxygen_qch_filename "${LLVM_DOXYGEN_QCH_FILENAME}") | |||||
| set(llvm_doxygen_qhp_namespace "${LLVM_DOXYGEN_QHP_NAMESPACE}") | |||||
| set(llvm_doxygen_qhelpgenerator_path "${LLVM_DOXYGEN_QHELPGENERATOR_PATH}") | |||||
| set(llvm_doxygen_qhp_cust_filter_name "${LLVM_DOXYGEN_QHP_CUST_FILTER_NAME}") | |||||
| set(llvm_doxygen_qhp_cust_filter_attrs "${LLVM_DOXYGEN_QHP_CUST_FILTER_ATTRS}") | |||||
| else() | |||||
| message(FATAL_ERROR "Failed to find qhelpgenerator binary") | |||||
| endif() | |||||
| else() | |||||
| set(llvm_doxygen_generate_qhp "NO") | |||||
| set(llvm_doxygen_qch_filename "") | |||||
| set(llvm_doxygen_qhp_namespace "") | |||||
| set(llvm_doxygen_qhelpgenerator_path "") | |||||
| set(llvm_doxygen_qhp_cust_filter_name "") | |||||
| set(llvm_doxygen_qhp_cust_filter_attrs "") | |||||
| endif() | |||||
| configure_file(${CMAKE_CURRENT_SOURCE_DIR}/doxygen.cfg.in | configure_file(${CMAKE_CURRENT_SOURCE_DIR}/doxygen.cfg.in | ||||
| ${CMAKE_CURRENT_BINARY_DIR}/doxygen.cfg @ONLY) | ${CMAKE_CURRENT_BINARY_DIR}/doxygen.cfg @ONLY) | ||||
| set(abs_top_srcdir) | set(abs_top_srcdir) | ||||
Not Done ReplyInline ActionsYou should undefine the cmake variables you are defining for use in doxygen here like the rest of the cmake variables used to configure the doxygen file. gottesmm: You should undefine the cmake variables you are defining for use in doxygen here like the rest… | |||||
| set(abs_top_builddir) | set(abs_top_builddir) | ||||
| set(DOT) | set(DOT) | ||||
| set(enable_searchengine) | set(enable_searchengine) | ||||
| set(searchengine_url) | set(searchengine_url) | ||||
| set(enable_server_based_search) | set(enable_server_based_search) | ||||
| set(enable_external_search) | set(enable_external_search) | ||||
| set(extra_search_mappings) | set(extra_search_mappings) | ||||
| set(llvm_doxygen_generate_qhp) | |||||
| set(llvm_doxygen_qch_filename) | |||||
| set(llvm_doxygen_qhp_namespace) | |||||
| set(llvm_doxygen_qhelpgenerator_path) | |||||
| set(llvm_doxygen_qhp_cust_filter_name) | |||||
| set(llvm_doxygen_qhp_cust_filter_attrs) | |||||
| add_custom_target(doxygen-llvm | add_custom_target(doxygen-llvm | ||||
| COMMAND ${DOXYGEN_EXECUTABLE} ${CMAKE_CURRENT_BINARY_DIR}/doxygen.cfg | COMMAND ${DOXYGEN_EXECUTABLE} ${CMAKE_CURRENT_BINARY_DIR}/doxygen.cfg | ||||
| WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} | WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} | ||||
| COMMENT "Generating llvm doxygen documentation." VERBATIM) | COMMENT "Generating llvm doxygen documentation." VERBATIM) | ||||
| if (LLVM_BUILD_DOCS) | if (LLVM_BUILD_DOCS) | ||||
| add_dependencies(doxygen doxygen-llvm) | add_dependencies(doxygen doxygen-llvm) | ||||
| endif() | endif() | ||||
| if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY) | if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY) | ||||
| install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/doxygen/html | install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/doxygen/html | ||||
| DESTINATION docs/html) | DESTINATION docs/html) | ||||
| endif() | endif() | ||||
| endif() | endif() | ||||
| endif() | endif() | ||||
I don't think we are doing banner comments in CMake. Double check there is precedent for this. If there is ok, but if not just get rid of the banner. Something like this would be good: