Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Export runListT? #83

Closed
kozak opened this issue Jan 14, 2017 · 11 comments
Closed

Export runListT? #83

kozak opened this issue Jan 14, 2017 · 11 comments

Comments

@kozak
Copy link

kozak commented Jan 14, 2017

Should runListT be exported? I stumbled on this when going through this:
https://github.com/Gabriel439/Haskell-List-Transformer-Library/blob/master/src/List/Transformer.hs
where runListT is used.

Ok it seems its a different runListT - so it probably shouldn't be exported.

@matthewleon
Copy link
Contributor

It does seem bizarre that the library defines runListT and then seems to do nothing with it.

@paf31
Copy link
Contributor

paf31 commented Mar 23, 2017

It should be removed since ListT has a Newtype instance now.

matthewleon added a commit to matthewleon/purescript-transformers that referenced this issue Mar 23, 2017
Addresses purescript#83. Superseded by use of unwrap from newtype.
@garyb
Copy link
Member

garyb commented Mar 23, 2017

It should be removed since ListT has a Newtype instance now.

It looks like there is a bug here... ListT's constructor is not exported, nor is Step, the inner type, so I don't think that Newtype instance should be allowed.

@matthewleon
Copy link
Contributor

Is the correct approach to export just the constructor and remove the Newtype instance? That would be breaking.

@garyb
Copy link
Member

garyb commented Mar 23, 2017

We'll be doing some breaking updates soon anyway... but there is a non-breaking solution too: export the stuff that is hidden!

I think I asked @paf31 about it before and he thought that it was better to keep it private though.

@matthewleon
Copy link
Contributor

At the very least, for consistency with the other transformers, it seems to me that runListT should indeed be exported. I imagine that @paf31 has some good reason for keeping it this way?

As for not exposing the ListT constructor, that makes some sense, as nil, singleton, unfold etc. provide ample ways to enter the monad.

@matthewleon
Copy link
Contributor

(adding to the above, perhaps what we really want is some modified version of runListT that "unrolls" the monad into a list, strict or lazy, in the parent monad?)

@matthewleon
Copy link
Contributor

https://hackage.haskell.org/package/list-t-1/docs/ListT.html

For reference, the above package provides such facilities as toList.

@paf31
Copy link
Contributor

paf31 commented Mar 23, 2017

I think I asked @paf31 about it before and he thought that it was better to keep it private though.

I think it's probably fine, especially since we have a Newtype instance anyway now. Let's just export the constructor.

@kozak
Copy link
Author

kozak commented Mar 23, 2017

In the Gabriel's library runListT :: Monad m => ListT m a -> m () is described as 'Use this to drain a ListT, running it to completion and discarding all values. And I think something like this should work:

runListT' :: forall m a. (Monad m) => ListT m a -> m Unit
runListT' = foldl const unit

?

@paf31
Copy link
Contributor

paf31 commented Mar 23, 2017

I think you would need foldl' at least, but there's no reason we couldn't provide that function.

matthewleon added a commit to matthewleon/purescript-transformers that referenced this issue Mar 23, 2017
Addresses purescript#83.

Independent of PR purescript#89.

This brings ListT into line with the other transformers.
matthewleon added a commit to matthewleon/purescript-transformers that referenced this issue Mar 23, 2017
Addresses purescript#83.

Independent of PR purescript#89.

This brings ListT into line with the other transformers.
@paf31 paf31 closed this as completed Mar 30, 2017
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

No branches or pull requests

4 participants