Make number parsing agnostic to decimal and thousands separators (#894) (#1029)

packages/desktop-client/e2e/page-models/account-page.js

@@ -91,6 +91,12 @@ export class AccountPage {
     return new CloseAccountModal(this.page.locator('css=[aria-modal]'));
   }
 
+  async _clearFocusedField() {
+    let isMac = process.platform === 'darwin';
+    await this.page.keyboard.press(isMac ? 'Meta+A' : 'Control+A');
+    await this.page.keyboard.press('Backspace');
+  }
+
   async _fillTransactionFields(transactionRow, transaction) {
     if (transaction.payee) {
       await transactionRow.getByTestId('payee').click();
@@ -117,12 +123,14 @@ export class AccountPage {
 
     if (transaction.debit) {
       await transactionRow.getByTestId('debit').click();
+      await this._clearFocusedField();
       await this.page.keyboard.type(transaction.debit);
       await this.page.keyboard.press('Tab');
     }
 
     if (transaction.credit) {
       await transactionRow.getByTestId('credit').click();
+      await this._clearFocusedField();
       await this.page.keyboard.type(transaction.credit);
       await this.page.keyboard.press('Tab');
     }

packages/loot-core/src/mocks/number-formats.ts

+export function generateTestCases(
+  configurableFormats: string[],
+  inputFormats: Array<{
+    name: string;
+    tests: Array<{ places: number; input: string; expected: number }>;
+  }>,
+) {
+  let cases = [];
+
+  for (let configurableFormat of configurableFormats) {
+    for (let inputFormat of inputFormats) {
+      for (let test of inputFormat.tests) {
+        cases.push([
+          configurableFormat,
+          inputFormat.name,
+          test.places,
+          test.input,
+          test.expected,
+        ]);
+      }
+    }
+  }
+
+  return cases;
+}

packages/loot-core/src/shared/arithmetic.test.ts

+import { generateTestCases } from '../mocks/number-formats';
+
 import evalArithmetic from './arithmetic';
+import { setNumberFormat } from './util';
 
 describe('arithmetic', () => {
   test('handles negative numbers', () => {
@@ -41,4 +44,76 @@ describe('arithmetic', () => {
   test('respects current number format', () => {
     expect(evalArithmetic('1,222.45')).toEqual(1222.45);
   });
+
+  let configurableFormats = [
+    'dot-comma',
+    'comma-dot',
+    'space-comma',
+    'space-dot',
+  ];
+
+  let inputFormats = [
+    {
+      name: 'dot-comma',
+      tests: [
+        { places: 3, input: '1.234.567', expected: 1234567 },
+        { places: 2, input: '1.234,56', expected: 1234.56 },
+        { places: 1, input: '1.234,5', expected: 1234.5 },
+        { places: 0, input: '1.234,', expected: 1234.0 },
+      ],
+    },
+    {
+      name: 'comma-dot',
+      tests: [
+        { places: 3, input: '1,234,567', expected: 1234567 },
+        { places: 2, input: '1,234.56', expected: 1234.56 },
+        { places: 1, input: '1,234.5', expected: 1234.5 },
+        { places: 0, input: '1,234.', expected: 1234.0 },
+      ],
+    },
+    {
+      name: 'dot-dot',
+      tests: [
+        { places: 3, input: '1.234.567', expected: 1234567 },
+        { places: 2, input: '1.234.56', expected: 1234.56 },
+        { places: 1, input: '1.234.5', expected: 1234.5 },
+        { places: 0, input: '1.234.', expected: 1234.0 },
+      ],
+    },
+    {
+      name: 'comma-comma',
+      tests: [
+        { places: 3, input: '1,234,567', expected: 1234567 },
+        { places: 2, input: '1,234,56', expected: 1234.56 },
+        { places: 1, input: '1,234,5', expected: 1234.5 },
+        { places: 0, input: '1,234,', expected: 1234.0 },
+      ],
+    },
+    {
+      name: 'space-comma',
+      tests: [
+        { places: 3, input: '1 234 567', expected: 1234567 },
+        { places: 2, input: '1 234,56', expected: 1234.56 },
+        { places: 1, input: '1 234,5', expected: 1234.5 },
+        { places: 0, input: '1 234,', expected: 1234.0 },
+      ],
+    },
+    {
+      name: 'space-dot',
+      tests: [
+        { places: 3, input: '1 234 567', expected: 1234567 },
+        { places: 2, input: '1 234.56', expected: 1234.56 },
+        { places: 1, input: '1 234.5', expected: 1234.5 },
+        { places: 0, input: '1 234.', expected: 1234.0 },
+      ],
+    },
+  ];
+
+  test.each(generateTestCases(configurableFormats, inputFormats))(
+    'format is agnostic: %s can parse %s with %d decimal place(s)',
+    (configurableFormat, inputFormat, places, input, expected) => {
+      setNumberFormat({ format: configurableFormat, hideFraction: false });
+      expect(evalArithmetic(input)).toEqual(expected);
+    },
+  );
 });

packages/loot-core/src/shared/arithmetic.ts

-import { getNumberFormat } from './util';
-
 function fail(state, msg) {
   throw new Error(
     msg + ': ' + JSON.stringify(state.str.slice(state.index, 10)),
@@ -29,6 +27,24 @@ function nextOperator(state, op) {
   return false;
 }
 
+function unifyNumberFormatForParsing(numberStr: string): string {
+  let unifiedNumberStr = '';
+  for (let i = 0; i < numberStr.length; i++) {
+    let ch = numberStr[i];
+    if (ch === ',' || ch === '.') {
+      // Skip thousands separators
+      let remainingChars = numberStr.length - i;
+      if (remainingChars > 3) {
+        continue;
+      }
+      // Unify decimal separator
+      ch = '.';
+    }
+    unifiedNumberStr += ch;
+  }
+  return unifiedNumberStr;
+}
+
 function parsePrimary(state) {
   // We only support numbers
   let isNegative = char(state) === '-';
@@ -44,21 +60,14 @@ function parsePrimary(state) {
   // and we should do more strict parsing
   let numberStr = '';
   while (char(state) && char(state).match(/[0-9,.]/)) {
-    let thousandsSep = getNumberFormat().separator === ',' ? '.' : ',';
-
-    // Don't include the thousands separator
-    if (char(state) === thousandsSep) {
-      next(state);
-    } else {
-      numberStr += next(state);
-    }
+    numberStr += next(state);
   }
 
   if (numberStr === '') {
     fail(state, 'Unexpected character');
   }
 
-  let number = parseFloat(numberStr.replace(getNumberFormat().separator, '.'));
+  let number = parseFloat(unifyNumberFormatForParsing(numberStr));
   return isNegative ? -number : number;
 }
 

upcoming-release-notes/1029.md

+---
+category: Enhancements
+authors: [chylex]
+---
+
+Make number parsing agnostic to decimal and thousands separators