Skip to content

Commit

Permalink
[FIX] locale: prevent identical decimal and thousands separator
Browse files Browse the repository at this point in the history
Currently, a locale with identical decimal and thousands separator
is considered valid.
It should not because something like "1.000.45" cannot be a valid
number

Opw: 3720586
Task: 3720586
Part-of: #3677
  • Loading branch information
LucasLefevre committed Feb 13, 2024
1 parent 9ff785f commit 0b7fffb
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 5 deletions.
3 changes: 3 additions & 0 deletions src/helpers/locale.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ export function isValidLocale(locale: any): locale is Locale {
if (locale.formulaArgSeparator === locale.decimalSeparator) {
return false;
}
if (locale.thousandsSeparator === locale.decimalSeparator) {
return false;
}

try {
formatValue(1, { locale, format: "#,##0.00" });
Expand Down
7 changes: 6 additions & 1 deletion tests/components/link/link_editor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,12 @@ describe("link editor component", () => {
});

test("label is changed to canonical form in model", async () => {
updateLocale(model, { ...DEFAULT_LOCALE, formulaArgSeparator: ";", decimalSeparator: "," });
updateLocale(model, {
...DEFAULT_LOCALE,
formulaArgSeparator: ";",
decimalSeparator: ",",
thousandsSeparator: " ",
});
await openLinkEditor(model, "A1");
setInputValueAndTrigger(labelInput(), "3,15", "input");
setInputValueAndTrigger(urlInput(), "https://url.com", "input");
Expand Down
6 changes: 6 additions & 0 deletions tests/functions/helper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -345,4 +345,10 @@ describe("Locale helpers", () => {
false
);
});

test("isValidLocale with identical thousands and decimal separators", () => {
const locale = { ...DEFAULT_LOCALE, thousandsSeparator: ".", decimalSeparator: "." };

expect(isValidLocale(locale)).toBe(false);
});
});
7 changes: 6 additions & 1 deletion tests/plugins/clipboard.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1921,7 +1921,12 @@ describe("clipboard: pasting outside of sheet", () => {

test("Can paste localized formula from the OS", () => {
const model = new Model();
updateLocale(model, { ...DEFAULT_LOCALE, decimalSeparator: ",", formulaArgSeparator: ";" });
updateLocale(model, {
...DEFAULT_LOCALE,
decimalSeparator: ",",
formulaArgSeparator: ";",
thousandsSeparator: " ",
});
pasteFromOSClipboard(model, "A1", "=SUM(5 ; 3,14)");
expect(getCell(model, "A1")?.content).toBe("=SUM(5 , 3.14)");
expect(getEvaluatedCell(model, "A1").value).toBe(8.14);
Expand Down
8 changes: 7 additions & 1 deletion tests/plugins/edition.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1131,7 +1131,12 @@ describe("edition", () => {
editCell(model, "A1", "=SUM(B2;5)");
expect(getEvaluatedCell(model, "A1").type).toBe(CellValueType.error);

updateLocale(model, { ...DEFAULT_LOCALE, formulaArgSeparator: ";", decimalSeparator: "," });
updateLocale(model, {
...DEFAULT_LOCALE,
formulaArgSeparator: ";",
decimalSeparator: ",",
thousandsSeparator: " ",
});
editCell(model, "A1", "=SUM(B2,5)");
expect(getEvaluatedCell(model, "A1").type).toBe(CellValueType.error);
editCell(model, "A1", "=SUM(B2;5)");
Expand All @@ -1141,6 +1146,7 @@ describe("edition", () => {
test("Decimal numbers as function argument", () => {
updateLocale(model, {
...DEFAULT_LOCALE,
thousandsSeparator: " ",
decimalSeparator: ",",
formulaArgSeparator: ";",
});
Expand Down
7 changes: 6 additions & 1 deletion tests/plugins/find_and_replace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -653,7 +653,12 @@ describe("replace", () => {

test("Replaced value is changed to canonical form in model", () => {
searchOptions.searchFormulas = true;
updateLocale(model, { ...DEFAULT_LOCALE, decimalSeparator: ",", formulaArgSeparator: ";" });
updateLocale(model, {
...DEFAULT_LOCALE,
decimalSeparator: ",",
formulaArgSeparator: ";",
thousandsSeparator: " ",
});
model.dispatch("UPDATE_SEARCH", { toSearch: "2", searchOptions });
model.dispatch("REPLACE_SEARCH", { replaceWith: "4,5" });
const matches = model.getters.getSearchMatches();
Expand Down
7 changes: 6 additions & 1 deletion tests/plugins/split_to_column.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,12 @@ describe("Split text into columns", () => {
});

test("Localized split values are handled", () => {
updateLocale(model, { ...DEFAULT_LOCALE, decimalSeparator: ",", formulaArgSeparator: ";" });
updateLocale(model, {
...DEFAULT_LOCALE,
decimalSeparator: ",",
formulaArgSeparator: ";",
thousandsSeparator: " ",
});
setGrid(model, { A1: "5,6||=SUM(5; 1,6)" });
splitTextToColumns(model, "||", "A1");
expect(getGrid(model)).toEqual({ A1: 5.6, B1: 6.6 });
Expand Down

0 comments on commit 0b7fffb

Please sign in to comment.