CGAL / cgal

The public CGAL repository, see the README below
https://github.com/CGAL/cgal#readme
Other
4.66k stars 1.35k forks source link

CGAL documentation: different ways skip things from the documentation #2448

Open lrineau opened 6 years ago

lrineau commented 6 years ago

I am writing of piece of documentation for Mesh_3, using the "inline" method: having the documentation of a class directly in its header, in Mesh_3/ include/CGAL/ (by opposition to having a dedicated file in Mesh_3/doc/ Mesh_3/CGAL/).

There are public types and methods that I do not want to document. To "hide" those methods from the documentation, I have already found three different ways:

  1. Using #ifdef/#ifndef DOXYGEN_RUNNING

  2. Using the Doxygen command \internal

  3. Using the Doxygen pair of commands \cond DEVELOPER and \endcond (where DEVELOPER is a string of your choice).

In CGAL,

I think we should write recommandations. Currently, our guidelines mention the three possibilities but I think we should try to unify.

Addition: \internal cannot be used to skip a member function entirely. But only parts of a comment block.

sgiraudot commented 6 years ago

I've only been using DOXYGEN_RUNNING and \cond SKIP_IN_MANUAL. Basically, the way I'm using it:

I don't know what the other ones are used for.

lrineau commented 6 years ago

I've only been using DOXYGEN_RUNNING and \cond SKIP_IN_MANUAL. Basically, the way I'm using it: [...]

I use DOXYGEN_RUNNING and \cond DEVELOPERS, the same way. We just disagree on the keyword after \cond.

lrineau commented 6 years ago

In PR #2609, @sloriot has replaced all uses of \internal by \cond, and has unified the uses of \cond by always having the label SKIP_IN_MANUAL, that way:

\cond SKIP_IN_MANUAL

I disagree.

  1. \internal is very convenient to use: only one command, \internal, without a corresponding "end-tag".
  2. I think that \cond DEVELOPERS has a different meaning from \cond SKIP_IN_MANUAL:
    • The later (\cond SKIP_IN_MANUAL) seems equivalent to #ifdef DOXYGEN_RUNNING. It means that no matter what we do not want that in the manual.
    • The version using DEVELOPER, or INTERNAL (or \internal`) seems to indicate a part of the documentation that is for the eyes of developers only.

So in my opinion the discussion is not over, and I would not like that PR #2609 is merged. I think we document different ways, depending of the intent ("remove from the doc", or "for developers only").

I also suggest that our CMake scripts allows to compile the "internal" documentation, by activating at the same time \internal and \cond <the-same-we-choose-for-interal-doc>.

lrineau commented 6 years ago

In our guidelines, I think we should document three things:

  1. How to substitute something for the manual, so that Doxygen and the usual C++ compiler do not see the same code, using DOXYGEN_RUNNING.
  2. How to remove something completely from the manual,
    • with either conditional on DOXYGEN_RUNNING,
    • or with \cond or \if, and for that use we could choose one common identifier, such as: SKIP_IN_MANUAL.
  3. How to have a manual-for-developers,
    • with either \internal, because that is very convenient to use,
    • or \cond or \if with a special identifier that maybe be unified.

For the third usage (manual-for-developers), there could be a CMake option that activates at the same time \internal and the \cond sections.

sloriot commented 6 years ago

I'm personally not going to read any developer doc directly in a html file, I'd better read directly the code. The major issue being that those for developer only functions will be rendered in the manual like any other function as can be seen in the following screenshot: screenshot from 2017-11-21 13-58-37

I understand your point and things would have been different if a thing similar to TODO was generated (that is a page where every \internal would have been gathered).

lrineau commented 6 years ago

I have already used Doxygen to help reading a code, by using CALL_GRAPH, CALLER_GRAPH, and EXTRACT_ALL. The CMake option could also activate those options.