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

Add FoldM1 #219

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Add FoldM1 #219

wants to merge 4 commits into from

Conversation

Topsii
Copy link
Collaborator

@Topsii Topsii commented Dec 28, 2024

I do not export the constructor FoldM1'. Instead I export the pattern synonym FoldM1, which has a Monad constraint. This way we retain the option to later change the datatype definition to

data FoldM1 m a b = forall x. FoldM1 (a -> m x) (x -> a -> m x) (x -> m b)

Note that currently hoists or the Functor instance can only be implemented with FoldM1' because they do not require Monad m.

I also changed the handler type signatures from Handler a b to Handler s a to match the s t a b naming scheme for optics.

@Topsii Topsii force-pushed the foldm1 branch 3 times, most recently from 095f66c to 49b9b86 Compare December 29, 2024 19:46
Copy link
Owner

@Gabriella439 Gabriella439 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one comment, otherwise this looks great!

@@ -375,16 +601,77 @@ minimumBy cmp = Fold1 (\begin -> Fold min' begin id)
_ -> x
{-# INLINABLE minimumBy #-}

-- | Upgrade a fold to accept the 'Fold1' type
-- | Upgrade a "non-empty" fold to accept the 'Fold1' type
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you want to escape quotes, otherwise haddock tries to interpret them as a module reference. Same comment for the other quotes in the documentation:

Suggested change
-- | Upgrade a "non-empty" fold to accept the 'Fold1' type
-- | Upgrade a \"non-empty\" fold to accept the 'Fold1' type

@Topsii Topsii force-pushed the foldm1 branch 2 times, most recently from a902c0c to 67c7826 Compare January 4, 2025 16:55
With background shading the rounded corners between two immediately
consecutive inline code spans would be visible. Merges these
inline code spans into a single one to avoid this.
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.

2 participants