Add replace functions. - Githubissues

snoyberg / mono-traversable

Type classes for mapping, folding, and traversing monomorphic containers

153 stars 63 forks source link

Add replace functions. #107

Closed richsmith92 closed 7 years ago

richsmith92 commented 7 years ago

Added replaceElem and replaceSeq functions

replaceElem old new = omap (\x -> if x == old then new else x)
replaceSeq old new = ointercalate new . splitSeq old

and rewrite rules for Text.

snoyberg commented 7 years ago

This looks good, but can I ask for:

An added test case. For historical reasons, the tests are mostly in classy-prelude, so if you want to add it there, that's fine, but adding in mono-traversable itself is fine too
A .cabal version bump/changelog/@since comment

richsmith92 commented 7 years ago

@snoyberg I added a test case but please review it. It seems a bit too trivial for me (2 of 3 tests just check that function is same as its definition). Besides that, are rewrite rules fired for tests?

richsmith92 commented 7 years ago

also CI build fails on lts-2 because it doesn't have sublistOf in QuickCheck. Not sure if we can ignore this.

snoyberg commented 7 years ago

For sublistOf, how about using #if MIN_VERSION_QuickCheck(...) in the test suite to make it compile with old and new LTSes. That way we still sanity check old LTS versions, but get the extra tests too.

snoyberg commented 7 years ago

Also, I'm OK with this level of complexity for the tests, thank you for adding them 👍

richsmith92 commented 7 years ago

I amended the last commit.

Some checks still fail with

ghc-pkg: streamingProcess: runInteractiveProcess: pipe: resource exhausted (Too many open files)

but I guess it's some kind of a system issue.

snoyberg commented 7 years ago

Yes, just a system issue, I'll restart the builds.