From 4dd445a60b62508210d941096faf1dc4965c1c4d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=89milie=20Feral?= <emilie.feral@numworks.com>
Date: Mon, 29 Jul 2019 11:04:47 +0200
Subject: [PATCH] [poincare] When beautifying, add missing user parentheses
 once both parent and child have been beautified (otherwise, the child might
 change and in a new expression that requires parentheses)

---
 poincare/include/poincare/factorial.h     |  2 --
 poincare/src/expression.cpp               |  7 ++++++-
 poincare/src/factorial.cpp                | 14 --------------
 poincare/src/multiplication_explicite.cpp | 11 +----------
 poincare/src/power.cpp                    |  6 ------
 5 files changed, 7 insertions(+), 33 deletions(-)

diff --git a/poincare/include/poincare/factorial.h b/poincare/include/poincare/factorial.h
index ae3e8a5db..f69eeb867 100644
--- a/poincare/include/poincare/factorial.h
+++ b/poincare/include/poincare/factorial.h
@@ -34,7 +34,6 @@ private:
   int serialize(char * buffer, int bufferSize, Preferences::PrintFloatMode floatDisplayMode, int numberOfSignificantDigits) const override;
   // Simplication
   Expression shallowReduce(ReductionContext reductionContext) override;
-  Expression shallowBeautify(ReductionContext reductionContext) override;
   // Evaluation
   template<typename T> static Complex<T> computeOnComplex(const std::complex<T> c, Preferences::ComplexFormat complexFormat, Preferences::AngleUnit angleUnit);
   Evaluation<float> approximate(SinglePrecision p, Context * context, Preferences::ComplexFormat complexFormat, Preferences::AngleUnit angleUnit) const override {
@@ -56,7 +55,6 @@ public:
   static Factorial Builder(Expression child) { return TreeHandle::FixedArityBuilder<Factorial, FactorialNode>(&child, 1); }
 
   Expression shallowReduce(ExpressionNode::ReductionContext reductionContext);
-  Expression shallowBeautify();
 private:
   constexpr static int k_maxOperandValue = 100;
 };
diff --git a/poincare/src/expression.cpp b/poincare/src/expression.cpp
index ee4ccba7e..a4d019dc8 100644
--- a/poincare/src/expression.cpp
+++ b/poincare/src/expression.cpp
@@ -609,7 +609,12 @@ Expression Expression::deepBeautify(ExpressionNode::ReductionContext reductionCo
   Expression e = shallowBeautify(reductionContext);
   int nbChildren = e.numberOfChildren();
   for (int i = 0; i < nbChildren; i++) {
-    e.childAtIndex(i).deepBeautify(reductionContext);
+    Expression child = e.childAtIndex(i);
+    child.deepBeautify(reductionContext);
+    // We add missing Parentheses after beautifying the parent and child
+    if (e.node()->childNeedsUserParentheses(child)) {
+      e.replaceChildAtIndexInPlace(i, Parenthesis::Builder(child));
+    }
   }
   return e;
 }
diff --git a/poincare/src/factorial.cpp b/poincare/src/factorial.cpp
index 4c3ddc598..f8e63fd8f 100644
--- a/poincare/src/factorial.cpp
+++ b/poincare/src/factorial.cpp
@@ -48,10 +48,6 @@ Expression FactorialNode::shallowReduce(ReductionContext reductionContext) {
   return Factorial(this).shallowReduce(reductionContext);
 }
 
-Expression FactorialNode::shallowBeautify(ReductionContext reductionContext) {
-  return Factorial(this).shallowBeautify();
-}
-
 template<typename T>
 Complex<T> FactorialNode::computeOnComplex(const std::complex<T> c, Preferences::ComplexFormat, Preferences::AngleUnit angleUnit) {
   T n = c.real();
@@ -118,16 +114,6 @@ Expression Factorial::shallowReduce(ExpressionNode::ReductionContext reductionCo
   return *this;
 }
 
-Expression Factorial::shallowBeautify() {
-  // +(a,b)! ->(+(a,b))!
-  if (node()->childNeedsUserParentheses(childAtIndex(0))) {
-    Expression result = Factorial::Builder(Parenthesis::Builder(childAtIndex(0)));
-    replaceWithInPlace(result);
-    return result;
-  }
-  return *this;
-}
-
 #if 0
 int Factorial::simplificationOrderGreaterType(const Expression * e) const {
   if (SimplificationOrder(childAtIndex(0),e) == 0) {
diff --git a/poincare/src/multiplication_explicite.cpp b/poincare/src/multiplication_explicite.cpp
index e9cda1ad5..83516eb25 100644
--- a/poincare/src/multiplication_explicite.cpp
+++ b/poincare/src/multiplication_explicite.cpp
@@ -90,16 +90,7 @@ Expression MultiplicationExplicite::shallowBeautify(ExpressionNode::ReductionCon
   }
   assert(thisExp.type() == ExpressionNode::Type::MultiplicationExplicite);
 
-  // Step 3: Add Parenthesis if needed
-  for (int i = 0; i < thisExp.numberOfChildren(); i++) {
-    const Expression o = thisExp.childAtIndex(i);
-    if (o.type() == ExpressionNode::Type::Addition) {
-      Parenthesis p = Parenthesis::Builder(o);
-      thisExp.replaceChildAtIndexInPlace(i, p);
-    }
-  }
-
-  // Step 4: Create a Division if needed
+  // Step 3: Create a Division if needed
   for (int i = 0; i < numberOfChildren(); i++) {
     Expression childI = thisExp.childAtIndex(i);
     if (!(childI.type() == ExpressionNode::Type::Power && childI.childAtIndex(1).type() == ExpressionNode::Type::Rational && childI.childAtIndex(1).convert<Rational>().isMinusOne())) {
diff --git a/poincare/src/power.cpp b/poincare/src/power.cpp
index 71a18a95d..e91513c87 100644
--- a/poincare/src/power.cpp
+++ b/poincare/src/power.cpp
@@ -865,12 +865,6 @@ Expression Power::shallowBeautify(ExpressionNode::ReductionContext reductionCont
     return result;
   }
 
-  // Step 4: +(a,b)^c ->(+(a,b))^c and *(a,b)^c ->(*(a,b))^c
-  if (node()->childNeedsUserParentheses(childAtIndex(0)))
-  {
-    Parenthesis p = Parenthesis::Builder(childAtIndex(0));
-    replaceChildAtIndexInPlace(0, p);
-  }
   return *this;
 }