(core) Ignore leading whitespace in formulas, and strip out leading '=' sign users might add

Summary:
This addresses two issues, differently:
- For a formula with leading whitespace, like " 1+1", it is stored as is, but
  is fixed to work (it should be valid Python, and whitespace is only stripped out
  at parsing time to avoid intentation errors caused by the way it gets parsed)
- For a formula with a leading equals-sign ("="), it is stripped out on the
  client side before the formula is stored. Grist documentation uses leading
  "=" to indicate formulas (because UI shows an "=" icon), and Excel formulas
  actually contain the leading "=", so it is a common mistake to include it.

Test Plan: Added new test cases

Reviewers: jarek

Reviewed By: jarek

Differential Revision: https://phab.getgrist.com/D3873
This commit is contained in:
Dmitry S 2023-04-24 23:01:26 -04:00
parent d6abe6a737
commit b4cc519616
6 changed files with 42 additions and 8 deletions

View File

@ -175,7 +175,10 @@ export class FormulaEditor extends NewBaseEditor {
} }
public getCellValue() { public getCellValue() {
return this._formulaEditor.getValue(); const value = this._formulaEditor.getValue();
// Strip the leading "=" sign, if any, in case users think it should start the formula body (as
// it does in Excel, and because the equal sign is also used for formulas in Grist UI).
return (value[0] === '=') ? value.slice(1) : value;
} }
public getTextValue() { public getTextValue() {

View File

@ -3,10 +3,11 @@ import contextlib
import itertools import itertools
import linecache import linecache
import re import re
import six import textwrap
import astroid import astroid
import asttokens import asttokens
import six
import friendly_errors import friendly_errors
import textbuilder import textbuilder
@ -41,6 +42,11 @@ def make_formula_body(formula, default_value, assoc_value=None):
if isinstance(formula, six.binary_type): if isinstance(formula, six.binary_type):
formula = formula.decode('utf8') formula = formula.decode('utf8')
# Remove any common leading whitespace. In python, extra indent should not be an error, but
# it is in Grist because we parse the formula body before it gets inserted into a function (i.e.
# as if at module level).
formula = textwrap.dedent(formula)
if not formula.strip(): if not formula.strip():
return textbuilder.Text('return ' + repr(default_value), assoc_value) return textbuilder.Text('return ' + repr(default_value), assoc_value)

View File

@ -1,15 +1,14 @@
# -*- coding: utf-8 -*- # -*- coding: utf-8 -*-
import unittest
import codebuilder import codebuilder
import six import six
import test_engine
unicode_prefix = 'u' if six.PY2 else '' unicode_prefix = 'u' if six.PY2 else ''
def make_body(formula, default=None): def make_body(formula, default=None):
return codebuilder.make_formula_body(formula, default).get_text() return codebuilder.make_formula_body(formula, default).get_text()
class TestCodeBuilder(unittest.TestCase): class TestCodeBuilder(test_engine.EngineTestCase):
def test_make_formula_body(self): def test_make_formula_body(self):
# Test simple usage. # Test simple usage.
self.assertEqual(make_body(""), "return None") self.assertEqual(make_body(""), "return None")
@ -239,3 +238,19 @@ return x or y
# Check that missing arguments is OK # Check that missing arguments is OK
self.assertEqual(make_body("ISERR()"), "return ISERR()") self.assertEqual(make_body("ISERR()"), "return ISERR()")
def test_leading_whitespace(self):
self.assertEqual(make_body(" $A + 1"), "return rec.A + 1")
self.assertEqual(make_body("""
if $A:
return $A
$B
"""), """
if rec.A:
return rec.A
return rec.B
""")

View File

@ -25,6 +25,13 @@ else:
""" """
if sum(3, 5) > 6: if sum(3, 5) > 6:
return 6 return 6
return 0
"""
other_err = \
"""
if sum(3, 5) > 6:
return 6
""" """
sample = testutil.parse_test_sample({ sample = testutil.parse_test_sample({
@ -34,7 +41,7 @@ else:
[12, "built_in_formula", "Text", True, "max(5)", "", ""], [12, "built_in_formula", "Text", True, "max(5)", "", ""],
[13, "syntax_err", "Text", True, syntax_err, "", ""], [13, "syntax_err", "Text", True, syntax_err, "", ""],
[14, "indent_err", "Text", True, indent_err, "", ""], [14, "indent_err", "Text", True, indent_err, "", ""],
[15, "other_err", "Text", True, textwrap.dedent(indent_err), "", ""], [15, "other_err", "Text", True, other_err, "", ""],
[15, "custom_err", "Text", True, "raise Exception('hello'); return 1", "", ""], [15, "custom_err", "Text", True, "raise Exception('hello'); return 1", "", ""],
]] ]]
], ],

View File

@ -16,7 +16,7 @@ import actions
import column import column
import sort_specs import sort_specs
import identifiers import identifiers
from objtypes import strict_equal, encode_object, decode_object from objtypes import strict_equal, encode_object
import schema import schema
from schema import RecalcWhen from schema import RecalcWhen
import summary import summary

View File

@ -614,6 +614,9 @@ export async function enterCell(...keys: string[]) {
/** /**
* Enter a formula into the currently selected cell. * Enter a formula into the currently selected cell.
*
* You can insert newlines by embedding `${Key.chord(Key.SHIFT, Key.ENTER)}` into the formula
* text. Note that ACE editor adds some indentation automatically.
*/ */
export async function enterFormula(formula: string) { export async function enterFormula(formula: string) {
await driver.sendKeys('='); await driver.sendKeys('=');
@ -621,7 +624,7 @@ export async function enterFormula(formula: string) {
if (await driver.find('.test-editor-tooltip-convert').isPresent()) { if (await driver.find('.test-editor-tooltip-convert').isPresent()) {
await driver.find('.test-editor-tooltip-convert').click(); await driver.find('.test-editor-tooltip-convert').click();
} }
await driver.sendKeys(formula, Key.ENTER); await sendKeys(formula, Key.ENTER);
await waitForServer(); await waitForServer();
} }