-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
C coverage support in sphinx.ext.coverage
is non-functional
#11590
Comments
I mentioned this in the PR to fix this, #11591, but I wonder if we want to deprecate the C coverage feature entirely instead? Nobody appears to actually be using it, as evidenced by the fact that no one complained about it being broken, and compared to the Python coverage feature the C one is super janky. We don't have any of the introspection capabilities for C that we have with Python and we also don't have the ability to parse C beyond the limited AST subset we've implemented to parse things like function definitions (the fact that we don't insist on the code being preprocessed to resolve macros etc. hamstrings us also). Because we're missing all these things, it means the user needs to "roll their own" for each and every project and provide a rather convoluted list of configuration options including the list of directories to find code (most likely header files) in, regexes that Sphinx can use to "parse" said code and differentiate between functions, macros, etc. (and because there's no requirement to preprocess, the user needs to take these into account also), additional regexes to ignore certain objects etc. It's a real mess IMO. If we wanted to properly do this, I'd like to incorporate e.g. https://github.com/eliben/pycparser (which, note, does insist of preprocessing the code first) so that we could truly "parse" the C code, but that doesn't feel like something that should live in Sphinx core. Also, we have Doxygen and a variety of documentation coverage tools that we could simply push users towards, such as https://github.com/psycofdj/coverxygen. What are peoples' thoughts on just dumping this whole aspect of the coverage tool and focusing on Python coverage only (at least within core Sphinx)? |
It's funny to see that 1) the feature seemed buggy for a long time 2) no one complained. Instead of trying to reinvent the wheel by incorporating the C coverage in Sphinx, I think it's indeed better to just tell users to use another tool. (Also, less features means less possible issues and less work) |
Techically we'd need to remove this in 8.0 or 9.0. Perhaps we could adapt the coverage builder to be language-agnostic, and offer the C parts as a first party extension? A |
Yes, to be clear I am suggesting deprecating for removal (hard deprecation). Removal wouldn't happen until a major version bump as per our usual deprecation policies.
We could but until someone comes asking for this and offers to do the work I'd be inclined to deprecate and remove. Again, it appears no one is using this functionality and I only discovered this while trying to add docs for the extension. |
Signed-off-by: Stephen Finucane <stephen@that.guru>
Fixed in #11591 |
Describe the bug
The
sphinx.ext.coverage
extension supports checking coverage for both the Python and C domain. However, the latter does not appear to work. The extension checks theself.env.domaindata['c']['objects']
but unlike in the Python domain, this attribute is never set. As a result, every API will show as undocumented. My git splunking suggests this has been the case for many years and there's a chance this has never worked, which probably explains why the only usage of the various C-specific configuration options I can find on GitHub are in CPython (an attempted user), Sphinx (the definition) or a fork of one of these two projects.How to Reproduce
To reproduce this you need a C application. I have packaged a minimal example, which you can find here:
https://github.com/stephenfin/sphinx_ext_coverage_test
Instructions are provided there on how to use this repo to reproduce.
Environment Information
Sphinx extensions
Additional context
No response
The text was updated successfully, but these errors were encountered: