Add spanEnd and breakEnd to Data.Text

[TheMatten]

Provides part of #244 for Data.Text.

[Lysxia]

This looks good to me. Would any other @haskell/text maintainers like to chime in? Let's merge this with one more approval. This PR seems to actually close #244, or did I miss something else in that issue?

[TheMatten]

It doesn't actually add corresponding functions to Data.Text.Lazy - while I guess that doesn't really matter for this specific PR, it means that #244 isn't technically fully resolved. I can try to add those if you're interested, but I imagine they are going to be slightly less trivial because of list-like nature of lazy Text.

[TheMatten]

Looking at breakOnEnd, would similar implementation using reverse make sense? I'm not familiar enough with performance characteristics of text to tell whether it would break existing optimizations.

[TheMatten]

I've locally prepared commit with

-- | /O(n)/ Similar to 'break', but searches from the end of the string.
--
-- >>> T.breakEnd (=='0') "180cm"
-- ("180","cm")
breakEnd :: (Char -> Bool) -> Text -> (Text, Text)
breakEnd p src = let (a,b) = break p (reverse src)
                 in  (reverse b, reverse a)
{-# INLINE breakEnd #-}

, spanEnd inmplemented in terms of it, and some fixes of whitespace in spanEnd_.

[Bodigrim]

Would any other @haskell/text maintainers like to chime in?

@Lysxia you can request reviews from other maintainers, adding them under the gear in the top right corner of "Reviewers" panel.

[Bodigrim]

@TheMatten would it be possible to add tests for new functions?

[Lysxia]

@Bodigrim Where would be a good place to add such tests? There doesn't seem to be a unit-testing suite, is there? It might be good to document this somewhere too.

[Bodigrim]

@Lysxia I suggest adding tests somewhere near

text/tests/Tests/Properties.hs

Lines 621 to 622 in 0998ad0

	t_span p = L.span p `eqP` (unpack2 . T.span p)
	tl_span p = L.span p `eqP` (unpack2 . TL.span p)

[Lysxia]

Thanks. I didn't realize we could (and were already) using Data.List as a reference implementation for property testing.

[TheMatten]

@Bodigrim no problem. Should I include mentioned Lazy versions too?

[Bodigrim]

Yeah, it would be nice.

[Lysxia]

Ping.

1. Add tests.
2. (optional) Add lazy versions.

[TheMatten]

Oops, sorry, I forgot to upload changes - will do so today.

[TheMatten]

Should I rebase it on current master?

[Lysxia]

Yes please :)

[Bodigrim]

I'm afraid that reversing is very expensive and annihilates all benefits of lazy Text. Would it be possible to have something lazier?

[TheMatten]

You mean something that wouldn't touch chunk contents on left side of text being broken?

[TheMatten]

See c76b9aa

[TheMatten]

I've rebased on top of current master.

[Bodigrim]

Looks good, but please do not enable OverloadedStrings in this module.

Would it be possible to implement via foldrChunks? Something like

breakEnd :: (Char -> Bool) -> Text -> (Text, Text)
breakEnd p = foldrChunks go (empty, empty) 
    where 
        go x (ys, zs) 
            | null ys   = let (y, z) = T.breakEnd p x in (chunk y empty, chunk z zs)
            | otherwise = (chunk x ys, zs)