Add CMakePPLang class documentation support

[AutonomicPerfectionist]

Fixes #55

[codecov[bot]]

Codecov Report

Merging #58 (e16f77e) into master (836cb80) will increase coverage by 10.81%. The diff coverage is 95.95%.

@@             Coverage Diff             @@
##           master      #58       +/-   ##
===========================================
+ Coverage   85.65%   96.47%   +10.81%     
===========================================
  Files           9        5        -4     
  Lines         955      652      -303     
===========================================
- Hits          818      629      -189     
+ Misses        137       23      -114     

Flag	Coverage Δ
unittests	96.47% <95.95%> (+10.81%)	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
cminx/parser/aggregator.py	95.85% <95.63%> (+5.95%)	:arrow_up:
cminx/documenter.py	98.16% <96.66%> (-1.84%)	:arrow_down:
cminx/__init__.py	90.36% <100.00%> (+0.61%)	:arrow_up:
cminx/parser/CMakeListener.py
cminx/parser/CMakeLexer.py
cminx/parser/CMakeVisitor.py
cminx/parser/CMakeParser.py

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b363125...e16f77e. Read the comment docs.

[zachcran]

@AutonomicPerfectionist What are your plans for documenting the type of a cpp_attr if there is not a default value from which to infer it? A way to explicitly document the intended data type would be useful for attribute types that are not easy to provide a default value for or infer the type of, like user-defined classes or a cpp_map. (Sorry if it is already implemented! I don't fully understand your code here yet.)

Right now, I am documenting my cpp_attr calls like the example below to make sure I capture the type, but I'm not sure if it is a reasonable layout to actually extract that information for doc generation on your end. In this case, Toolchain is a class I wrote and is the data type for the attribute. Since I am writing and documenting code now, I'd like to make sure it conforms with your intended format here, so I don't have to go back and fix everything after this PR is complete.

#[[[
# User-specified and autopopulated toolchain file values.
#
# :var toolchain: User-specified and autopopulated toolchain file values.
# :vartype toolchain: ``Toolchain``
#]]
cpp_attr(ProjectSpecification toolchain)

[AutonomicPerfectionist]

@zachcran I am currently working on that, the goal is to use the :type: option of the .. py:attribute directive to specify the variable type. The issue I'm currently running into is a directive and its options cannot be separated by a blank line, so I'm considering having the documentation author put a :type: option at the beginning of the doc comment, and CMinx will extract it and put it in the correct place. I don't personally like making exceptions/special treatments like that, but it's probably the simplest way of getting it to work

[AutonomicPerfectionist]

It should also be noted that CMinx cannot currently infer the attribute type, that would require adding additional parsing logic and would be ambiguous in situations where the default value is a value of another variable. I would recommend setting the type explicitly for all attributes even if there is a default value

[zachcran]

Putting a :type: option at the top sounds reasonable to me as a quick fix, although I agree with your sentiment about the special treatment. From a quick search, I couldn't find anything about documenting global variables or class attributes in Python using Sphinx that would specify the types, so I don't have a better suggestion.

Would the format just be something like :type: str, or would you need the variable name, too, like :type variable_name: str?

Edit: FTR I am definitely not asking you to nail down a permanent format for this right now, I just want to adhere to the format you are thinking of right now so hopefully I am close to the correct format when you finalize things. If it changes while you implement things, I'll just fix my docs later.

[AutonomicPerfectionist]

@zachcran all that should be required is to do :type: str or whatever type you want in place of str at the top of the documentation comment, the variable name is not required

[AutonomicPerfectionist]

Is there a way to specify the type of the variable in the cpp_attr() call itself? If so that would be preferred, so CMinx could generate that option on its own

[zachcran]

@AutonomicPerfectionist I used :type: var_type in a recent PR to CMaize and even without special formatting the documentation for cpp_attr is more readable. Thank you!

Is there a way to specify the type of the variable in the cpp_attr() call itself? If so that would be preferred, so CMinx could generate that option on its own

Attributes are loosely typed, so no type is declared when declaring an attribute. source

[AutonomicPerfectionist]

@ryanmrichard I think this PR is ready as well, let me know if there's anything you'd like changed

[zachcran]

@AutonomicPerfectionist As an integration test, I installed this branch of CMinx and tried running it on the master branch of CMaize, but it gave the following error. Edit: I forgot to note that I double-checked that the master branch of CMinx does not throw an error.

Call to cminx

cminx "cmake/cmaize" -o "build/api_docs/cmaize" -r -p cmaize

Error

Writing RST files to /home/zachcran/programs/cmakepp_workspace/cmaize/build/api_docs/cmaize
Writing for file /home/zachcran/programs/cmakepp_workspace/cmaize/cmake/cmaize/cmaize.cmake
Writing RST file /home/zachcran/programs/cmakepp_workspace/cmaize/build/api_docs/cmaize/cmaize.rst
Traceback (most recent call last):
  File "/home/zachcran/programs/cmakepp_workspace/venv/bin/cminx", line 8, in <module>
    sys.exit(main())
  File "/home/zachcran/programs/cmakepp_workspace/venv/lib/python3.8/site-packages/cminx/__init__.py", line 70, in main
    document(input, output_path, args.recursive, args.prefix)
  File "/home/zachcran/programs/cmakepp_workspace/venv/lib/python3.8/site-packages/cminx/__init__.py", line 120, in document
    document_single_file(os.path.join(root, file), input_path, output_path, prefix)
  File "/home/zachcran/programs/cmakepp_workspace/venv/lib/python3.8/site-packages/cminx/__init__.py", line 159, in document_single_file
    output_writer = documenter.process()
  File "/home/zachcran/programs/cmakepp_workspace/venv/lib/python3.8/site-packages/cminx/documenter.py", line 79, in process
    self.process_docs(self.aggregator.documented)
  File "/home/zachcran/programs/cmakepp_workspace/venv/lib/python3.8/site-packages/cminx/documenter.py", line 102, in process_docs
    self.process_class_doc(doc)
  File "/home/zachcran/programs/cmakepp_workspace/venv/lib/python3.8/site-packages/cminx/documenter.py", line 211, in process_class_doc
    self.add_method_doc(member, d)
  File "/home/zachcran/programs/cmakepp_workspace/venv/lib/python3.8/site-packages/cminx/documenter.py", line 224, in add_method_doc
    doc.params) + ("[, ...]" if "args" in doc.param_types else "")
TypeError: argument of type 'NoneType' is not iterable

[AutonomicPerfectionist]

@zachcran should be fixed now

[ryanmrichard]

@zachcran can you confirm this works for you?

[zachcran]

@AutonomicPerfectionist @ryanmrichard It looks like it made it further this time! However, another error occurred:

Writing RST files to /home/zachcran/programs/cmakepp_workspace/cmaize/build/api_docs/cmaize
Writing for file /home/zachcran/programs/cmakepp_workspace/cmaize/cmake/cmaize/cmaize.cmake
Writing RST file /home/zachcran/programs/cmakepp_workspace/cmaize/build/api_docs/cmaize/cmaize.rst
Writing for file /home/zachcran/programs/cmakepp_workspace/cmaize/cmake/cmaize/toolchain/toolchain.cmake
Writing RST file /home/zachcran/programs/cmakepp_workspace/cmaize/build/api_docs/cmaize/toolchain/toolchain.rst
Writing for file /home/zachcran/programs/cmakepp_workspace/cmaize/cmake/cmaize/project_specification/project_specification.cmake
Writing RST file /home/zachcran/programs/cmakepp_workspace/cmaize/build/api_docs/cmaize/project_specification/project_specification.rst
Writing for file /home/zachcran/programs/cmakepp_workspace/cmaize/cmake/cmaize/targets/target.cmake
Writing RST file /home/zachcran/programs/cmakepp_workspace/cmaize/build/api_docs/cmaize/targets/target.rst
Writing for file /home/zachcran/programs/cmakepp_workspace/cmaize/cmake/cmaize/targets/installed_target.cmake
Writing RST file /home/zachcran/programs/cmakepp_workspace/cmaize/build/api_docs/cmaize/targets/installed_target.rst
Writing for file /home/zachcran/programs/cmakepp_workspace/cmaize/cmake/cmaize/targets/targets.cmake
Writing RST file /home/zachcran/programs/cmakepp_workspace/cmaize/build/api_docs/cmaize/targets/targets.rst
Writing for file /home/zachcran/programs/cmakepp_workspace/cmaize/cmake/cmaize/targets/build_target.cmake
Writing RST file /home/zachcran/programs/cmakepp_workspace/cmaize/build/api_docs/cmaize/targets/build_target.rst
Writing for file /home/zachcran/programs/cmakepp_workspace/cmaize/cmake/cmaize/utilities/split_version.cmake
Writing RST file /home/zachcran/programs/cmakepp_workspace/cmaize/build/api_docs/cmaize/utilities/split_version.rst
Writing for file /home/zachcran/programs/cmakepp_workspace/cmaize/cmake/cmaize/utilities/utilities.cmake
Writing RST file /home/zachcran/programs/cmakepp_workspace/cmaize/build/api_docs/cmaize/utilities/utilities.rst
Writing RST files to /home/zachcran/programs/cmakepp_workspace/cmaize/build/api_docs/cpp
Writing for file /home/zachcran/programs/cmakepp_workspace/cmaize/cmake/cpp/cpp.cmake
Writing RST file /home/zachcran/programs/cmakepp_workspace/cmaize/build/api_docs/cpp/cpp.rst
Writing for file /home/zachcran/programs/cmakepp_workspace/cmaize/cmake/cpp/fetch/fetch_and_available.cmake
Writing RST file /home/zachcran/programs/cmakepp_workspace/cmaize/build/api_docs/cpp/fetch/fetch_and_available.rst
Writing for file /home/zachcran/programs/cmakepp_workspace/cmaize/cmake/cpp/fetch/fetch.cmake
Writing RST file /home/zachcran/programs/cmakepp_workspace/cmaize/build/api_docs/cpp/fetch/fetch.rst
Traceback (most recent call last):
  File "/home/zachcran/programs/cmakepp_workspace/venv/bin/cminx", line 8, in <module>
    sys.exit(main())
  File "/home/zachcran/programs/cmakepp_workspace/venv/lib/python3.8/site-packages/cminx/__init__.py", line 70, in main
    document(input, output_path, args.recursive, args.prefix)
  File "/home/zachcran/programs/cmakepp_workspace/venv/lib/python3.8/site-packages/cminx/__init__.py", line 120, in document
    document_single_file(os.path.join(root, file), input_path, output_path, prefix)
  File "/home/zachcran/programs/cmakepp_workspace/venv/lib/python3.8/site-packages/cminx/__init__.py", line 158, in document_single_file
    documenter = Documenter(file, header_name)
  File "/home/zachcran/programs/cmakepp_workspace/venv/lib/python3.8/site-packages/cminx/documenter.py", line 67, in __init__
    self.walker.walk(self.aggregator, self.tree)
  File "/home/zachcran/programs/cmakepp_workspace/venv/lib/python3.8/site-packages/antlr4/tree/Tree.py", line 151, in walk
    self.walk(listener, child)
  File "/home/zachcran/programs/cmakepp_workspace/venv/lib/python3.8/site-packages/antlr4/tree/Tree.py", line 149, in walk
    self.enterRule(listener, t)
  File "/home/zachcran/programs/cmakepp_workspace/venv/lib/python3.8/site-packages/antlr4/tree/Tree.py", line 163, in enterRule
    ctx.enterRule(listener)
  File "/home/zachcran/programs/cmakepp_workspace/venv/lib/python3.8/site-packages/cminx/parser/CMakeParser.py", line 188, in enterRule
    listener.enterCommand_invocation(self)
  File "/home/zachcran/programs/cmakepp_workspace/venv/lib/python3.8/site-packages/cminx/parser/aggregator.py", line 380, in enterCommand_invocation
    self.documented_classes_stack.pop()
IndexError: pop from empty list

[AutonomicPerfectionist]

@zachcran which branch is that occuring on? I swear I tested on the master branch of CMaize last night and I didn't see that...

[AutonomicPerfectionist]

I have found the issue though, shouldn't be too difficult to fix

[zachcran]

which branch is that occuring on? I swear I tested on the master branch of CMaize last night and I didn't see that...

I was on the master branch, and I tried it on a feature branch I am working on as well.

[zachcran]

@AutonomicPerfectionist Oh, I realized what the difference is! I am building documentation for both cmake/cmaize and cmake/cpp in my build_docs.sh script.

Run cminx "cmake/cpp" -o "build/api_docs/cpp" -r -p cpp to see the new error. Sorry about that.

[AutonomicPerfectionist]

@zachcran @ryanmrichard should be fixed and ready now, assuming I'm running the documentation command correctly. I also added some enhancements since it was easiest to fix the issue by doing so anyway. Now, all functions, macros, classes, attributes, members, etc. are documented, regardless of whether or not they have docstrings, similar to how Javadoc and the like function. The only exceptions are generic command invocations and undocumented variables, since both would generate a ton of worthless documentation. Additionally I stripped the .cmake part of the header and module names because it got confusing whether that was part of the module path or just the file extension.

[zachcran]

I like it. I generated the docs locally for CMaize and they look good! My only complaint is that it is hard to tell the attributes from the functions at a glance, since they all just have a grey box around them and there is no separator between them. I don't know if that is something you have control over, though, since I do not know what Python class attribute documentation looks like.

[AutonomicPerfectionist]

I like it. I generated the docs locally for CMaize and they look good! My only complaint is that it is hard to tell the attributes from the functions at a glance, since they all just have a grey box around them and there is no separator between them. I don't know if that is something you have control over, though, since I do not know what Python class attribute documentation looks like.

@zachcran unfortunately that is up to the theme being used. I can try splitting attributes and methods up and have a separator in between though

[ryanmrichard]

I agree that having the attributes somehow separate (and delimited) from the methods would be nice.

You're welcome to explore other themes (or customize ours). I spent some time looking at other themes, but out of the box none of them ever seemed to look as nice as the read the docs theme. Apparently the RTD theme has a lot of stuff built into it that your average Sphinx theme doesn't.

[AutonomicPerfectionist]

@ryanmrichard @zachcran I've now separated out the class attributes, methods, and constructors with small bolded headings above each. Unfortunately it doesn't appear to be possible to add a line separating them as Sphinx interprets that as a malformed section header. Since the constructors are now separated from the methods I felt it was okay to remove the info admonition stating that they are constructors, it helps with readability in my opinion.

Finally, I noticed that variable documentation (generated from documented set() commands) was showing the type as the string representation of the internal enum used to represent the type, so it looked a bit odd next to the others. I've fixed that as well so it shows properly now.

[zachcran]

That renders really nicely now, thank you! One more question (maybe): how is the order of files in the toctree directives determined? It does not appear to be alphabetical or follow any other obvious pattern to me. The order of methods and attributes seems to be in the order they are declared, so that is not an issue to me, though.

[AutonomicPerfectionist]

@zachcran that is determined by three lines in cminx/__init__.py, under the document() function: https://github.com/CMakePP/CMinx/blob/b36312566178b3ff37d1bd22859f10c14823369f/cminx/__init__.py#L92 https://github.com/CMakePP/CMinx/blob/b36312566178b3ff37d1bd22859f10c14823369f/cminx/__init__.py#L111-L112

So the toctree files are added by the order in which they appear in the filenames return value of os.walk(), which judging by the Python documentation appears to be entirely arbitrary. We can insert a call to sorted() if that is so desired however.

[zachcran]

@AutonomicPerfectionist I think the call to sorted() would be a good addition. @ryanmrichard What do you think?

[ryanmrichard]

@zachcran I think sorted for files is a good idea. I think declaration order for methods and such is the way to go (let's the user ultimately pick it).

[ryanmrichard]

It also occurs to me that it would be good to update the developer docs with answers to these questions. Basically I'm thinking a cheat sheet like section that would be like: where is file order determined, where are the index files generated, etc.

[CLAassistant]

All committers have signed the CLA.

[AutonomicPerfectionist]

@zachcran @ryanmrichard calls to sorted() have been inserted and the toctrees should now be alphabetically sorted. Subdirs are placed after filenames, so it will look like so:

.. toctree:: 
   :maxdepth: 2

   add_dir
   add_section
   add_test
   cmake_test
   colors
   exec_tests
   execution_unit
   expectfail_subprocess
   overrides
   register_exception_handler
   set_print_length
   asserts/index.rst
   detail_/index.rst
   templates/index.rst

While writing tests for this change I noticed that the top-level index.rst file generated in recursive mode with no set prefix has a header of:

#
.
#

I was able to fix this quite easily by setting the last element of the input path as the prefix if no other prefix is specified in recursive mode, but I would like to know if that is desired behavior. Here's an example:

Running cminx -r cmake_test/ -o build/ generates the following:

index.rst:

##########
cmake_test
##########

.. toctree::
   :maxdepth: 2

   add_dir
   add_section
   add_test
   cmake_test
   colors
   exec_tests
   execution_unit
   expectfail_subprocess
   overrides
   register_exception_handler
   set_print_length
   asserts/index.rst
   detail_/index.rst
   templates/index.rst

add_dir.rst:

##################
cmake_test.add_dir
##################

.. module:: cmake_test.add_dir

.. function:: ct_add_dir(_ad_dir)

   This function will find all *.cmake files in the specified directory as well as recursively through all subdirectories.
   It will then configure the boilerplate template to include() each cmake file and register each configured boilerplate
   with CTest. The configured templates will be executed seperately via CTest during the Test phase, and each *.cmake
   file found in the specified directory is assumed to contain CMakeTest tests.

   :param _ad_dir: The directory to search for *.cmake files. Subdirectories will be recursively searched.
   :type _ad_dir: path
   :param **kwargs: See below

   :Keyword Arguments:
   * *CMAKE_OPTIONS* (``list``) -- List of additional CMake options to be passed to all test invocations. Options should follow the syntax: `-D<variable_name>=<value>`

detail_/index.rst:

##################
cmake_test.detail_
##################

.. toctree::
   :maxdepth: 2

   utilities/index.rst

[zachcran]

@AutonomicPerfectionist It looks like the sorted() solution is working for CMaize.

I was able to fix this quite easily by setting the last element of the input path as the prefix if no other prefix is specified in recursive mode, but I would like to know if that is desired behavior.

I vote for implementing this fix. It would eliminate the need for the -p <package_name> that I have to put in all of my cminx calls.

[ryanmrichard]

@AutonomicPerfectionist I second @zachcran. I think defaulting to the directory is the way to go.

[AutonomicPerfectionist]

@zachcran @ryanmrichard the prefix fix has been implemented and tests have been added. Is there anything else that should be changed or added?