From b4cc5196161bf64f2469cb04cb13630ed0ba560a Mon Sep 17 00:00:00 2001 From: Dmitry S Date: Mon, 24 Apr 2023 23:01:26 -0400 Subject: [PATCH] (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 --- app/client/widgets/FormulaEditor.ts | 5 ++++- sandbox/grist/codebuilder.py | 8 +++++++- sandbox/grist/test_codebuilder.py | 21 ++++++++++++++++++--- sandbox/grist/test_formula_error.py | 9 ++++++++- sandbox/grist/useractions.py | 2 +- test/nbrowser/gristUtils.ts | 5 ++++- 6 files changed, 42 insertions(+), 8 deletions(-) diff --git a/app/client/widgets/FormulaEditor.ts b/app/client/widgets/FormulaEditor.ts index 4968a572..1f595168 100644 --- a/app/client/widgets/FormulaEditor.ts +++ b/app/client/widgets/FormulaEditor.ts @@ -175,7 +175,10 @@ export class FormulaEditor extends NewBaseEditor { } 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() { diff --git a/sandbox/grist/codebuilder.py b/sandbox/grist/codebuilder.py index bcd8086d..30224a0a 100644 --- a/sandbox/grist/codebuilder.py +++ b/sandbox/grist/codebuilder.py @@ -3,10 +3,11 @@ import contextlib import itertools import linecache import re -import six +import textwrap import astroid import asttokens +import six import friendly_errors import textbuilder @@ -41,6 +42,11 @@ def make_formula_body(formula, default_value, assoc_value=None): if isinstance(formula, six.binary_type): 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(): return textbuilder.Text('return ' + repr(default_value), assoc_value) diff --git a/sandbox/grist/test_codebuilder.py b/sandbox/grist/test_codebuilder.py index 2ce55fca..27c6422d 100644 --- a/sandbox/grist/test_codebuilder.py +++ b/sandbox/grist/test_codebuilder.py @@ -1,15 +1,14 @@ # -*- coding: utf-8 -*- -import unittest - import codebuilder import six +import test_engine unicode_prefix = 'u' if six.PY2 else '' def make_body(formula, default=None): return codebuilder.make_formula_body(formula, default).get_text() -class TestCodeBuilder(unittest.TestCase): +class TestCodeBuilder(test_engine.EngineTestCase): def test_make_formula_body(self): # Test simple usage. self.assertEqual(make_body(""), "return None") @@ -239,3 +238,19 @@ return x or y # Check that missing arguments is OK 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 +""") diff --git a/sandbox/grist/test_formula_error.py b/sandbox/grist/test_formula_error.py index 206c7505..983dbb4b 100644 --- a/sandbox/grist/test_formula_error.py +++ b/sandbox/grist/test_formula_error.py @@ -25,6 +25,13 @@ else: """ if sum(3, 5) > 6: return 6 +return 0 +""" + + other_err = \ +""" + if sum(3, 5) > 6: + return 6 """ sample = testutil.parse_test_sample({ @@ -34,7 +41,7 @@ else: [12, "built_in_formula", "Text", True, "max(5)", "", ""], [13, "syntax_err", "Text", True, syntax_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", "", ""], ]] ], diff --git a/sandbox/grist/useractions.py b/sandbox/grist/useractions.py index 4f387fd6..00c94fbc 100644 --- a/sandbox/grist/useractions.py +++ b/sandbox/grist/useractions.py @@ -16,7 +16,7 @@ import actions import column import sort_specs import identifiers -from objtypes import strict_equal, encode_object, decode_object +from objtypes import strict_equal, encode_object import schema from schema import RecalcWhen import summary diff --git a/test/nbrowser/gristUtils.ts b/test/nbrowser/gristUtils.ts index a9babd48..a1f060bb 100644 --- a/test/nbrowser/gristUtils.ts +++ b/test/nbrowser/gristUtils.ts @@ -614,6 +614,9 @@ export async function enterCell(...keys: string[]) { /** * 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) { await driver.sendKeys('='); @@ -621,7 +624,7 @@ export async function enterFormula(formula: string) { if (await driver.find('.test-editor-tooltip-convert').isPresent()) { await driver.find('.test-editor-tooltip-convert').click(); } - await driver.sendKeys(formula, Key.ENTER); + await sendKeys(formula, Key.ENTER); await waitForServer(); }