-
Notifications
You must be signed in to change notification settings - Fork 15
RFC0020 implementation #688
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@georg-schwarz do you agree with the design so far? I had to introduce a new Valuetype |
|
Do you see any major negative aspects of this design? |
ea85b57 to
41468a7
Compare
2e1aec6 to
3aaf914
Compare
…ntermediate `TableRow`
…h table row expressions
…value is no longer readonly
- Remove the logic deriving column definitions with/without header - The columns property is the definitive source of truth - The header row is used to map sheet-column-names to sheet-column-indices
3aaf914 to
2eaa215
Compare
libs/language-server/src/lib/ast/wrappers/typed-object/block-type-wrapper.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Georg Schwarz <dev@georg-schwarz.com>
Co-authored-by: Georg Schwarz <dev@georg-schwarz.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements RFC0020, introducing a new syntax for table parsing in Jayvee. The implementation replaces the inline column definitions in TableInterpreter with a valuetype-based schema definition and a parsing transform that uses the new cellInColumn operator to access sheet cells by column name or index.
Changes:
- Replaced
columns: [...]array syntax withcolumns: ValueTypeandparseWith: Transformproperties inTableInterpreter - Introduced new
cellInColumnbinary operator andTableRowLiteralsyntax for transform expressions - Removed
skipLeadingWhitespaceandskipTrailingWhitespaceproperties fromTableInterpreter
Reviewed changes
Copilot reviewed 72 out of 72 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| expression.langium | Added TableRowLiteral grammar and cellInColumn operator |
| TableInterpreter.jv | Updated documentation and properties to use valuetype schema |
| GTFS*.jv files | Migrated all GTFS interpreters to new syntax with valuetypes and transforms |
| table-interpreter-executor.ts | Refactored to use transform-based parsing instead of direct column definitions |
| constraint-executor.ts | Updated to work with Map-based values instead of single values |
| table.ts | Changed TableRow from Record to Map, added constraint checking |
| value-type-definition-value-type.ts | Added new primitive value type for valuetype definitions |
| cell-in-column-operator-evaluator.ts | Implemented evaluator for new cellInColumn operator |
| Example files (cars.jv, electric-vehicles.jv, gtfs-rt.jv) | Updated to use new syntax |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...ons/tabular/exec/test/assets/table-interpreter-executor/valid-without-header-without-trim.jv
Outdated
Show resolved
Hide resolved
libs/language-server/src/stdlib/domain/mobility/GTFSFareRulesInterpreter.jv
Outdated
Show resolved
Hide resolved
libs/language-server/src/lib/validation/checks/transform-body.ts
Outdated
Show resolved
Hide resolved
libs/language-server/src/lib/validation/checks/block-type-specific/property-body.ts
Outdated
Show resolved
Hide resolved
libs/language-server/src/lib/ast/expressions/evaluators/cell-in-column-operator-evaluator.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Co-authored-by: Copilot
Co-authored-by: Copilot
This PR implements RFC 0020 with a few caveats:
row["column"]torow cellInColumn "column"SheetRowvaluetype, I decided to useCollection<text>Other changes unrelated to the RFC:
skipLeadingWhitespaceandskipTrailingWhitespacehave been removed fromTableInterpreterbecause this can now be part of the parsing transform. We already have a replace operator.