Skip to content

Assorted improvements - #354

Merged
Lysxia merged 6 commits into
haskell:masterfrom
Bodigrim:another-omnibus
Jul 1, 2021
Merged

Assorted improvements#354
Lysxia merged 6 commits into
haskell:masterfrom
Bodigrim:another-omnibus

Conversation

@Bodigrim

@Bodigrim Bodigrim commented Jul 1, 2021

Copy link
Copy Markdown
Contributor

Data.Text.Internal.Functions in text-1.2.4.1 contains only one function intersperse, whose definition actually matches Data.List since base-4.4. It makes no sense to carry on with this module.

unsafeWithForeignPtr was added to the same module in #325, but this change has not been released yet, and I think Data.Text.Internal.Unsafe is a better place for such function.

I discovered that our tests intentionally disable rewrite rules. This is extremely weird, because it means that rewrite rules (which are always enabled for users) remain completely untested. I don't know good reasons to test configuration with disabled rules, so I enabled them unconditionally. Potentially we can add a dedicated CI build with cabal test --ghc-option=-fno-enable-rewrite-rules, but I'm not sure it's worth it.

words were unlikely to be thoroughly tested before; I've added a special QuickCheck generator, which creates strings with many interestring space characters. I also written some additional tests for other functions.

I was looking for Apple Silicon CI, but it seems there are no free services of this kind so far. So I added drone.io for Ubuntu ARM build, which is better than nothing. E. g., https://cloud.drone.io/Bodigrim/text/4/1/2

Comment thread tests/Tests/Properties/Text.hs
@Boarders

Boarders commented Jul 1, 2021

Copy link
Copy Markdown

Thanks, as always, for the many improvements @Bodigrim!

@Lysxia
Lysxia merged commit 9c67dad into haskell:master Jul 1, 2021
@Bodigrim
Bodigrim deleted the another-omnibus branch August 7, 2021 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants