-
Notifications
You must be signed in to change notification settings - Fork 71
Add support for separator alignment in sequences #1594
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
- Implement logic to calculate prefix, infix, and postfix separator alignments and lengths in sequence terms. - Updated `priorAlignmentApprox` and `endingAlignmentApprox` to incorporate separator alignments and lengths when determining alignment. - Refactored `encodingLengthApprox` to reuse new separator-related alignment methods. - Added new test cases `test_sep_alignment_1`, `test_sep_alignment_2`, and `test_sep_alignment_3` for validation of separator alignments. - Updated TDML files with corresponding test data and schema definitions. DAFFODIL-2295
848e35f to
7111fa5
Compare
stevedlawrence
left a comment
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.
+1, question if we also need to consider model groups with separators
| } | ||
| } | ||
|
|
||
| private lazy val priorAlignmentWithLeadingSkipApprox: AlignmentMultipleOf = { |
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.
Maybe we should rename this, since it's now a bit more than just prior alignment with leading skip. I think it's correct to all separators, leading skip, initiators (not currently implemented), etc. as "framing"? So maybe this wants to be priorAlignmentWithLeftFramingApprox?
| + separatorLengthApprox | ||
| endingAlignmentWithSeparatorApprox | ||
| } | ||
| case mg: ModelGroup => { |
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.
I believe model groups can also have postfix separators if the model group is a child of a separated sequence, so the above logic can't just be for ElementBase.
Does the postifx separator mta and length always needs to be added to the result of the match case? So where this thing ends (whether it be an element or a model group) we add the approximate alignment?
- Added `test_sep_alignment_4` to validate postfix separator alignment with complex nested sequences. - Refactored alignment logic, renaming `priorAlignmentWithLeadingSkipApprox` to `priorAlignmentWithLeftFramingApprox` and introducing `endingAlignmentWithRightFramingApprox` to handle right framing more intuitively. DAFFODIL-2295
priorAlignmentApproxandendingAlignmentApproxto incorporate separator alignments and lengths when determining alignment.encodingLengthApproxto reuse new separator-related alignment methods.test_sep_alignment_1,test_sep_alignment_2, andtest_sep_alignment_3for validation of separator alignments.DAFFODIL-2295
(Need to add ticket numbers to to-do, and running regression test suite to see if this broke anything)