search

jbms / sphinx-immaterial

Adaptation of the popular mkdocs-material material design theme to the sphinx documentation system
https://jbms.github.io/sphinx-immaterial/
Other
200 stars 32 forks source link

Refactor cpp fixes in preparation for C++ autosummary extension #116

Closed jbms closed 2 years ago

2bndy5 commented 2 years ago

I'm a bit confused why this is a separate PR since its geared directly for #117

jbms commented 2 years ago

117 involves a lot of changes and may need to be revised for a while. I figured I'd separate out these changes so that they can possibly get merged in sooner. Since this refactors a lot of code, it is better not to leave it pending for a long time, since that would make it more difficult to make other changes at the same time, like making the parameter cross-linking optional.

2bndy5 commented 2 years ago

Kinda makes sense, but it seems like a complex rebasing tactic. Should this be made a draft and kept up in parallel to #117 (for as long as that's open)?

jbms commented 2 years ago

I plan to keep the refactor-cpp-fixes branch (this PR) as an ancestor of cpp-apigen (#117).

https://github.com/jbms/sphinx-immaterial/network

I'm hoping though that this can be merged relatively soon while we still revise #117, since I don't think further revisions of #117 will require additional changes here.

jbms commented 2 years ago

Thanks for splitting this up into bite sized extensions (much easier to review this way).

Possible separate issues

I wouldn't mind seeing a new cpp:namespace-decl directive to actually allow documenting a namespace declaration (even if not using the upcoming cpp.apigen ext). I think breathe uses a modified type declaration for this.

Yes that could be added to the C++ domain.

Separate from that, it would be nice to modify the C++ domain to suppress the nitpick reference warnings for every namespace, though.

However, I think that might be kind of annoying to monkey patch in --- maybe it can be, though.

I also noticed that the cpp:function directive doesn't support exception specifications like

void function(int n) throws(std::out_of_range);

While I understand that this is generally discouraged due to differences in compiler behavior, it is still part of the C++ specs.

I imagine this could be supported by the C++ domain easily enough. But note that it was deprecated in C++11 and removed in C++17:

https://en.cppreference.com/w/cpp/language/except_spec

2bndy5 commented 2 years ago

I imagine this could be supported by the C++ domain easily enough. But note that it was deprecated in C++11 and removed in C++17

This is why I haven't taken the issue upstream. Still the spec compatibility can be set at build time (like tensorstore does).