snoyberg / mono-traversable

Type classes for mapping, folding, and traversing monomorphic containers
153 stars 63 forks source link

RFC: Cleanups #95

Closed snoyberg closed 8 years ago

snoyberg commented 8 years ago

Now that mono-traversable has been in active use for a while, it's worth reviewing design points and seeing if we can do better. I've given some thought to the following points, and would appreciate feedback on them:

I'm not 100% behind all of these ideas, I'm sharing them now to try and elicit useful feedback.

gregwebs commented 8 years ago

I have once found it useful to declare a MinLen of 2. I think the o prefix is still useful to avoid import conflicts and to give mono-traversable the chance to make differences with foldable and the opportunity to see different error messages.re-exporting without the prefix seems to work for classy-prelude

snoyberg commented 8 years ago

Actually, having a Prelude with both the Data.Foldable and Data.MonoTraversable identifiers all exported sounds like an interesting idea. OK, scratch the renaming thought.

Is there any objection then to other things I put in this list?

gregwebs commented 8 years ago

Sounds fine. I like the idea of MinLen as long as NonNull can still present its own interface and error messages

snoyberg commented 8 years ago

Alright, it took a while for me to free up time, but I've finally taken a stab at this. The work is on various various branches of different repos:

Unexpected outcomes:

This addresses most of the points above. There are two questions that have come up for me which are worth exploring:

I have no particular time pressure on releasing these changes, so I'm fine with taking a bit more time if warranted.

gregwebs commented 8 years ago

Sounds great! As long as documentation mentions mono-traversable-instances it doesn't matter to me if stuff is moved there. You should make all the changes you are confident in now and come back to Containers later.

snoyberg commented 8 years ago

This is merged to master, closing. Thanks for all the feedback :)

saurabhnanda commented 8 years ago

I'm not sure if I'm qualified to comment here, or if this is the right place to give feedback, but here's my experience as an application author (as against a library author).

I was exposed to ClassyPrelude via the Yesod scaffolding template and I was under the impression that it's just a saner version of the same old Prelude functions that I'm used to. That is, until I was hit with http://lpaste.net/178218 The error message made absolutely no sense to me, and for a moment I thought that ClassyPrelude had changed the core list ([]) type to act funny.

I dug deeper into ClassyPrelude and discovered MonoTraversable. However, I found no mention of MinLen in the MonoTraversable documentation. Digging even deeper made me realise that MinLen was there in 0.10.2 (which is what the Yesod scaffolding template gives up to LTS 6.12). Finally, discovering the ChangeLog pointed me to this issue and made me realise that MinLen was pretty much being deprecated.

Now, I'm in a fix. I'm on LTS-6.6 which is frozen (?) to v0.10.2 of MonoTraversable, which has defined regular functions like head in terms of MinLen, which is clearly not the way forward. However Data.NotNull in that version is clearly marked as experimental.

I know that I'll have to bite the bullet and figure out how to get the latest version of ClassyPrelude and MonoTraversable to work with LTS-6.6. However, here are a bunch of related questions: