-
-
Notifications
You must be signed in to change notification settings - Fork 95
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
expand documentation of type signatures and fix inconsistencies #259
Conversation
//. are curried. In JavaScript, though, we may wish to represent the types of | ||
//. functions with arities less than or greater than one. The general form is | ||
//. `(<input-types>) -> <output-type>`, where `<input-types>` comprises zero | ||
//. or more comma–space (<code>, </code>) -separated type representations: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to use (<code>, </code>)
here because GitHub renders (
, )
as (,
) rather than (,
).
I first tried the NO-BREAK SPACE character, but our linter complained about irregular whitespace.
//. Ditto for each other type variable. In the case of the function above, the | ||
//. types to which `a` and `b` resolve may be different, but need not be. | ||
//. | ||
//. Since all Sanctuary functions are curried it's common to see a binary |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currying comes into the picture here with no prelude earlier in the documentation. While I would think it's probably likely that people using Sanctuary will have knowledge of functional terminology, including currying, my experience has been that it isn't a very common term outside of Haskell and FP circles.
Depending on the target audience, would it be helpful to add a parenthetical remark here? Something like:
Since all Sanctuary functions are curried (arguments can be passed in one at a time instead of all at once) it's common to see a binary function ...
or would that be overkill?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ❤️ this suggestion. I've updated the pull request. :)
@@ -182,25 +228,20 @@ | |||
return f(g(x)); | |||
}; | |||
|
|||
// filter :: (Monad m, Monoid m) => ((a -> Boolean), m a) -> m a | |||
// filter :: (Monad m, Monoid m a) => (a -> Boolean, m a) -> m a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be Monoid (m a)
instead of Monoid m a
? (not sure)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wrote this function in Haskell to find out, and you're quite right!
‘Monoid’ is applied to too many type arguments In the type signature for ‘filter'’: filter' :: (Applicative m, Monad m, Monoid m a) => (a -> Bool) -> m a -> m a
//. S.K :: a -> b -> a | ||
//. | ||
//. It must be possible to replace all occurrences of `a` with a concrete type. | ||
//. Ditto for each other type variable. In the case of the function above, the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Users for which english is not native, maybe it would be easier to understand The same
instead of Ditto
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I'll update this. :)
33863a7
to
bc4c57f
Compare
bc4c57f
to
292a3ea
Compare
This is the first of several improvements I plan to extract from #216 in order to make them easier to review. :)