-
Notifications
You must be signed in to change notification settings - Fork 116
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 Semigroup/Monoid to Parser #463
Comments
Given that the |
Hmm, this might be a bit above my head, do you mind elaborating? I get that Gabriella also discusses this a bit:
This seems to be specifically not that, as the proposed
I'm not sure what confusion you mean exactly. Maybe I've internalized something that's unusual, but seeing |
I didn't want to express anything particularly deep, just that |
As a data point, in some other library I wrote I have a functor |
Yup, just want to make sure I'm not missing something. (I also just find all this stuff fascinating, so thank you for engaging).
I think you're saying that because, instance Semigroup (Parser a) where
(<>) = (<|>)
instance Semigroup a => Semigroup (Parser a) where
(<>) = liftA2 Are both reasonable things to do, it presents confusion to the user which the library happens to be doing. To that I would counter two points:
I know you said you don't have a strong opinion, and neither do I (believe it or not). As you mentioned, it doesn't clean up too much. So I won't take up any more of your time. Feel free to close, or give me the go-ahead to make the PR -- up to you. |
G'day, I thought I'd chime in. I'm generally of the belief that instances should really only exist if there's one canonical / possible version of them. I'm also pretty skeptical of using foldMap in places where there is a monad or applicative involved because one has to think hard about what that effect does. For example: The other day I wrote somethng like this code, actually thinking about the post above: let
melt :: a -> Maybe [a]
melt = _
apcat = liftA2 (<>)
in
foldMap melt as `apcat` pure [a] `apcat` foldMap melt bs Here, The idea was for a list of as and bs, if they can all be melted I can build up the final list. There are two functions the scala cats library which would do the trick flatTraverse
:: (Monad m, Traversable m, Applicative f)
=> (a -> f (m b)) -> m a -> f (m b)
flatTraverse f xs
= join <$> traverse f xs and foldMapM
:: (Monad m, Monoid w, Foldable t)
=> (a -> m w)
-> t a
-> m w
foldMapM f = foldlM
(\acc a -> do
w <- f a
return $! mappend acc w)
mempty Obviously the foldMapA
:: (Monoid b, Traversable t, Applicative f) =>
(a -> f b) -> t a -> f b
foldMapA f as = fold <$> traverse f as I'm not really trying to convince you to use these functions everywhere, but I do believe that sometimes we can make things a bit too polymorphic, which can cause bugs like the one I had above if there aren't proper laws in place or there's more than one way to write the instance. |
Would you accept a documentation PR -- to the |
👋 Hi there-
I'm wondering if you would accept a PR to add:
Apologies if this has been discussed elsewhere, I did a light search and couldn't find anything.
Gabriella Gonzalez has a good general justification for why this is useful. My own concrete use cases usually revolve around building up a settings modifier by option flags (vs parsing the settings structure itself).
For example, supporting
--log-level=<level>
or--debug
neatly by turning both into anEndo LogSettings
modification:With the above instance, this is somewhat simpler,
Or in this case I'm adding a
--{language}
for every value in aLanguage
enumeration.This one could be,
In general, I'm indeed finding being able to
fold
,m/sconcat
or<>
anyParser
of a Semigroup or Monoid value pretty useful, as Gabriella indicated I would.The text was updated successfully, but these errors were encountered: