diff --git a/src/Mod/Spreadsheet/App/Cell.cpp b/src/Mod/Spreadsheet/App/Cell.cpp
index 1f01fdb477..749f5835ee 100644
--- a/src/Mod/Spreadsheet/App/Cell.cpp
+++ b/src/Mod/Spreadsheet/App/Cell.cpp
@@ -38,9 +38,11 @@
#include
#include
#include
+#include
#include
#include "Sheet.h"
#include
+#include
FC_LOG_LEVEL_INIT("Spreadsheet",true,true)
@@ -279,62 +281,98 @@ void Cell::afterRestore() {
void Cell::setContent(const char * value)
{
PropertySheet::AtomicPropertyChange signaller(*owner);
- App::Expression * expr = 0;
+ ExpressionPtr newExpr;
clearException();
- if (value != 0) {
- if(owner->sheet()->isRestoring()) {
- expression.reset(new App::StringExpression(owner->sheet(),value));
+ if (value) {
+ if (owner->sheet()->isRestoring()) {
+ expression.reset(new App::StringExpression(owner->sheet(), value));
setUsed(EXPRESSION_SET, true);
return;
}
if (*value == '=') {
try {
- expr = App::ExpressionParser::parse(owner->sheet(), value + 1);
+ newExpr = ExpressionPtr(App::ExpressionParser::parse(owner->sheet(), value + 1));
}
catch (Base::Exception & e) {
- expr = new App::StringExpression(owner->sheet(), value);
+ newExpr = std::make_unique(owner->sheet(), value);
setParseException(e.what());
}
}
else if (*value == '\'') {
- expr = new App::StringExpression(owner->sheet(), value + 1);
+ newExpr = std::make_unique(owner->sheet(), value + 1);
}
else if (*value != '\0') {
+ // check if value is just a number
char * end;
errno = 0;
- double float_value = strtod(value, &end);
- if (!*end && errno == 0) {
- expr = new App::NumberExpression(owner->sheet(), Quantity(float_value));
+ const double float_value = strtod(value, &end);
+ if (errno == 0) {
+ const bool isEndEmpty = *end == '\0' || strspn(end, " \t\n\r") == strlen(end);
+ if (isEndEmpty) {
+ newExpr = std::make_unique(owner->sheet(), Quantity(float_value));
+ }
}
- else {
+
+ // if not a float, check if it is a quantity or compatible fraction
+ const bool isStartingWithNumber = value != end;
+ if (!newExpr && isStartingWithNumber) {
try {
- expr = ExpressionParser::parse(owner->sheet(), value);
- if (expr)
- delete expr->eval();
- }
- catch (Base::Exception &) {
- expr = new App::StringExpression(owner->sheet(), value);
+ ExpressionPtr parsedExpr(App::ExpressionParser::parse(owner->sheet(), value));
+
+ if (const auto fraction = freecad_dynamic_cast(parsedExpr.get())) {
+ if (fraction->getOperator() == OperatorExpression::UNIT) {
+ const auto left = freecad_dynamic_cast(fraction->getLeft());
+ const auto right = freecad_dynamic_cast(fraction->getRight());
+ if (left && right) {
+ newExpr = std::move(parsedExpr);
+ }
+ }
+ else if (fraction->getOperator() == OperatorExpression::DIV) {
+ // only the following types of fractions are ok:
+ // 1/2, 1m/2, 1/2s, 1m/2s, 1/m
+
+ // check for numbers in (de)nominator
+ const bool isNumberNom = freecad_dynamic_cast(fraction->getLeft());
+ const bool isNumberDenom = freecad_dynamic_cast(fraction->getRight());
+
+ // check for numbers with units in (de)nominator
+ const auto opNom = freecad_dynamic_cast(fraction->getLeft());
+ const auto opDenom = freecad_dynamic_cast(fraction->getRight());
+ const bool isQuantityNom = opNom && opNom->getOperator() == OperatorExpression::UNIT;
+ const bool isQuantityDenom = opDenom && opDenom->getOperator() == OperatorExpression::UNIT;
+
+ // check for units in denomainator
+ const auto uDenom = freecad_dynamic_cast(fraction->getRight());
+ const bool isUnitDenom = uDenom && uDenom->getTypeId() == UnitExpression::getClassTypeId();
+
+ const bool isNomValid = isNumberNom || isQuantityNom;
+ const bool isDenomValid = isNumberDenom || isQuantityDenom || isUnitDenom;
+ if (isNomValid && isDenomValid) {
+ newExpr = std::move(parsedExpr);
+ }
+ }
+ }
+ else if (const auto number = freecad_dynamic_cast(parsedExpr.get())) {
+ // NumbersExpressions can accept more than can be parsed with strtod.
+ // Example: 12.34 and 12,34 are both valid NumberExpressions
+ newExpr = std::move(parsedExpr);
+ }
}
+ catch (...) {}
}
}
- }
- try {
- setExpression(App::ExpressionPtr(expr));
- signaller.tryInvoke();
- }
- catch (Base::Exception &e) {
- if (value) {
- std::string _value = value;
- if (_value[0] != '=') {
- _value.insert (0, 1, '=');
- }
-
- setExpression(App::ExpressionPtr(new App::StringExpression(owner->sheet(), _value)));
- setParseException(e.what());
+ if (!newExpr && *value != '\0') {
+ newExpr = std::make_unique(owner->sheet(), value);
}
+
+ // trying to add an empty string will make newExpr = nullptr
}
+
+ // set expression, or delete the current expression by setting nullptr if empty string was entered
+ setExpression(std::move(newExpr));
+ signaller.tryInvoke();
}
/**
@@ -1027,4 +1065,3 @@ std::string Cell::getFormattedQuantity(void)
return result;
}
-
diff --git a/src/Mod/Spreadsheet/TestSpreadsheet.py b/src/Mod/Spreadsheet/TestSpreadsheet.py
index acc803993c..157c57d9a7 100644
--- a/src/Mod/Spreadsheet/TestSpreadsheet.py
+++ b/src/Mod/Spreadsheet/TestSpreadsheet.py
@@ -668,6 +668,23 @@ class SpreadsheetCases(unittest.TestCase):
self.assertEqual(sheet.A17, 0.5)
self.assertEqual(sheet.A18, 0.5)
+ def testQuantitiesAndFractionsAsNumbers(self):
+ """ Test quantities and simple fractions as numbers """
+ sheet = self.doc.addObject('Spreadsheet::Sheet','Spreadsheet')
+ sheet.set('A1', '1mm')
+ sheet.set('A2', '1/2')
+ sheet.set('A3', '4mm/2')
+ sheet.set('A4', '2/mm')
+ sheet.set('A5', '4/2mm')
+ sheet.set('A6', '6mm/3s')
+ self.doc.recompute()
+ self.assertEqual(sheet.A1, Units.Quantity('1 mm'))
+ self.assertEqual(sheet.A2, 0.5)
+ self.assertEqual(sheet.A3, Units.Quantity('2 mm'))
+ self.assertEqual(sheet.A4, Units.Quantity('2 1/mm'))
+ self.assertEqual(sheet.A5, Units.Quantity('2 1/mm'))
+ self.assertEqual(sheet.A6, Units.Quantity('2 mm/s'))
+
def testRemoveRows(self):
""" Removing rows -- check renaming of internal cells """
sheet = self.doc.addObject('Spreadsheet::Sheet','Spreadsheet')
@@ -1034,6 +1051,16 @@ class SpreadsheetCases(unittest.TestCase):
self.doc.recompute()
self.assertEqual(sheet.get('C1'), Units.Quantity('3 mm'))
+ def testIssue4156(self):
+ """ Regression test for issue 4156; necessarily use of leading '=' to enter an expression, creates inconsistent behavior depending on the spreadsheet state"""
+ sheet = self.doc.addObject('Spreadsheet::Sheet','Spreadsheet')
+ sheet.set('A3', 'A1')
+ sheet.set('A1', '1000')
+ self.doc.recompute()
+ sheet.set('A3', '')
+ sheet.set('A3', 'A1')
+ self.assertEqual(sheet.getContents('A3'), 'A1')
+
def testInsertRowsAlias(self):
""" Regression test for issue 4429; insert rows to sheet with aliases"""
sheet = self.doc.addObject('Spreadsheet::Sheet','Spreadsheet')