Update Data.Text.Utf16.Rope to Data.Text.Utf16.Rope.Mixed #542
Conversation
|
I finish the migration, @michaelpj @Bodigrim . |
| cuc <- codePointOffsetToCodeUnitOffset utfLine (fromIntegral cpc) | ||
| pure $ J.Position l (fromIntegral cuc) | ||
| lineRope <- extractLine text $ fromIntegral l | ||
| guard $ c <= fromIntegral (Rope.charLength lineRope) |
There was a problem hiding this comment.
I think we should include a comment about why we're not using Rope.splitAtPosition. The difference is quite subtle!
There was a problem hiding this comment.
splitAtPosition just wrapped over the line. when column index out of range or when the column indexing at the newline char :(
There was a problem hiding this comment.
The first out of range problem is easy to deal with,
But the second case when the column indexing at the newline char and still wrapped over the line, it becomes the pain.
| @@ -458,15 +427,12 @@ codeUnitOffsetToCodePointOffset rope offset = do | |||
| the position. | |||
There was a problem hiding this comment.
Hmm, is this definitely still the same? For some reason I thought it might be faster 🤔 But maybe it's just that the code is simpler...
There was a problem hiding this comment.
It just remove the fromText and toText part, saving the conversion, other logic still the same.
since the mixed version have two indices, one for codePoint, one for utf16.
Should be a little faster, since saving the reindexing.
It should simplify a bit of the conversion logic. Although the asymptotic complexity doe not change.
This is the second attempt of #436