This is an archive of the discontinued LLVM Phabricator instance.

[libcxx] Add new Sphinx documentation
ClosedPublic

Authored by EricWF on Aug 18 2015, 5:47 PM.

Details

Summary

This patch adds Sphinx based documentation to libc++. The goal is to make it easier to write documentation for libc++ since writing new documentation in HTML is cumbersome. This patch rewrites the main page for libc++ along with the instructions for using, building and testing libc++.

The built documentation can be found and reviewed here: http://efcs.ca/libcxx-docs

In order to build the sphinx documentation you need to specify the cmake options -DLLVM_ENABLE_SPHINX=ON -DLIBCXX_INCLUDE_DOCS=ON. This will add the makefile rule docs-libcxx-html.

Diff Detail

Event Timeline

EricWF updated this revision to Diff 32482.Aug 18 2015, 5:47 PM
EricWF retitled this revision from to [libcxx] Add new Sphinx documentation.
EricWF updated this object.
EricWF added a subscriber: cfe-commits.
silvas added a subscriber: silvas.Aug 18 2015, 6:35 PM

From a Sphinx perspective, this looks fine to me. You may want to consider using html_theme = 'haiku' for consistency with clang, but reusing LLVM's is fine. (the reason I had clang use 'haiku' is that it is a bit better put together since it ships with sphinx (llvm's theme also has a notorious problem distinguishing different header levels); 'haiku' also doesn't require any static theme-related assets to be checked in since sphinx has them already as part of its own installation)

From a Sphinx perspective, this looks fine to me. You may want to consider using html_theme = 'haiku' for consistency with clang, but reusing LLVM's is fine. (the reason I had clang use 'haiku' is that it is a bit better put together since it ships with sphinx (llvm's theme also has a notorious problem distinguishing different header levels); 'haiku' also doesn't require any static theme-related assets to be checked in since sphinx has them already as part of its own installation)

I'll take a look at using clang's theme and see what comes out. Ironically I think I chose LLVM's theme because I preferred the way it distinguished between headers levels (although I have noticed some issues distinguishing nested levels). It would be nice to remove some assets from the repository.

I've uploaded a version of the docs built with the "haiku" theme to http://efcs.ca/libcxx-haiku-docs. I would like other's input on which style they prefer (Note: The LLVM themed docs are here http://efcs.ca/libcxx-docs).

Gentle ping. There is going to be a couple of days lag time between committing this and it showing up at libcxx.llvm.org/docs.
Once it is available at libcxx.llvm.org/docs we still have time to review it before we link to it from the homepage.
For this reason, and because the sphinx bits already got a thumbs up I would like to commit this now.

Plus I think the documentation is better reviewed in its generated form and not it's as plaintext.

@mclow.lists Would you have an objection to this approach?

NOTE: This patch does not change libcxx.llvm.org page or any existing documentation.
jroelofs accepted this revision.Aug 20 2015, 10:50 AM
jroelofs edited edge metadata.

Both styles look ok to me, with a slight preference toward matching clang's.

docs/BuildingLibcxx.rst
57

Is there a way to make this more prominent, perhaps by putting it in a red box? This is a very important detail that is very painful to recover from if you miss it.

244

s/Note the/Note that the/

docs/index.rst
88

Should we mention the unwinders used/supported here too?

This revision is now accepted and ready to land.Aug 20 2015, 10:50 AM
danalbert accepted this revision.Aug 20 2015, 10:55 AM
danalbert edited edge metadata.

I think I prefer the haiku style, but I couldn't give you any concrete reasons for that.

EricWF updated this revision to Diff 32718.Aug 20 2015, 11:45 AM
EricWF marked an inline comment as done.
EricWF updated this object.
EricWF edited edge metadata.

Address review comments.

docs/BuildingLibcxx.rst
57

I'll make it happen. I was also thinking about adding loud CMake warnings whenever CMake tries to install libc++ to /usr on OS X.

docs/index.rst
88

Sure but can we do that later? I have no clue what platforms support what and I would have to get some input on that.

jroelofs added inline comments.Aug 20 2015, 12:13 PM
docs/BuildingLibcxx.rst
57

I feel like a warning is not enough... perhaps a hard error, with a message that says something like: "If you're really really sure you know what you're doing, add -DLIBCXX_OVERRIDE_DARWIN_INSTALL=YES to silence this message"?

docs/index.rst
88

Yeah. This is good for now. I just wanted to mention it while I was thinking of it.

silvas added inline comments.Aug 20 2015, 1:16 PM
docs/BuildingLibcxx.rst
57

FYI, Sphinx has some directives designed for prominent warnings like this: http://docutils.sourceforge.net/docs/ref/rst/directives.html#danger

Should render similar to the "Warning" box on http://clang.llvm.org/docs/ReleaseNotes.html (see clang/docs/ReleaseNotes.rst)

silvas added inline comments.Aug 20 2015, 1:19 PM
docs/BuildingLibcxx.rst
57

The general names for these things are "admonitions" and there are e.g. 'note' 'danger' 'warning' etc. that are hopefully appropriately styled by the theme.

EricWF closed this revision.Aug 22 2015, 12:41 PM