Skip to content

Commit

Permalink
Merge pull request #4215 from hyarion/feature-spreadsheet-equal-prefi…
Browse files Browse the repository at this point in the history 
…x-for-expressions

[Spreadsheet] Only evaluate cell values when prefixed with '='
  • Loading branch information
@yorikvanhavre
yorikvanhavre committed Feb 5, 2021
2 parents 21dc72e + 24c6183 commit aa52a3f
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 32 deletions.
101 changes: 69 additions & 32 deletions src/Mod/Spreadsheet/App/Cell.cpp
View file
Expand Up @@ -38,9 +38,11 @@
#include <Base/UnitsApi.h>
#include <Base/Writer.h>
#include <Base/Console.h>
#include <Base/StdStlTools.h>
#include <App/ExpressionParser.h>
#include "Sheet.h"
#include <iomanip>
#include <cctype>

FC_LOG_LEVEL_INIT("Spreadsheet",true,true)

Expand Down Expand Up @@ -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<App::StringExpression>(owner->sheet(), value);
setParseException(e.what());
}
}
else if (*value == '\'') {
expr = new App::StringExpression(owner->sheet(), value + 1);
newExpr = std::make_unique<App::StringExpression>(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<App::NumberExpression>(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<OperatorExpression>(parsedExpr.get())) {
if (fraction->getOperator() == OperatorExpression::UNIT) {
const auto left = freecad_dynamic_cast<NumberExpression>(fraction->getLeft());
const auto right = freecad_dynamic_cast<UnitExpression>(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<NumberExpression>(fraction->getLeft());
const bool isNumberDenom = freecad_dynamic_cast<NumberExpression>(fraction->getRight());

// check for numbers with units in (de)nominator
const auto opNom = freecad_dynamic_cast<OperatorExpression>(fraction->getLeft());
const auto opDenom = freecad_dynamic_cast<OperatorExpression>(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<UnitExpression>(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<NumberExpression>(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<App::StringExpression>(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();
}

/**
Expand Down Expand Up @@ -1027,4 +1065,3 @@ std::string Cell::getFormattedQuantity(void)
return result;
}


27 changes: 27 additions & 0 deletions src/Mod/Spreadsheet/TestSpreadsheet.py
View file
Expand Up @@ -668,6 +668,23 @@ def testNumbers(self):
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')
Expand Down Expand Up @@ -1034,6 +1051,16 @@ def testIssue3432(self):
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')
Expand Down

0 comments on commit aa52a3f

Please sign in to comment.