Draft
Conversation
CryZe
commented
Apr 25, 2019
| /** | ||
| * Describes whether the row should be considered an even or an odd row. | ||
| * This is useful for visualizing the rows with alternating colors. | ||
| */ |
Collaborator
Author
There was a problem hiding this comment.
This talks about rows, but in horizontal, we don't even use rows.
CryZe
commented
Jun 28, 2019
For now all the subsplit information is discarded to the point where only normal segment names are left.
The individual subsplits sections are now called segment groups that now exist in the individual form of the SegmentGroup and combined form of the SegmentGroups type. The groups are stored in the Run.
Dealing with the index based group ranges is not ergonomic. Therefore this commits adds an iterator that returns all the groups as views of segment slices. Also segments that are not part of any groups become pseudo-groups of single segments. The splits component should be able to build on top of this.
All the splits without any segment groups behave like they did before and the segment groups now properly open up and close as they are being entered. This is done by transforming the segment groups iterator and implementing an iterator on top that flattens it with a specific segment in focus that determines the group that is supposed to be open. After this flattening almost all the original logic remains, but now with the current segment index and the run's length instead being based on this flattened segment group representation.
It seems like we should have them as a separate block in there, such that old parsers will have no problem simply ignoring that new block and parsing the splits regardless. This however has the disadvantage that the groups can now overlap, which we will have to account for when parsing.
Parsing the segment groups works now and it automatically makes sure the ranges are sorted and don't overlap. There are two remaining problems here though. One is that the group name doesn't differentiate between being empty and not existing here. I don't think we want to differentiate those two case long term anyway as it doesn't make much sense in the Run Editor to do so either. The other problem is that it parses segment groups right now that may be out of the total amount of segments' range. That shouldn't cause any major problems, but those ranges probably don't interact well with the Run Editor, so we need to be wary of that.
This file contains hidden or 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
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.
This is a work in progress implementation of Subsplits. Instead of encoding the subsplits information into the segment names we extend the run format properly so the subsplits information can be stored. This is the roadmap for this Pull Request:
Closes #36