pcl icon indicating copy to clipboard operation
pcl copied to clipboard

[WIP] Automatically fix doxygen commands messed up by clang-format

Open kunaltyagi opened this issue 3 years ago • 6 comments

kunaltyagi avatar Dec 16 '21 07:12 kunaltyagi

This works, but there's tons of work to be done on this to make it usable.

However, thoughts?


I used this as:

python doxygen_fixer.py test.cpp
clang-format test.cpp

kunaltyagi avatar Dec 16 '21 07:12 kunaltyagi

To avoid having clang-format breaking the doxygen in the first place, maybe these could be added to our configuration:

https://github.com/dealii/dealii/blob/63ea9d02bfd72eb19c50bfa36c2ae1b1e68a5262/.clang-format#L159

I'm not that familiar with either tools, so I haven't tested this.

larshg avatar Dec 16 '21 08:12 larshg

The list of doxygen commands is a bit long. And PCL uses both @command and \command styles. The CommentPragmas line would be a monster. I could create a separate PR for that

kunaltyagi avatar Dec 17 '21 05:12 kunaltyagi

Can you add an example to the description, where clang-format is messing up commands? Is it to forced line breaks?

The list of doxygen commands is a bit long. And PCL uses both @command and \command styles. The CommentPragmas line would be a monster. I could create a separate PR for that

Or maybe just change all \command to @command to be consistent (or the other way) ;-)

SunBlack avatar Dec 21 '21 12:12 SunBlack

Tried this script now, but it seems not perfect until now:

python3 .dev/doxygen_fixer.py apps/in_hand_scanner/include/pcl/apps/in_hand_scanner/icp.h
clang-format-11 -i apps/in_hand_scanner/include/pcl/apps/in_hand_scanner/icp.h

(Note: Ubuntu 21.10 provied Clang-format 9, 11, 12 & 13, but not 10, which we are using within the PCL^^)

This results in:

diff --git a/apps/in_hand_scanner/include/pcl/apps/in_hand_scanner/icp.h b/apps/in_hand_scanner/include/pcl/apps/in_hand_scanner/icp.h
index 23d40ff3a..07f2008cc 100644
--- a/apps/in_hand_scanner/include/pcl/apps/in_hand_scanner/icp.h
+++ b/apps/in_hand_scanner/include/pcl/apps/in_hand_scanner/icp.h
@@ -11,15 +11,15 @@
  * modification, are permitted provided that the following conditions
  * are met:
  *
- *  * Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer.
- *  * Redistributions in binary form must reproduce the above
- *    copyright notice, this list of conditions and the following
- *    disclaimer in the documentation and/or other materials provided
- *    with the distribution.
- *  * Neither the name of the copyright holder(s) nor the names of its
- *    contributors may be used to endorse or promote products derived
- *    from this software without specific prior written permission.
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials provided
+ * with the distribution.
+ * * Neither the name of the copyright holder(s) nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
  *
  * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
  * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
@@ -71,7 +71,8 @@ public:
   /** @{ */
   /** \brief Convergence is detected when the change of the fitness between the current
    * and previous iteration becomes smaller than the given epsilon (set in cm^2). The
-   * fitness is the mean squared euclidean distance between corresponding points. \note
+   * fitness is the mean squared euclidean distance between corresponding points.
+   * \note
    * Only accepted if it is greater than 0.
    */
   void
@@ -83,7 +84,8 @@ public:
 
   /** @{ */
   /** \brief The registration fails if the number of iterations exceeds the maximum
-   * number of iterations. \note Must be greater than 0. Smaller values are set to 1.
+   * number of iterations.
+   * \note Must be greater than 0. Smaller values are set to 1.
    */
   void
   setMaxIterations(const unsigned int max_iter);
@@ -108,7 +110,8 @@ public:
 
   /** @{ */
   /** \brief The registration fails at the state of convergence if the fitness is bigger
-   * than this threshold (set in cm^2) \note Must be greater than zero.
+   * than this threshold (set in cm^2)
+   * \note Must be greater than zero.
    */
   void
   setMaxFitness(const float fitness);
@@ -121,7 +124,8 @@ public:
   /** \brief Correspondences are rejected if the squared distance is above a threshold.
    * This threshold is initialized with infinity (all correspondences are accepted in
    * the first iteration). The threshold of the next iterations is set to the fitness of
-   * the current iteration multiplied by the factor set by this method. \note Must be
+   * the current iteration multiplied by the factor set by this method.
+   * \note Must be
    * greater or equal one. Smaller values are set to one.
    */
   void
@@ -133,7 +137,8 @@ public:
 
   /** @{ */
   /** \brief Correspondences are rejected if the angle between the normals is bigger
-   * than this threshold. Set in degrees. \note Must be between 180 degrees and 0.
+   * than this threshold. Set in degrees.
+   * \note Must be between 180 degrees and 0.
    * Values outside this range are clamped to the nearest valid value.
    */
   void
@@ -168,14 +173,16 @@ private:
   /** \brief Selects the model points that are pointing towards to the camera (data
    * coordinate system = camera coordinate system). \param[in] mesh_model Input mesh.
    * \param[in] T_inv Transformation that brings the model mesh into the camera
-   * coordinate system. \return Cloud containing the selected points (the connectivity
+   * coordinate system.
+   * \return Cloud containing the selected points (the connectivity
    * information of the mesh is currently not used during the registration).
    */
   CloudNormalConstPtr
   selectModelPoints(const MeshConstPtr& mesh_model, const Eigen::Matrix4f& T_inv) const;
 
   /** \brief Selects the valid data points. The input cloud is organized -> contains
-   * nans which are removed \param[in] cloud_data Input cloud. \return Cloud containing
+   * nans which are removed \param[in] cloud_data Input cloud.
+   * \return Cloud containing
    * the selected points.
    */
   CloudNormalConstPtr
@@ -186,7 +193,8 @@ private:
    * one correspondences (point 0 in source corresponds to point 0 in target, point 1 in
    * source to point 1 in target and so on). \param[in] cloud_source Source cloud
    * (data). \param[in] cloud_target Target cloud (model). \param[out] T The computed
-   * transformation. \return true if success
+   * transformation.
+   * \return true if success
    */
   bool
   minimizePointPlane(const CloudNormal& cloud_source,

As you can see: e.g the copyright is modified, Sometime \note is now allone on a line, ...

On another file: \param[in] cloud_data Input cloud. is not put to a separate line

SunBlack avatar Jan 03 '22 14:01 SunBlack

Due to the max line length sth. like this could happen:

  /** \brief Returns true if the distance between the three points is below a threshold.
   */

This looks a bit ugly.

Maybe we should think about changing the style to

  /**
   * \brief Returns true if the distance between the three points is below a threshold.
   */

Even it means we have one more line.

SunBlack avatar Jan 03 '22 14:01 SunBlack