-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
issue #7231 Improvement of layout of refines relations. #7236
Conversation
Implemented: - no extra "Refines" page - Refines on one line - Refines always in code style - `2 arguments` -> `two arguments` (for consistency - added also the `\qualifier` Implemented in Kernel_23/doc/Kernel_23/Concepts/FunctionObjectConcepts.h Results a.o. - doc_output/Kernel_23/classKernel_1_1Circle__2.html - doc_output/Kernel_23/classKernel_1_1BoundedSide__3.html Note: due to the fact that this is a test implementation pages outside the scope of Kernel_23/doc/Kernel_23/Concepts/FunctionObjectConcepts.h might not be correct (in respect to code appearance of refines and multiple refines)
Updating the 1.8.4 and 1.8.13 version as well
This comment was marked as duplicate.
This comment was marked as duplicate.
1 similar comment
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
1 similar comment
The documentation is built. It will be available, after a few minutes, here: https://cgal.github.io/7236/v0/Manual/index.html |
For me a title is a title, and it looks cluttered with the additional boxes. |
@afabri Hopefully more comments will follow so we can make a good decision soon (and apply it on other places as well). |
Why not use the same the same presentation as "Inherits" (e.g. https://doc.cgal.org/latest/Triangulation_3/classCGAL_1_1Regular__triangulation__3.html)? In fact, I would actually inherit concepts so all the requirements of a single concept would show on a single page, and you don't have to walk |
@MaelRL Thanks for the reaction.
|
That's a shame :)
I'm not very sensible to this space argument: nobody is printing CGAL documentation and it becomes less readable on a single line. We already have "examples where this function/class appears" where it's on the same line, and I find it poor: It will be the same if you put all "see also" and concepts with longer names on the same line.
Do you mean a concept having many refinements? What I quoted is a real concept stack (https://doc.cgal.org/latest/Mesh_3/classMeshTriangulationTraits__3.html). If you meant, a page with refinement of concepts using inheritance, there is one here. |
|
We are apparently OK with the place that the "inherits" Section takes on class pages, and that is a lot more space than the "Refines" takes.
If you don't include the actual requirements on the page, you don't remove the fact that you need to click away and try to remember what you saw at previous concept levels. If you see look at https://doc.cgal.org/latest/Triangulation_3/classCGAL_1_1Regular__triangulation__3.html for example, it shows: which is it's base's base. And it makes sense, because if we only showed
Sure, like for class inheritance :>
Seems relevant to me because I'm arguing that we could fully harmonize presentation of "refinement" and presentation of "inheritance".
No, but it did not use any fancy trick: you just need to put an actual inheritance in the concept, and doxygen does the rest class Concept
: public ConceptBase
{ } |
Regarding the Note (and the mess in the 1.8.13 version), I'm not sure whether it is doxygen that creates the mess or that it is some postprocessing that is done by CGAL. Is there another place where this type of construct is used (Isosurfacing_3 is a new package and not in the "master" set present), I remember seeing it at other places as well but don't remember where. |
Are there any more comments / thoughts about this issue? |
So if I get it right, the conclusion is that people like the proposal except that we should not use |
I mean we want:
|
I never worked with concepts so I cannot really tell about the support and how it works, but in the doxygen 1.9.2 version there is the feature (see the doxygen changelog ): Regarding the summary in #7236 (comment) I'm going to work on it (probably this or next week). |
- Adjusted cgalRefines according to reviews - Implemented it in all files
Any comments or plans for integration? |
I just missed that it was ready for testing. I'll test it in the next batch |
Algebraic_kernel_d/doc/Algebraic_kernel_d/Concepts/AlgebraicKernel_d_2--MakeCoprime_2.h
Outdated
Show resolved
Hide resolved
Algebraic_kernel_d/doc/Algebraic_kernel_d/Concepts/AlgebraicKernel_d_1--MakeCoprime_1.h
Outdated
Show resolved
Hide resolved
Alpha_shapes_2/doc/Alpha_shapes_2/Concepts/WeightedAlphaShapeTraits_2.h
Outdated
Show resolved
Hide resolved
Algebraic_kernel_d/doc/Algebraic_kernel_d/Concepts/AlgebraicKernel_d_1--MakeCoprime_1.h
Outdated
Show resolved
Hide resolved
Kernel_d/doc/Kernel_d/Concepts/Kernel--ConstructCartesianConstIterator_d.h
Outdated
Show resolved
Hide resolved
Kernel_d/doc/Kernel_d/Concepts/Kernel--Construct_max_vertex_d.h
Outdated
Show resolved
Hide resolved
Kernel_d/doc/Kernel_d/Concepts/Kernel--Construct_min_vertex_d.h
Outdated
Show resolved
Hide resolved
@sloriot Thanks for the review, I posted a number of questions before I start updating. Some of the suggested changes I have to look into. |
In Voronoi_diagram_2/doc/Voronoi_diagram_2/Concepts/AdaptationTraits_2.h I found:
Should here also the |
Exactly |
Any suggestion for:
as found in Kernel_23/doc/Kernel_23/Concepts/FunctionObjectConcepts.h In the same file there are a number of occurences of:
what to do wit these? Furthermore there are the occurrences:
How to handle these? |
|
Honestly when we leave |
Adjusted after review - usage of `Adaptable...Function` instead of `AdapatableFunctor (with... arguments)` - corrected some incorrect / superfluous `}`
Corrected all mentions in the review above, ready for testing. |
Successfully tested in CGAL-5.6-Ic-223 |
Implemented:
2 arguments
->two arguments
(for consistency)\qualifier
Implemented in Kernel_23/doc/Kernel_23/Concepts/FunctionObjectConcepts.h Results a.o.
doc_output/Kernel_23/classKernel_1_1CartesianConstIterator__2.html

doc_output/Kernel_23/classKernel_1_1BoundedSide__3.html

Note: due to the fact that this is a test implementation pages outside the scope of Kernel_23/doc/Kernel_23/Concepts/FunctionObjectConcepts.h might not be correct (in respect to code appearance of refines and multiple refines)