Revert “Make number parsing agnostic to decimal and thousands separators” (#1144)

This reverts #1029. As raised in #1097, the formatting chosen doesn’t
work well when doing math. There may be a way to balance compatibility
with multiple format styles with handling non-currency amounts
correctly, but it will require some more careful consideration. Re-opens
#894.

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

@@ -91,12 +91,6 @@ 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();
@@ -123,14 +117,12 @@ 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', () => {
@@ -44,76 +41,4 @@ 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)),
@@ -27,24 +29,6 @@ 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) === '-';
@@ -60,14 +44,21 @@ function parsePrimary(state) {
   // and we should do more strict parsing
   let numberStr = '';
   while (char(state) && char(state).match(/[0-9,.]/)) {
-    numberStr += next(state);
+    let thousandsSep = getNumberFormat().separator === ',' ? '.' : ',';
+
+    // Don't include the thousands separator
+    if (char(state) === thousandsSep) {
+      next(state);
+    } else {
+      numberStr += next(state);
+    }
   }
 
   if (numberStr === '') {
     fail(state, 'Unexpected character');
   }
 
-  let number = parseFloat(unifyNumberFormatForParsing(numberStr));
+  let number = parseFloat(numberStr.replace(getNumberFormat().separator, '.'));
   return isNegative ? -number : number;
 }
 

upcoming-release-notes/1144.md

+---
+category: Bugfix
+authors: [j-f1]
+---
+
+Revert “Make number parsing agnostic to decimal and thousands separators” because it produced undesirable behavior