CGAL / cgal

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

Missing links #7839

Open afabri opened 11 months ago

afabri commented 11 months ago

Issue Details

I am wondering what we can do on the missing links for CGAL::is_closed(tm) here.

Is this because we are in the subnamespace CGAL::Polygon_mesh_processing? Is the tm the problem?

It is the same for doxygen master here

@albert-github do you have any idea or suggestion?

albert-github commented 11 months ago

Yes, the tm is the problem as this is the, dummy / placeholder, name of the argument. One should either, in this case, use no arguments at all or use the type. I did a small test with

 * @pre `CGAL::is_closed(tm)`
 * @pre `CGAL::is_closed`
 * @pre `CGAL::is_closed()`
 * @pre `CGAL::is_closed(const TriangleMesh&)`

in include/CGAL/Polygon_mesh_processing/orientation.h and the result was: image

where the last 3 is_closed are hyperlinked.

afabri commented 11 months ago

As we want to express a precondition the tm is important here. So we probaby have to use a doxygen link command?

albert-github commented 11 months ago

Yes like:

 * @pre `CGAL::is_closed(tm)`
 * @pre \link CGAL::is_closed `CGAL::is_closed(tm)` \endlink

result: image

or use a descriptive text, as the usage if tm (is a bit contradicting the normal usage of the links by using the "dummy / placeholder name of the argument". Furthermore the tm is for a C++ programmer a bit confusing as it could also refer to a tm time type of struct see e.g. https://cplusplus.com/reference/ctime/tm/)

afabri commented 11 months ago

I would go for the \link. I don't see a problem with the tm of <ctime>. We are in the scope of the explanation of the function and we have introduced tm as a \param in this scope. And again, it seems normal that a precondition is an expression that concerns the input of the function. Thanks for having investigated. And you can confirm that this is not a doxygen issue that might get fixed in the future?

albert-github commented 11 months ago

No it is not a doxygen issue, it is so called usage.

You wrote "We are in the scope of the explanation of the function and we have introduced tm as a \param in this scope. " this is true, but some people only scan documentation and see at that moment tm and might think that it is the time struct (well they most likely will get a compilation error and can correct their error by properly reading the documentation). Personally I like more expressive argument names as it makes reading the code most of the time also easier (even though there is more effort in typing in the names with a chance of typo's).

sloriot commented 11 months ago

It is still not clear to me why doxygen can generate a link in examples but not in a header doc.

albert-github commented 11 months ago

Where did you see the link. Probably the reason is that in the example only the part is_closed is hyperlinked in the precondition the full word is attempted to link

sloriot commented 11 months ago

See https://doc.cgal.org/latest/Polygon_mesh_processing/Polygon_mesh_processing_2orient_polygon_soup_example_8cpp-example.html

albert-github commented 11 months ago

I assume you mean here the links of the function calls in the main function. The links in the comment and the links in the code (example) are handled differently. In the comment what is more "natural speech" the separators are most of the time characters like spaces, commas etc. whilst in the code brackets etc. also have an effect.