Conversation
sjakobi
left a comment
There was a problem hiding this comment.
Thanks for the PR! :)
I have a few comments.
There are also some CI failures that need fixes.
| -- | identical to the strict version | ||
| unsafeLazyTextWithLength :: Lazy.Text -> Int -> Doc ann | ||
| unsafeLazyTextWithLength txt l = Text l (Lazy.toStrict txt) |
There was a problem hiding this comment.
What's the use case for this function? I don't imagine that wide characters and emojis will typically exist in the form of a lazy Text values?!
If there's no obvious use case, I'd prefer not to add this function.
| -- | Convert text to Doc. Must not contain newlines. | ||
| -- Useful when dealing with wide characters and emojis |
There was a problem hiding this comment.
The documentation should clarify why and how this function is useful for dealing with wide characters and emojis.
| -- | Convert text to Doc. Must not contain newlines. | ||
| -- Useful when dealing with wide characters and emojis | ||
| unsafeTextWithLength :: Text -> Int -> Doc ann | ||
| unsafeTextWithLength txt l = Text l txt |
There was a problem hiding this comment.
I think this definition is a bit misplaced between these Pretty instances. I suggest placing it below unsafeTextWithoutNewlines.
| -- | Convert text to Doc. Must not contain newlines. | ||
| -- Useful when dealing with wide characters and emojis | ||
| unsafeTextWithLength :: Text -> Int -> Doc ann | ||
| unsafeTextWithLength txt l = Text l txt |
There was a problem hiding this comment.
The Text invariants need to be handled in some way too: https://github.com/quchen/prettyprinter/blob/0dbe08f65468697b729d5e05c98c89a58a42486b/prettyprinter/src/Prettyprinter/Internal.hs#L153-L159
|
I think that invariant should be handled by user because single character may have different length. |
Add
unsafeTextWithLengthaccording to #103