From 58357ee999936f44d13f1118e027c1574a9f74b2 Mon Sep 17 00:00:00 2001 From: Jonas Zeltner Date: Tue, 4 Feb 2025 14:37:43 +0100 Subject: [PATCH 1/4] Add tests for trimming whitespace --- .../lib/table-interpreter-executor.spec.ts | 52 ++++++++++++++++++ .../test-with-whitespace.xlsx | Bin 0 -> 6194 bytes .../test-with-whitespace.xlsx.license | 3 + .../valid-without-header-without-trim.jv | 25 +++++++++ 4 files changed, 80 insertions(+) create mode 100644 libs/extensions/tabular/exec/test/assets/table-interpreter-executor/test-with-whitespace.xlsx create mode 100644 libs/extensions/tabular/exec/test/assets/table-interpreter-executor/test-with-whitespace.xlsx.license create mode 100644 libs/extensions/tabular/exec/test/assets/table-interpreter-executor/valid-without-header-without-trim.jv diff --git a/libs/extensions/tabular/exec/src/lib/table-interpreter-executor.spec.ts b/libs/extensions/tabular/exec/src/lib/table-interpreter-executor.spec.ts index cf41dfec..8f135063 100644 --- a/libs/extensions/tabular/exec/src/lib/table-interpreter-executor.spec.ts +++ b/libs/extensions/tabular/exec/src/lib/table-interpreter-executor.spec.ts @@ -299,5 +299,57 @@ describe('Validation of TableInterpreterExecutor', () => { expect(result.right.getNumberOfRows()).toEqual(0); } }); + + it('should skip leading and trailing whitespace on numeric columns but not text columns', async () => { + const text = readJvTestAsset('valid-without-header.jv'); + + const testWorkbook = await readTestWorkbook('test-with-whitespace.xlsx'); + const result = await parseAndExecuteExecutor( + text, + testWorkbook.getSheetByName('Sheet1') as R.Sheet, + ); + + expect(R.isErr(result)).toEqual(false); + assert(R.isOk(result)); + + expect(result.right.ioType).toEqual(IOType.TABLE); + expect(result.right.getNumberOfColumns()).toEqual(3); + expect(result.right.getNumberOfRows()).toEqual(3); + + expect([...result.right.getColumns().keys()]).toStrictEqual([ + 'index', + 'name', + 'flag', + ]); + + const row = result.right.getRow(0); + const index = row.get('index'); + expect(index).toBe(0); + const name = row.get('name'); + expect(name).toBe(' text with leading whitespace'); + + for (let rowIdx = 1; rowIdx < result.right.getNumberOfRows(); rowIdx++) { + const row = result.right.getRow(rowIdx); + const index = row.get('index'); + expect(index).toBe(rowIdx); + } + }); + + it('should not skip leading or trailing whitespace if the relevant block properties are false', async () => { + const text = readJvTestAsset('valid-without-header-without-trim.jv'); + + const testWorkbook = await readTestWorkbook('test-with-whitespace.xlsx'); + const result = await parseAndExecuteExecutor( + text, + testWorkbook.getSheetByName('Sheet1') as R.Sheet, + ); + + expect(R.isErr(result)).toEqual(false); + if (R.isOk(result)) { + expect(result.right.ioType).toEqual(IOType.TABLE); + expect(result.right.getNumberOfColumns()).toEqual(3); + expect(result.right.getNumberOfRows()).toEqual(0); + } + }); }); }); diff --git a/libs/extensions/tabular/exec/test/assets/table-interpreter-executor/test-with-whitespace.xlsx b/libs/extensions/tabular/exec/test/assets/table-interpreter-executor/test-with-whitespace.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..b880fe1964984e8948d6fd6aba9052f2217645aa GIT binary patch literal 6194 zcmaJ_1z1#V(*|LwC6*2e3F$8B21z9aL7JtTUFq)b4y7d|q*OpzN@B?s=~PNO1bGqo z7T@>!E&LzEjrh5YU1$YbgbcJ$!! zba8jEaB*?q_HuIMzP|HPTh_jd7u2>+YO&X}T@t3vxlV-hrfd@}tgTtyQN{M2)8Tog zhAlN#A?CAgxV=q;`QU`xMh}b9*EStZD^V;+?_|CMso(ZOfr**k&;Zv-4ubrZ@tx$S z)NFO|Wj$4XGE9wZf{pQ#1GmvUGe_{Kb*)j_k%Z}h$fuyMN;;1A05=`-J5S4luEtI@ z5R^}RJt#&GXn_58rdNIdxMhCY&GnWEmtmq^{@Z5S6x5;&80?%Hv#P&_1 z2kPRS9!YGkc#m^eCm6i=Y$3hZvH+gN&l91wwEs+l)Mc zuqbe`E!*30AB&-s9HK z?AlK^+F$WifpQf=Q*9wRv%$_XS|uOL)|AVp0nL0JKqFK$u26G|2cbE@9vM8^%W!^<9xpH;4Ls$==Cdw<*N&kD+|Iakv)XK53(6rU=YSu>QQ@iD{v44!0ij z>y!90Eie8eOGD1*5su{;sT*sanCPtu2QGV7FGTX}<3OJQypQd1hLw;7a^2}obT$ad zBdZjO-0mv)FdgQ#9;Z1wC5z2t0HP|n6rb3Qh&)n$n11hTTi$)}xOdV$a zv9ps>+7Pm?r=pFx;1NE6@L3MsQ&bcb zXS6@%4DsJN1F;1=fq8zO`L1`?e7dpA>>ZMby;Ba*DTmvff(l7_PKILZi^f<;yEJU7 z4Al|UDKk(S_rXeS1wzp7s~<7G#NJ@AhehpPfO4=&mT;Rcc7iiA1)t(zT9pKBk?tuS zIRzz<9iYXYu|zEJ-TBBIkHd(!KPi=?Aq1WL(<2T!TqwfKc_4i+9W{)`Jv)%926g=NG1q&$ z{oC%6(gY}_yR2R9#enYA*XBl=6bWBIBblSCMwbUcks{WTSKkK~@xO-``(C!_2M8~1 zx~uSbj)!!*N_9Rgj)Hm-kcL!!-iU7y@Rxn1?MKsN(||=5iSb^$WI{@|*0DU>(4T3! z;-fg`_m#H~^!f_eGf<&(EJLzpx^8N*G|Jo$xE14UQD=C!VjjETabR!_Wk|w43(SP_ zyKq#T3EbVh%l*cuy!9!UlNg={$}}+uEq8_nQ7BAu}BW zRk)f9#!bAddrz(wyWm5^UtgcQyHM#H-AmKUF)MbDllih+Ec5bY=!u zrAE%lQV42QuXQsbBH+Ri7k$Pmm)}W^qo9u9bk5hW4mKtJ*qUU{f*#vOihBDE$c*eT zs;)ay|7f~O9;-x-Q!Y(lyX%&k7g5`O2|MdMb-qV3HfH0I720PGACk>u1ei{}60+YB zdnD7BwlcU`ar4HM(1SlxzTNHa2buHkPch_zRYQ_^vJRI6&`^EPKz2^4gy_QaY>AC& zr&;E~tdLI_iLEdf;V_N2<~;9tUW;8yNVe9!{EXM(_}9VVF}zvBx|~XOsVDCx27d?{ zmV`HfGvNg3yf)TA-OfWmy?m6GlC3$c5Vey8yJDxQ@l}*YlcJob&Bkialme@hBb6Kw z-6SEcGh_&Ysk`HyyoK3yyxm|o@Y+;?9;>^+75{UR5OVwGphI}RgUumZGreCyvu_tsK z{KKoV;Z?DI$(V_dQ(0x`VIG-)a|&yH2kCw@?xu{6DpAHVX7G9l2X&?UYNqQ_p%|#- zNs@2=j6_F?q1~Edp|b5AaT6>OlS*Js(om7mJa*cM&{USPfiBsnQ!93?*6brnJ7>O~o7n`?**eNI zhdVy9FBsq_c^|@b3cxQC=;Fk~F5=PmCoVoEG1H7&aoN$MTbqx$;)~^tjfmtMM$#<6 z#1=%!3Ink4R$s{1mwR&vKy9E{G(V7a853)Z@!|amfp?dYcb&f7)F;+Wb$V7Q{8OO9yR+L?FX7_%XZCCtfYx8=j?2jFBfPCdHT@IGA9 zY%`$Xig4$RR|;kC04fTzjdPi<+p0JJuxrAXAo47j`ha2r32x7))bI{gzFhaO;qRmm z+!eAM2y421dA)K{C>Hl$!})Z`6wG2sMl&hdtm04d+JHQSwglP^Qd}|jo|eIYN;Y1i z(SS?|z0j<6(k74e%!O5s{1Jv9LM3Ln+MWuY?A5Tt?~?2uOa%igjArDT6?laTG;wC0 zw3I)5mrKIfIo}10!kI(Ze+S*w=N7I{Q2ZuJD6H%cDRMO03p z$~IGKw)Irl-yxl;=Ig2NRPkM`)Fnx&COC^gVBCSRu~&?f+@T?d`eDY|a3j8zp!5Ut zErQu5#Ee-!Ia~pL^!GtcL`Ek0iZn|*PK`c`H(Ur-Zh6Pzw+VtPtiKgEE&ou76s)Oj zwbI@Ai%eNMf)qF@3z|0{wC1x<+Abn#A|G2#~@k3RMExw z7y|PW!S79VolC=^Is$CH>Vk*_r801ru6WZltojBsPb61z&3(Ee+v5e`b~Vt1RzGS! zjrk}MVyJdhQT0HJ9%{;=ncHe%)We^`l+~U+$uu-}1P_m7V}bjrL3v+>GB@gOy}DS- z>tbK8>!&POKdye_OHCiXmb~P9pCUlG;U%08iijd6sy!d?*;c3P`?Ps%%#Ei%#Yz7* z_dK};fnG%#I87?+pt$eT(^-RM#L;OTj`^0#&3t4Jl|#^e}?!~BrRZ-iC={^&WG7$a7cP;bB**%Q`atnc}F ziU$ZGwE!yxp+_H4Z!fz0^Syos82QG7u6_Gn1k{8+z1heU{0??P^ES=Pn@Y;5M??1c zrlInDs3p@H0JgS@ZEs%HBq-!glu|azQigk(Y)xZww|}wqbGvAwSBgQP_*Ky13Zm05 z;QM3uZ<~bGO_1FVA?bbUdTF|SGRKCxRBK~UPVH8oc&Dv`+S()tszL4H?_$PL-RH0s zW_~0-`Xlffz}~LD?({`RK~W?66M&KYQ_yYA-N9Bm5O+Igo1b+(K3UfJ79XzSsbu)7 zq?MVLRvLD!swAqUjG(1*i8_Bb?emt}!mpLuqvqI6qG{zs=TYBw}4P z;EN2C4*zV8uTW|tr8%ow2JIp!gPI4}eswGk$tZ3ZiIN-%7`6IlR;e$CiIOk0Qtcge zNp6ol4{+AH#Z)@lvqV-jsxxoJ2(=@2rge+YkYZM}BYNhyB8(Eh*v#-FAYy^+a)m;& z?i}OVKft5cy9>y-r~Ts~21Gvb_q6)=gqi|-(8Y@zvF97ZekwFCgH>48s;=BhFG$8hb9UVRyZy!e{bE+4B zUD41Io_oBt2N!-#GTsVBQv}g;JskWR%)8xtY?sjYLffAW*SUOEpTJ?fTxn48?S8tT zK{fW+m%jUXq0)gHOpG6EUX}E6&yP>u`aTcye$o+I!iDK9Tx zkOGn}w5@P{dN^CNk=1ORyYqpCcQ#RwnY+0&W3)^D`?~bn;&r4t_}j@HkW=F9A0q|2 z87XsD*Xuy#C-*yd@`57v{A2pQrcYs8$tP%)RTb3OUD|Pw0Sv?K zg#E8LAyDeb4U|+JrAgLk>=+p!WBS6Ujn=cQbXNBQhknfBLM0Yg+a8mRNh?luli=7} zvaM7*Z7phX`BShyW9k7hT_1t?xk%R;3&g2vM=RkuVdDojdeMktOfK2sZM>+O?p2sH zFkLBc-6MC+3N3LALrL6~Aa12~mSQAQDZXRPu5XdyFYBG>=%>p}jxXP0OYu;N46O(< zAN+v6JieT|O!Lm=DT+)G<~#s)S}H5NUjn1jsDP8f`9Wc+On3DZah9^vindiuw0^vR zk}W(jg{5#Z!?^LTL}T{!ZGNKXN5icL1W0GTMIQf~rnw%uf2BRt1LETJ)1SljIGwyC z;mh;oSktq*n91Xfu_EHSr%4OVs2(nT8nOPqj3K)Tk#tfI5Rw=2)u3rY`!_rsY$#lK z_$*v57o{R9u>K`8+OX%V7@Vz7C!a6^qWV!|Iqx`#mXp|*P7N>NWr|vIGb*}h7r1E) zCJc9GZ?B-His?>hO@|YoG5Cp|9efqKP{X%c-21-ZLhtKT9AGm`ew`%CpHPA_k*hvH z9)Er>BY78R2-q27s_X3v_AtJFuXeqDB>G)bPzD|*`$nMQ;0Ir=L`ad zv1CG`E2wRzmNdq!oY($epBr zW4iLRD%Le+uGw45UOB6&=?kY>Rxj&(w1?(7T1PmU(e5~p4JAV^wW-P$&k2Wtu`{+Z zDM##iE8!m5Q~4Yei-NlO2RO>i@bLR(yusWKe^JF2()A84#ASnR*~C4(eZ%hEeW)X# zNH2=jwT?rd-GZOBPbwpGG?c@}{EhG;$P#_WV{}?x-a}zDQV;KPZdJG>&=!ie-}w0L zR|Ne~Rkw>NkPv8ry$K2-4br)&D{I-?eYT!M`In tf@~ZAt8MvTX#3s3&D{LAfpX;K|AQf#Y5-&!qo4qh9~iPy8IfLJ{Rb)nT37%8 literal 0 HcmV?d00001 diff --git a/libs/extensions/tabular/exec/test/assets/table-interpreter-executor/test-with-whitespace.xlsx.license b/libs/extensions/tabular/exec/test/assets/table-interpreter-executor/test-with-whitespace.xlsx.license new file mode 100644 index 00000000..e39adc51 --- /dev/null +++ b/libs/extensions/tabular/exec/test/assets/table-interpreter-executor/test-with-whitespace.xlsx.license @@ -0,0 +1,3 @@ +SPDX-FileCopyrightText: 2025 Friedrich-Alexander-Universitat Erlangen-Nurnberg + +SPDX-License-Identifier: AGPL-3.0-only diff --git a/libs/extensions/tabular/exec/test/assets/table-interpreter-executor/valid-without-header-without-trim.jv b/libs/extensions/tabular/exec/test/assets/table-interpreter-executor/valid-without-header-without-trim.jv new file mode 100644 index 00000000..ec1fc6b7 --- /dev/null +++ b/libs/extensions/tabular/exec/test/assets/table-interpreter-executor/valid-without-header-without-trim.jv @@ -0,0 +1,25 @@ +// SPDX-FileCopyrightText: 2025 Friedrich-Alexander-Universitat Erlangen-Nurnberg +// +// SPDX-License-Identifier: AGPL-3.0-only + +pipeline TestPipeline { + + block TestExtractor oftype TestSheetExtractor { } + + block TestBlock oftype TableInterpreter { + header: false; + columns: [ + "index" oftype integer, + "name" oftype text, + "flag" oftype boolean + ]; + skipLeadingWhitespace: false; + skipTrailingWhitespace: false; + } + + block TestLoader oftype TestTableLoader { } + + TestExtractor + -> TestBlock + -> TestLoader; +} From efa6f9612ed945c4898370390329d9880ff4a1fd Mon Sep 17 00:00:00 2001 From: Jonas Zeltner Date: Tue, 4 Feb 2025 14:38:33 +0100 Subject: [PATCH 2/4] Enable `TableInterpreter` to trim whitespace --- .../internal-representation-parsing.ts | 34 +++++++++++++++++-- .../src/lib/table-interpreter-executor.ts | 31 +++++++++++++++-- .../builtin-block-types/TableInterpreter.jv | 32 +++++++++++------ 3 files changed, 81 insertions(+), 16 deletions(-) diff --git a/libs/execution/src/lib/types/value-types/internal-representation-parsing.ts b/libs/execution/src/lib/types/value-types/internal-representation-parsing.ts index 39c2b032..51433361 100644 --- a/libs/execution/src/lib/types/value-types/internal-representation-parsing.ts +++ b/libs/execution/src/lib/types/value-types/internal-representation-parsing.ts @@ -18,8 +18,15 @@ import { export function parseValueToInternalRepresentation< I extends InternalValueRepresentation, ->(value: string, valueType: ValueType): I | undefined { - const visitor = new InternalRepresentationParserVisitor(value); +>( + value: string, + valueType: ValueType, + parseOpts?: ParseOpts, +): I | undefined { + const visitor = new InternalRepresentationParserVisitor( + value, + parseOpts ?? { skipLeadingWhitespace: true, skipTrailingWhitespace: true }, + ); const result = valueType.acceptVisitor(visitor); if (!valueType.isInternalValueRepresentation(result)) { return undefined; @@ -27,22 +34,43 @@ export function parseValueToInternalRepresentation< return result; } +export interface ParseOpts { + skipLeadingWhitespace: boolean; + skipTrailingWhitespace: boolean; +} + class InternalRepresentationParserVisitor extends ValueTypeVisitor< InternalValueRepresentation | undefined > { - constructor(private value: string) { + constructor(private value: string, private parseOpts: ParseOpts) { super(); } + private trim() { + // BUG: https://github.com/jvalue/jayvee/issues/646 + if (typeof this.value !== 'string') { + return; + } + if (this.parseOpts.skipLeadingWhitespace) { + this.value = this.value.trimStart(); + } + if (this.parseOpts.skipTrailingWhitespace) { + this.value = this.value.trimEnd(); + } + } + visitBoolean(vt: BooleanValuetype): boolean | undefined { + this.trim(); return vt.fromString(this.value); } visitDecimal(vt: DecimalValuetype): number | undefined { + this.trim(); return vt.fromString(this.value); } visitInteger(vt: IntegerValuetype): number | undefined { + this.trim(); return vt.fromString(this.value); } diff --git a/libs/extensions/tabular/exec/src/lib/table-interpreter-executor.ts b/libs/extensions/tabular/exec/src/lib/table-interpreter-executor.ts index ffab27b8..c581241c 100644 --- a/libs/extensions/tabular/exec/src/lib/table-interpreter-executor.ts +++ b/libs/extensions/tabular/exec/src/lib/table-interpreter-executor.ts @@ -58,6 +58,14 @@ export class TableInterpreterExecutor extends AbstractBlockExecutor< context.valueTypeProvider.Primitives.ValuetypeAssignment, ), ); + const skipLeadingWhitespace = context.getPropertyValue( + 'skipLeadingWhitespace', + context.valueTypeProvider.Primitives.Boolean, + ); + const skipTrailingWhitespace = context.getPropertyValue( + 'skipTrailingWhitespace', + context.valueTypeProvider.Primitives.Boolean, + ); let columnEntries: ColumnDefinitionEntry[]; @@ -107,6 +115,8 @@ export class TableInterpreterExecutor extends AbstractBlockExecutor< inputSheet, header, columnEntries, + skipLeadingWhitespace, + skipTrailingWhitespace, context, ); context.logger.logDebug( @@ -119,6 +129,8 @@ export class TableInterpreterExecutor extends AbstractBlockExecutor< sheet: Sheet, header: boolean, columnEntries: ColumnDefinitionEntry[], + skipLeadingWhitespace: boolean, + skipTrailingWhitespace: boolean, context: ExecutionContext, ): Table { const table = new Table(); @@ -141,6 +153,8 @@ export class TableInterpreterExecutor extends AbstractBlockExecutor< sheetRow, sheetRowIndex, columnEntries, + skipLeadingWhitespace, + skipTrailingWhitespace, context, ); if (tableRow === undefined) { @@ -158,6 +172,8 @@ export class TableInterpreterExecutor extends AbstractBlockExecutor< sheetRow: string[], sheetRowIndex: number, columnEntries: ColumnDefinitionEntry[], + skipLeadingWhitespace: boolean, + skipTrailingWhitespace: boolean, context: ExecutionContext, ): R.TableRow | undefined { let invalidRow = false; @@ -168,7 +184,13 @@ export class TableInterpreterExecutor extends AbstractBlockExecutor< const value = sheetRow[sheetColumnIndex]!; const valueType = columnEntry.valueType; - const parsedValue = this.parseAndValidateValue(value, valueType, context); + const parsedValue = this.parseAndValidateValue( + value, + valueType, + skipLeadingWhitespace, + skipTrailingWhitespace, + context, + ); if (parsedValue === undefined) { const currentCellIndex = new CellIndex(sheetColumnIndex, sheetRowIndex); context.logger.logDebug( @@ -192,9 +214,14 @@ export class TableInterpreterExecutor extends AbstractBlockExecutor< private parseAndValidateValue( value: string, valueType: ValueType, + skipLeadingWhitespace: boolean, + skipTrailingWhitespace: boolean, context: ExecutionContext, ): InternalValueRepresentation | undefined { - const parsedValue = parseValueToInternalRepresentation(value, valueType); + const parsedValue = parseValueToInternalRepresentation(value, valueType, { + skipLeadingWhitespace, + skipTrailingWhitespace, + }); if (parsedValue === undefined) { return undefined; } diff --git a/libs/language-server/src/stdlib/builtin-block-types/TableInterpreter.jv b/libs/language-server/src/stdlib/builtin-block-types/TableInterpreter.jv index 866eacec..0d60cd70 100644 --- a/libs/language-server/src/stdlib/builtin-block-types/TableInterpreter.jv +++ b/libs/language-server/src/stdlib/builtin-block-types/TableInterpreter.jv @@ -4,7 +4,7 @@ /** * Interprets a `Sheet` as a `Table`. In case a header row is present in the sheet, its names can be matched with the provided column names. Otherwise, the provided column names are assigned in order. -* +* * @example Interprets a `Sheet` about cars with a topmost header row and interprets it as a `Table` by assigning a primitive value type to each column. The column names are matched to the header, so the order of the type assignments does not matter. * block CarsTableInterpreter oftype TableInterpreter { * header: true; @@ -14,7 +14,7 @@ * "cyl" oftype integer, * ]; * } -* +* * @example Interprets a `Sheet` about cars without a topmost header row and interprets it as a `Table` by sequentially assigning a name and a primitive value type to each column of the sheet. Note that the order of columns matters here. The first column (column `A`) will be named "name", the second column (column `B`) will be named "mpg" etc. * block CarsTableInterpreter oftype TableInterpreter { * header: false; @@ -28,14 +28,24 @@ publish builtin blocktype TableInterpreter { input default oftype Sheet; output default oftype Table; - - /** - * Whether the first row should be interpreted as header row. + + /** + * Whether the first row should be interpreted as header row. + */ + property header oftype boolean: true; + + /** + * Collection of value type assignments. Uses column names (potentially matched with the header or by sequence depending on the `header` property) to assign a primitive value type to each column. */ - property header oftype boolean: true; - - /** - * Collection of value type assignments. Uses column names (potentially matched with the header or by sequence depending on the `header` property) to assign a primitive value type to each column. + property columns oftype Collection; + + /** + * Whether to ignore whitespace before values. Does not apply to `text` cells + */ + property skipLeadingWhitespace oftype boolean: true; + + /** + * Whether to ignore whitespace after values. Does not apply to `text` cells */ - property columns oftype Collection; -} \ No newline at end of file + property skipTrailingWhitespace oftype boolean: true; +} From 626d94c222ce5a156c6708a024ad763f4f9af220 Mon Sep 17 00:00:00 2001 From: Jonas Zeltner Date: Wed, 5 Feb 2025 15:06:50 +0100 Subject: [PATCH 3/4] Extract parser defaults into their own constant --- .../internal-representation-parsing.ts | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/libs/execution/src/lib/types/value-types/internal-representation-parsing.ts b/libs/execution/src/lib/types/value-types/internal-representation-parsing.ts index 51433361..72bd5a90 100644 --- a/libs/execution/src/lib/types/value-types/internal-representation-parsing.ts +++ b/libs/execution/src/lib/types/value-types/internal-representation-parsing.ts @@ -16,17 +16,27 @@ import { ValueTypeVisitor, } from '@jvalue/jayvee-language-server'; +export interface ParseOpts { + skipLeadingWhitespace: boolean; + skipTrailingWhitespace: boolean; +} + +const DEFAULT_PARSE_OPTS: ParseOpts = { + skipLeadingWhitespace: true, + skipTrailingWhitespace: true, +}; + export function parseValueToInternalRepresentation< I extends InternalValueRepresentation, >( value: string, valueType: ValueType, - parseOpts?: ParseOpts, + parseOpts?: Partial, ): I | undefined { - const visitor = new InternalRepresentationParserVisitor( - value, - parseOpts ?? { skipLeadingWhitespace: true, skipTrailingWhitespace: true }, - ); + const visitor = new InternalRepresentationParserVisitor(value, { + ...DEFAULT_PARSE_OPTS, + ...parseOpts, + }); const result = valueType.acceptVisitor(visitor); if (!valueType.isInternalValueRepresentation(result)) { return undefined; @@ -34,11 +44,6 @@ export function parseValueToInternalRepresentation< return result; } -export interface ParseOpts { - skipLeadingWhitespace: boolean; - skipTrailingWhitespace: boolean; -} - class InternalRepresentationParserVisitor extends ValueTypeVisitor< InternalValueRepresentation | undefined > { From ffc7d738bb2b4394f3f0115755cd50d8531335e0 Mon Sep 17 00:00:00 2001 From: Jonas Zeltner Date: Wed, 5 Feb 2025 15:18:22 +0100 Subject: [PATCH 4/4] Apply review suggestions --- .../internal-representation-parsing.ts | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/libs/execution/src/lib/types/value-types/internal-representation-parsing.ts b/libs/execution/src/lib/types/value-types/internal-representation-parsing.ts index 72bd5a90..ee1c7ad4 100644 --- a/libs/execution/src/lib/types/value-types/internal-representation-parsing.ts +++ b/libs/execution/src/lib/types/value-types/internal-representation-parsing.ts @@ -51,32 +51,29 @@ class InternalRepresentationParserVisitor extends ValueTypeVisitor< super(); } - private trim() { + private applyTrimOptions(value: string): string { // BUG: https://github.com/jvalue/jayvee/issues/646 - if (typeof this.value !== 'string') { - return; - } - if (this.parseOpts.skipLeadingWhitespace) { - this.value = this.value.trimStart(); - } - if (this.parseOpts.skipTrailingWhitespace) { - this.value = this.value.trimEnd(); + if (typeof this.value === 'string') { + if (this.parseOpts.skipLeadingWhitespace) { + value = value.trimStart(); + } + if (this.parseOpts.skipTrailingWhitespace) { + value = value.trimEnd(); + } } + return value; } visitBoolean(vt: BooleanValuetype): boolean | undefined { - this.trim(); - return vt.fromString(this.value); + return vt.fromString(this.applyTrimOptions(this.value)); } visitDecimal(vt: DecimalValuetype): number | undefined { - this.trim(); - return vt.fromString(this.value); + return vt.fromString(this.applyTrimOptions(this.value)); } visitInteger(vt: IntegerValuetype): number | undefined { - this.trim(); - return vt.fromString(this.value); + return vt.fromString(this.applyTrimOptions(this.value)); } visitText(vt: TextValuetype): string {