Skip to content

Commit 3c0fb50

Browse files
laa-odoolaa-odoo
committed
[IMP] composer: prettify v2
Tofixup task: 4735172
1 parent d66f417 commit 3c0fb50

File tree

5 files changed

+66
-71
lines changed

5 files changed

+66
-71
lines changed

‎src/components/composer/composer/prettifier_content.ts‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,15 @@ import {
1414
// Once created, this structure is used to determine the best formatting path based on the available width,
1515
// allowing dynamic selection of the most suitable layout for the given constraints.
1616

17-
type PrettifyPossibility = string | ChooseBetween | Join | Nest | Line;
17+
type PrettifyPossibility = string | ChooseBetween | Join | Nest | insertLine;
1818
type ChooseBetween = { type: "chooseBetween"; p1: PrettifyPossibility; p2: PrettifyPossibility };
1919
type Join = { type: "join"; rules: PrettifyPossibility[] };
2020
type Nest = { type: "nest"; indentLvl: number; p: PrettifyPossibility };
21-
type Line = { type: "line" };
21+
type insertLine = { type: "insertLine" };
2222

2323
/** Useful for indicating where to insert a new line. Placed in a group, it will be used if there is insufficient space*/
24-
function line(): Line {
25-
return { type: "line" };
24+
function line(): insertLine {
25+
return { type: "insertLine" };
2626
}
2727

2828
/** Useful for indicating where to insert a new line with a specific indentation level.
@@ -68,8 +68,8 @@ function flatten(pp: PrettifyPossibility): PrettifyPossibility {
6868
p: flatten(pp.p),
6969
};
7070
}
71-
if (pp.type === "line") {
72-
return " ";
71+
if (pp.type === "insertLine") {
72+
return "";
7373
}
7474
return pp;
7575
}
@@ -167,7 +167,7 @@ function _best(width: number, currentIndentLvl: number, head: RestToFitNode | nu
167167
if (pp.type === "nest") {
168168
return _best(width, currentIndentLvl, { indentLvl: indentLvl + pp.indentLvl, pp: pp.p, next });
169169
}
170-
if (pp.type === "line") {
170+
if (pp.type === "insertLine") {
171171
return {
172172
type: "subLine",
173173
indent: indentLvl,
@@ -207,7 +207,7 @@ function fits(width: number, x: SubRule): boolean {
207207
// ---------------------------------------
208208

209209
export function prettify(ast: AST) {
210-
return "= " + print(astToPp(ast), 38); // 39 but 40 with the `= ` at the beginning
210+
return "=" + print(astToPp(ast), 39); // 39 but 40 with the `= ` at the beginning
211211
}
212212

213213
/** transform an AST composed of sub-ASTs into a PrettifyPossibility composed of sub-PrettifyPossibility.*/
@@ -228,7 +228,7 @@ function astToPp(ast: AST): PrettifyPossibility {
228228
case "FUNCALL":
229229
const pps = ast.args.map(astToPp);
230230
return splitParenthesesContent(
231-
join(pps.map((pp, i) => (i < 1 ? pp : join([",", line(), pp])))),
231+
join(pps.map((pp, i) => (i < 1 ? pp : join([", ", line(), pp])))),
232232
ast.value
233233
);
234234

@@ -250,7 +250,7 @@ function astToPp(ast: AST): PrettifyPossibility {
250250
const needParenthesisRightPp = rightOperandNeedsParenthesis(ast);
251251
const finalRightPp = needParenthesisRightPp ? splitParenthesesContent(rightPp) : rightPp;
252252

253-
const operator = ` ${ast.value}`;
253+
const operator = `${ast.value}`;
254254
return group(join([finalLeftPp, operator, nest(1, join([line(), finalRightPp]))]));
255255
}
256256

‎tests/composer/auto_complete/function_auto_complete_store.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ describe("Function auto complete", () => {
1414
expect(proposals?.[0].text).toEqual("SUM");
1515
expect(proposals?.[1].text).toEqual("SUMIF");
1616
composer.insertAutoCompleteValue(proposals![0].text);
17-
expect(composer.currentContent).toEqual("= SUM(");
17+
expect(composer.currentContent).toEqual("=SUM(");
1818
});
1919

2020
test("function auto complete uses fuzzy search", async () => {
@@ -26,7 +26,7 @@ describe("Function auto complete", () => {
2626
expect(proposals).toHaveLength(1);
2727
expect(proposals?.[0].text).toBe("VLOOKUP");
2828
composer.insertAutoCompleteValue(proposals![0].text);
29-
expect(composer.currentContent).toEqual("= VLOOKUP(");
29+
expect(composer.currentContent).toEqual("=VLOOKUP(");
3030
});
3131

3232
test("reselect cell with existing content shows correct autocomplete proposals", async () => {
@@ -39,6 +39,6 @@ describe("Function auto complete", () => {
3939
expect(proposals).toHaveLength(1);
4040
expect(proposals?.[0].text).toBe("VLOOKUP");
4141
composer.insertAutoCompleteValue(proposals![0].text);
42-
expect(composer.currentContent).toEqual("= VLOOKUP(");
42+
expect(composer.currentContent).toEqual("=VLOOKUP(");
4343
});
4444
});

‎tests/composer/composer_component.test.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -924,14 +924,14 @@ describe("composer", () => {
924924
await simulateClick("div.o-composer");
925925
composerEl = fixture.querySelector<HTMLElement>("div.o-composer")!;
926926

927-
composerStore.changeComposerCursorSelection(8, 8);
927+
composerStore.changeComposerCursorSelection(5, 5);
928928
await nextTick();
929929

930-
composerStore.changeComposerCursorSelection(8, 3);
930+
composerStore.changeComposerCursorSelection(5, 2);
931931
await nextTick();
932932
const selection = document.getSelection()!;
933933
await nextTick();
934-
expect(selection?.toString()).toBe("1 + S");
934+
expect(selection?.toString()).toBe("1+S");
935935
expect(selection.anchorNode?.textContent).toBe("A1");
936936
expect(selection.focusNode?.textContent).toBe("SUM");
937937
});
@@ -1536,7 +1536,7 @@ describe("composer highlights color", () => {
15361536
composerEl = await startComposition();
15371537
expect(composerStore.highlights.length).toBe(1);
15381538
expect(composerStore.highlights[0].color).toBe(colors[0]);
1539-
expect(composerEl.textContent).toBe("= sum( A1:A10 )");
1539+
expect(composerEl.textContent).toBe("=sum(A1:A10)");
15401540
});
15411541

15421542
test("highlight range using +", async () => {
@@ -1562,7 +1562,7 @@ describe("composer highlights color", () => {
15621562
setCellContent(model, "A1", "=A1A");
15631563
composerEl = await startComposition();
15641564
expect(composerStore.highlights.length).toBe(0);
1565-
expect(composerEl.textContent).toBe("= A1A");
1565+
expect(composerEl.textContent).toBe("=A1A");
15661566
});
15671567

15681568
test("highlight cross-sheet ranges", async () => {

‎tests/composer/composer_store.test.ts‎

Lines changed: 47 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ describe("edition", () => {
160160
renameSheet(model, sheet2, nextName);
161161
composerStore.startEdition();
162162
expect(getCellText(model, "A1")).toBe("='NEXT NAME'!A1");
163-
expect(composerStore.currentContent).toBe("= 'NEXT NAME'!A1");
163+
expect(composerStore.currentContent).toBe("='NEXT NAME'!A1");
164164
});
165165

166166
test("setting content sets selection at the end by default", () => {
@@ -527,7 +527,7 @@ describe("edition", () => {
527527
test("content is the raw cell content, not the evaluated text", () => {
528528
setCellContent(model, "A1", "=SUM(5)");
529529
composerStore.startEdition();
530-
expect(composerStore.currentContent).toBe("= SUM( 5 )");
530+
expect(composerStore.currentContent).toBe("=SUM(5)");
531531
});
532532

533533
test("default active cell content when model is started", () => {
@@ -559,7 +559,7 @@ describe("edition", () => {
559559
composerStore.startEdition();
560560
composerStore.setCurrentContent("=Sheet1");
561561
composerStore.stopEdition();
562-
expect(composerStore.currentContent).toBe("= Sheet1");
562+
expect(composerStore.currentContent).toBe("=Sheet1");
563563
});
564564

565565
test("start editing where theres a merge on other sheet, change sheet, and stop edition", () => {
@@ -1186,77 +1186,77 @@ describe("edition", () => {
11861186
});
11871187

11881188
describe("prettifier", () => {
1189-
test("add missing spaces around operators", () => {
1190-
setCellContent(model, "A1", "=A2+A3");
1189+
test("remove extra spaces around operators", () => {
1190+
setCellContent(model, "A1", "= A2 + A3 ");
11911191
composerStore.startEdition();
1192-
expect(composerStore.currentContent).toBe("= A2 + A3");
1192+
expect(composerStore.currentContent).toBe("=A2+A3");
11931193
});
11941194

1195-
test("add missing spaces around formula arguments ", () => {
1196-
setCellContent(model, "A1", "=SUM(1,2,3)");
1195+
test("remove extra spaces around formula arguments ", () => {
1196+
setCellContent(model, "A1", "=SUM( 1 , 2 , 3 )");
11971197
composerStore.startEdition();
1198-
expect(composerStore.currentContent).toBe("= SUM( 1, 2, 3 )");
1198+
expect(composerStore.currentContent).toBe("=SUM(1, 2, 3)");
11991199
});
12001200

12011201
test("remove the extra parentheses", () => {
12021202
// should not be. But we loose the parentheses information during the parsing into an AST
12031203
setCellContent(model, "A1", "=(2*(((A2)+A3)))");
12041204
composerStore.startEdition();
1205-
expect(composerStore.currentContent).toBe("= 2 * ( A2 + A3 )");
1205+
expect(composerStore.currentContent).toBe("=2*(A2+A3)");
12061206

12071207
setCellContent(model, "A1", "=1+(2+(3+4))");
12081208
composerStore.startEdition();
1209-
expect(composerStore.currentContent).toBe("= 1 + 2 + 3 + 4");
1209+
expect(composerStore.currentContent).toBe("=1+2+3+4");
12101210

12111211
setCellContent(model, "A1", "=2^3%");
12121212
composerStore.startEdition();
1213-
expect(composerStore.currentContent).toBe("= 2 ^ 3%");
1213+
expect(composerStore.currentContent).toBe("=2^3%");
12141214
});
12151215

12161216
test("keep parentheses who against priority order", () => {
12171217
setCellContent(model, "A1", "=((2+3))*4");
12181218
composerStore.startEdition();
1219-
expect(composerStore.currentContent).toBe("= ( 2 + 3 ) * 4");
1219+
expect(composerStore.currentContent).toBe("=(2+3)*4");
12201220

12211221
setCellContent(model, "A1", "=1-((2-(3+4)))");
12221222
composerStore.startEdition();
1223-
expect(composerStore.currentContent).toBe("= 1 - ( 2 - ( 3 + 4 ) )");
1223+
expect(composerStore.currentContent).toBe("=1-(2-(3+4))");
12241224

12251225
setCellContent(model, "A1", "=2^((3^4))");
12261226
composerStore.startEdition();
1227-
expect(composerStore.currentContent).toBe("= 2 ^ ( 3 ^ 4 )");
1227+
expect(composerStore.currentContent).toBe("=2^(3^4)");
12281228

12291229
setCellContent(model, "A1", "=2/((3/4))");
12301230
composerStore.startEdition();
1231-
expect(composerStore.currentContent).toBe("= 2 / ( 3 / 4 )");
1231+
expect(composerStore.currentContent).toBe("=2/(3/4)");
12321232
});
12331233

12341234
test("Split the content in multiple lines when it is too long", () => {
12351235
setCellContent(model, "A1", "=SUM(11111111,22222222,33333333)"); // 41 characters
12361236
composerStore.startEdition();
1237-
expect(composerStore.currentContent).toBe("= SUM( 11111111, 22222222, 33333333 )");
1237+
expect(composerStore.currentContent).toBe("=SUM(11111111, 22222222, 33333333)");
12381238

12391239
setCellContent(model, "A1", "=SUM(11111111,22222222,33333333,44444444)"); // 41 characters
12401240
composerStore.startEdition();
12411241
expect(composerStore.currentContent).toBe(
12421242
// prettier-ignore
1243-
"= SUM(\n" +
1244-
"\t11111111,\n" +
1245-
"\t22222222,\n" +
1246-
"\t33333333,\n" +
1243+
"=SUM(\n" +
1244+
"\t11111111, \n" +
1245+
"\t22222222, \n" +
1246+
"\t33333333, \n" +
12471247
"\t44444444\n" +
12481248
")"
12491249
);
12501250
});
12511251

12521252
test("nested functions are properly indented", () => {
1253-
setCellContent(model, "A1", "=SUM(AVERAGE(1,2,3), MAX(4,5,6))");
1253+
setCellContent(model, "A1", "=SUM(AVERAGE(1,2,3,4), MAX(5,6,7,8))");
12541254
composerStore.startEdition();
12551255
expect(composerStore.currentContent).toBe(
12561256
// prettier-ignore
1257-
"= SUM(\n" +
1258-
"\tAVERAGE( 1, 2, 3 ),\n" +
1259-
"\tMAX( 4, 5, 6 )\n" +
1257+
"=SUM(\n" +
1258+
"\tAVERAGE(1, 2, 3, 4), \n" +
1259+
"\tMAX(5, 6, 7, 8)\n" +
12601260
")"
12611261
);
12621262
});
@@ -1265,18 +1265,18 @@ describe("edition", () => {
12651265
setCellContent(
12661266
model,
12671267
"A1",
1268-
"=SUM(AVERAGE(COUNT(4,5,6),COUNT(7,8,9)), MAX(COUNT(4,5,6),COUNT(7,8,9)))"
1268+
"=SUM(AVERAGE(COUNT(4,5,6,7),COUNT(10,11,12,13)), MAX(COUNT(4,5,6,7),COUNT(10,11,12,13)))"
12691269
);
12701270
composerStore.startEdition();
12711271
expect(composerStore.currentContent).toBe(
1272-
"= SUM(\n" +
1272+
"=SUM(\n" +
12731273
"\tAVERAGE(\n" +
1274-
"\t\tCOUNT( 4, 5, 6 ),\n" +
1275-
"\t\tCOUNT( 7, 8, 9 )\n" +
1276-
"\t),\n" +
1274+
"\t\tCOUNT(4, 5, 6, 7), \n" +
1275+
"\t\tCOUNT(10, 11, 12, 13)\n" +
1276+
"\t), \n" +
12771277
"\tMAX(\n" +
1278-
"\t\tCOUNT( 4, 5, 6 ),\n" +
1279-
"\t\tCOUNT( 7, 8, 9 )\n" +
1278+
"\t\tCOUNT(4, 5, 6, 7), \n" +
1279+
"\t\tCOUNT(10, 11, 12, 13)\n" +
12801280
"\t)\n" +
12811281
")"
12821282
);
@@ -1290,11 +1290,10 @@ describe("edition", () => {
12901290
);
12911291
composerStore.startEdition();
12921292
expect(composerStore.currentContent).toBe(
1293-
"= SUM(\n" +
1294-
"\t1111 + 2222 + 3333 + 4444 + 5555 +\n" +
1295-
"\t\t6666 +\n" +
1296-
"\t\t7777 +\n" +
1297-
"\t\t8888 +\n" +
1293+
//prettier-ignore
1294+
"=SUM(\n" +
1295+
"\t1111+2222+3333+4444+5555+6666+7777+\n" +
1296+
"\t\t8888+\n" +
12981297
"\t\t9999\n" +
12991298
")"
13001299
);
@@ -1304,33 +1303,29 @@ describe("edition", () => {
13041303
setCellContent(
13051304
model,
13061305
"A1",
1307-
"=SUM(1111 + 2222 + 3333 + 4444 + 5555 + 6666 * 7777 - 8888 + 9999 / 10000 )"
1306+
"=SUM(1111 + 2222 + 3333 + 4444 + 5555 + 6666 + 7777 + 8888 * 9999 - 10000 + 20000 / 30000 )"
13081307
);
13091308
composerStore.startEdition();
13101309
expect(composerStore.currentContent).toBe(
1311-
"= SUM(\n" +
1312-
"\t1111 + 2222 + 3333 + 4444 + 5555 +\n" +
1313-
"\t\t6666 * 7777 -\n" +
1314-
"\t\t8888 +\n" +
1315-
"\t\t9999 / 10000\n" +
1310+
"=SUM(\n" +
1311+
"\t1111+2222+3333+4444+5555+6666+7777+\n" +
1312+
"\t\t8888*9999-\n" +
1313+
"\t\t10000+\n" +
1314+
"\t\t20000/30000\n" +
13161315
")"
13171316
);
13181317
});
13191318

13201319
test("long functions with nested parenthesis for mathematical operation are properly indented with sub-lvls", () => {
1321-
setCellContent(
1322-
model,
1323-
"A1",
1324-
"=1*(2 -2 -2 -2 -( 3 + 3 + 3 + 3 / (4 + 5 + 6 + 7 + 5 + 6 + 7)))"
1325-
);
1320+
setCellContent(model, "A1", "=1*(2-2-2-2-2-2-2-(3+3+3+3+3+3+3+3+3/(4+5+6+7+5+6+7+8+9)))");
13261321
composerStore.startEdition();
13271322
expect(composerStore.currentContent).toBe(
1328-
"= 1 *\n" +
1323+
"=1*\n" +
13291324
"\t(\n" +
1330-
"\t\t2 - 2 - 2 - 2 -\n" +
1325+
"\t\t2-2-2-2-2-2-2-\n" +
13311326
"\t\t\t(\n" +
1332-
"\t\t\t\t3 + 3 + 3 +\n" +
1333-
"\t\t\t\t\t3 / ( 4 + 5 + 6 + 7 + 5 + 6 + 7 )\n" +
1327+
"\t\t\t\t3+3+3+3+3+3+3+3+\n" +
1328+
"\t\t\t\t\t3/(4+5+6+7+5+6+7+8+9)\n" +
13341329
"\t\t\t)\n" +
13351330
"\t)"
13361331
);

‎tests/top_bar_component.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -752,7 +752,7 @@ describe("TopBar component", () => {
752752
)!;
753753
setCellContent(model, "A1", "=A1+A2");
754754
await nextTick();
755-
expect(topbarComposerElement.textContent).toBe("= A1 + A2");
755+
expect(topbarComposerElement.textContent).toBe("=A1+A2");
756756
});
757757
});
758758

0 commit comments

Comments
 (0)