From 4249a0beb14ddeb2fcd76e65227865af0b023195 Mon Sep 17 00:00:00 2001
From: chylex <info@chylex.com>
Date: Sat, 20 May 2023 14:27:17 +0200
Subject: [PATCH] Make number parsing agnostic to decimal and thousands
separators (#894) (#1029)
---
.../e2e/page-models/account-page.js | 8 ++
.../loot-core/src/mocks/number-formats.ts | 25 +++++++
.../loot-core/src/shared/arithmetic.test.ts | 75 +++++++++++++++++++
packages/loot-core/src/shared/arithmetic.ts | 31 +++++---
upcoming-release-notes/1029.md | 6 ++
5 files changed, 134 insertions(+), 11 deletions(-)
create mode 100644 packages/loot-core/src/mocks/number-formats.ts
create mode 100644 upcoming-release-notes/1029.md
diff --git a/packages/desktop-client/e2e/page-models/account-page.js b/packages/desktop-client/e2e/page-models/account-page.js
index 37398971a..bf4fc96ba 100644
--- a/packages/desktop-client/e2e/page-models/account-page.js
+++ b/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');
}
diff --git a/packages/loot-core/src/mocks/number-formats.ts b/packages/loot-core/src/mocks/number-formats.ts
new file mode 100644
index 000000000..e586b4906
--- /dev/null
+++ b/packages/loot-core/src/mocks/number-formats.ts
@@ -0,0 +1,25 @@
+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;
+}
diff --git a/packages/loot-core/src/shared/arithmetic.test.ts b/packages/loot-core/src/shared/arithmetic.test.ts
index c0f5afa46..4d94c606f 100644
--- a/packages/loot-core/src/shared/arithmetic.test.ts
+++ b/packages/loot-core/src/shared/arithmetic.test.ts
@@ -1,4 +1,7 @@
+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);
+ },
+ );
});
diff --git a/packages/loot-core/src/shared/arithmetic.ts b/packages/loot-core/src/shared/arithmetic.ts
index 4dd2b8955..0957d10b1 100644
--- a/packages/loot-core/src/shared/arithmetic.ts
+++ b/packages/loot-core/src/shared/arithmetic.ts
@@ -1,5 +1,3 @@
-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;
}
diff --git a/upcoming-release-notes/1029.md b/upcoming-release-notes/1029.md
new file mode 100644
index 000000000..535089458
--- /dev/null
+++ b/upcoming-release-notes/1029.md
@@ -0,0 +1,6 @@
+---
+category: Enhancements
+authors: [chylex]
+---
+
+Make number parsing agnostic to decimal and thousands separators
--
GitLab