YAML: Support other cases of lists of lists #4187
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The commit 264e8e8 introduced support for list of lists of scalars. But this didn't fix lists of lists of other blocks (like mappings). This commit will also fix these cases. The underlying problem was that some tokens were interpreted twice. This was caused by YamlParser.parseFromInput. This method calculates the prefix of a token by using the substring yamlSource[lastEnd:tokenIdx]. Normally, the characters that were newly interpreted should be marked as used by incrementing the index `lastEnd`. But for sequences this index wasn't updated in every case. It was only updated if there was an anchor or an openingBracketIndex. Because of this, the prefix was used for multiple consecutive sequences in other cases. To fix this, update `lastEnd` in every case for sequences. The yaml-parser has inconsistent behaviour when parsing sequences with dashes. If a dash-sequence has indentation the start- and the event-mark point to the same character: the dash. If a dash-sequence has no indentation the event-mark points to the character AFTER the dash. Because of this, the `lastEnd` would get an offset and one which means the YamlParser would skip one character. Introduce a workaround to correct the `lastEnd` if this faulty case. With these changes, every dash is only interpreted once. That means that in the SequenceBuilder.push method the rawPrefix will contain at most one dash. Because of this, the old fix isn't needed anymore. Remove it. Fixes openrewrite#4176
timtebeek
approved these changes
May 11, 2024
|
You're very welcome 😊 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The commit 264e8e8 introduced support for list of lists of scalars. But this didn't fix lists of lists of other blocks (like mappings). This commit will also fix these cases.
The underlying problem was that some tokens were interpreted twice. This was caused by YamlParser.parseFromInput. This method calculates the prefix of a token by using the substring yamlSource[lastEnd:tokenIdx]. Normally, the characters that were newly interpreted should be marked as used by incrementing the index
lastEnd. But for sequences this index wasn't updated in every case. It was only updated if there was an anchor or an openingBracketIndex. Because of this, the prefix was used for multiple consecutive sequences in other cases.To fix this, update
lastEndin every case for sequences.The yaml-parser has inconsistent behaviour when parsing sequences with dashes. If a dash-sequence has indentation the start- and the event-mark point to the same character: the dash. If a dash-sequence has no indentation the event-mark points to the character AFTER the dash. Because of this, the
lastEndwould get an offset of one which means the YamlParser would skip one character.Introduce a workaround to correct the
lastEndif this faulty case.With these changes, every dash is only interpreted once. That means that in the SequenceBuilder.push method the rawPrefix will contain at most one dash. Because of this, the old fix isn't needed anymore. Remove it.
Fixes #4176
Checklist