[Sketcher] Fix angle constraint expression regression (#10743)

* fix 10742

* moved expression string modifier to helper function

* Tests: Add example tests for SketchObject

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* made expr modifier func static, pass by value and added unit tests

---------

Co-authored-by: Chris Hennes <chennes@gmail.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

src/Mod/Sketcher/App/SketchObject.cpp

@@ -660,25 +660,20 @@ int SketchObject::moveDatumsToEnd()
     return 0;
 }
 
-
 void SketchObject::reverseAngleConstraintToSupplementary(Constraint* constr, int constNum)
 {
     std::swap(constr->First, constr->Second);
     std::swap(constr->FirstPos, constr->SecondPos);
     constr->FirstPos = (constr->FirstPos == Sketcher::PointPos::start) ? Sketcher::PointPos::end : Sketcher::PointPos::start;
-    double actAngle = constr->getValue();
-    constr->setValue(M_PI - actAngle);
 
-    // Edit the expression if any
+    // Edit the expression if any, else modify constraint value directly
     if (constraintHasExpression(constNum)) {
         std::string expression = getConstraintExpression(constNum);
-        if (expression.substr(0, 7) == "180 - (") {
-            expression = expression.substr(7, expression.size() - 8);
-        }
-        else {
-            expression = "180 - (" + expression + ")";
-        }
-        setConstraintExpression(constNum, expression);
+        setConstraintExpression(constNum, reverseAngleConstraintExpression(expression));
+    }
+    else {
+        double actAngle = constr->getValue();
+        constr->setValue(M_PI - actAngle);
     }
 }
 
@@ -719,6 +714,30 @@ void SketchObject::setConstraintExpression(int constNum, const std::string& newE
     }
 }
 
+std::string SketchObject::reverseAngleConstraintExpression(std::string expression)
+{
+    // Check if expression contains units (°, deg, rad)
+    if (expression.find("°") != std::string::npos
+        || expression.find("deg") != std::string::npos
+        || expression.find("rad") != std::string::npos) {
+        if (expression.substr(0, 9) == "180 ° - ") {
+            expression = expression.substr(9, expression.size() - 9);
+        }
+        else {
+            expression = "180 ° - (" + expression + ")";
+        }
+    }
+    else {
+        if (expression.substr(0, 6) == "180 - ") {
+            expression = expression.substr(6, expression.size() - 6);
+        }
+        else {
+            expression = "180 - (" + expression + ")";
+        }
+    }
+    return expression;
+}
+
 int SketchObject::setVirtualSpace(int ConstrId, bool isinvirtualspace)
 {
     // no need to check input data validity as this is an sketchobject managed operation.

src/Mod/Sketcher/App/SketchObject.h

@@ -271,6 +271,8 @@ public:
 
     /// Change an angle constraint to its supplementary angle.
     void reverseAngleConstraintToSupplementary(Constraint* constr, int constNum);
+    /// Modify an angle constraint expression string to its supplementary angle
+    static std::string reverseAngleConstraintExpression(std::string expression);
 
     // Check if a constraint has an expression associated.
     bool constraintHasExpression(int constNum) const;

tests/src/Mod/Sketcher/App/SketchObject.cpp

@@ -6,6 +6,8 @@
 
 #include <App/Application.h>
 #include <App/Document.h>
+#include <App/Expression.h>
+#include <App/ObjectIdentifier.h>
 #include <Mod/Sketcher/App/GeoEnum.h>
 #include <Mod/Sketcher/App/SketchObject.h>
 
@@ -162,3 +164,96 @@ TEST_F(SketchObjectTest, testGeoIdFromShapeTypeRootPoint)
     EXPECT_EQ(geoId, Sketcher::GeoEnum::RtPnt);
     EXPECT_EQ(posId, Sketcher::PointPos::start);
 }
+
+TEST_F(SketchObjectTest, testReverseAngleConstraintToSupplementaryExpressionNoUnits1)
+{
+    std::string expr = Sketcher::SketchObject::reverseAngleConstraintExpression("180 - 60");
+    EXPECT_EQ(expr, std::string("60"));
+}
+
+TEST_F(SketchObjectTest, testReverseAngleConstraintToSupplementaryExpressionNoUnits2)
+{
+    std::string expr = Sketcher::SketchObject::reverseAngleConstraintExpression("60");
+    EXPECT_EQ(expr, std::string("180 - (60)"));
+}
+
+TEST_F(SketchObjectTest, testReverseAngleConstraintToSupplementaryExpressionWithUnits1)
+{
+    std::string expr = Sketcher::SketchObject::reverseAngleConstraintExpression("180 ° - 60 °");
+    EXPECT_EQ(expr, std::string("60 °"));
+}
+
+TEST_F(SketchObjectTest, testReverseAngleConstraintToSupplementaryExpressionWithUnits2)
+{
+    std::string expr = Sketcher::SketchObject::reverseAngleConstraintExpression("60 °");
+    EXPECT_EQ(expr, std::string("180 ° - (60 °)"));
+}
+
+TEST_F(SketchObjectTest, testReverseAngleConstraintToSupplementaryExpressionWithUnits3)
+{
+    std::string expr = Sketcher::SketchObject::reverseAngleConstraintExpression("60 deg");
+    EXPECT_EQ(expr, std::string("180 ° - (60 deg)"));
+}
+
+TEST_F(SketchObjectTest, testReverseAngleConstraintToSupplementaryExpressionWithUnits4)
+{
+    std::string expr = Sketcher::SketchObject::reverseAngleConstraintExpression("1rad");
+    EXPECT_EQ(expr, std::string("180 ° - (1rad)"));
+}
+
+TEST_F(SketchObjectTest, testReverseAngleConstraintToSupplementaryExpressionApplyAndReverse1)
+{
+    std::string expr = "180";
+    expr = Sketcher::SketchObject::reverseAngleConstraintExpression(expr);
+    expr = Sketcher::SketchObject::reverseAngleConstraintExpression(expr);
+    EXPECT_EQ(expr, std::string("(180)"));
+}
+
+TEST_F(SketchObjectTest, testReverseAngleConstraintToSupplementaryExpressionApplyAndReverse2)
+{
+    std::string expr = "(30 + 15) * 2 / 3";
+    expr = Sketcher::SketchObject::reverseAngleConstraintExpression(expr);
+    expr = Sketcher::SketchObject::reverseAngleConstraintExpression(expr);
+    EXPECT_EQ(expr, std::string("((30 + 15) * 2 / 3)"));
+}
+
+TEST_F(SketchObjectTest, testReverseAngleConstraintToSupplementaryExpressionSimple)
+{
+    // Arrange
+    auto constraint = new Sketcher::Constraint();  // Ownership will be transferred to the sketch
+    constraint->Type = Sketcher::ConstraintType::Angle;
+    auto id = getObject()->addConstraint(constraint);
+
+    App::ObjectIdentifier path(App::ObjectIdentifier::parse(getObject(), "Constraints[0]"));
+    std::shared_ptr<App::Expression> shared_expr(App::Expression::parse(getObject(), "0"));
+    getObject()->setExpression(path, shared_expr);
+
+    getObject()->setConstraintExpression(id, "180 - (60)");
+
+    // Act
+    getObject()->reverseAngleConstraintToSupplementary(constraint, id);
+
+    // Assert
+    EXPECT_EQ(std::string("60"), getObject()->getConstraintExpression(id));
+}
+
+TEST_F(SketchObjectTest, testReverseAngleConstraintToSupplementaryExpressionApplyAndReverse)
+{
+    // Arrange
+    auto constraint = new Sketcher::Constraint();  // Ownership will be transferred to the sketch
+    constraint->Type = Sketcher::ConstraintType::Angle;
+    auto id = getObject()->addConstraint(constraint);
+
+    App::ObjectIdentifier path(App::ObjectIdentifier::parse(getObject(), "Constraints[0]"));
+    std::shared_ptr<App::Expression> shared_expr(App::Expression::parse(getObject(), "0"));
+    getObject()->setExpression(path, shared_expr);
+
+    getObject()->setConstraintExpression(id, "32 °");
+
+    // Act
+    getObject()->reverseAngleConstraintToSupplementary(constraint, id);
+    getObject()->reverseAngleConstraintToSupplementary(constraint, id);
+
+    // Assert
+    EXPECT_EQ(std::string("32 °"), getObject()->getConstraintExpression(id));
+}