-
Notifications
You must be signed in to change notification settings - Fork 43
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
Use modern definitions for Monad and Alternative #46
base: main
Are you sure you want to change the base?
Conversation
…onad` Use `Alternative` instead of `MonadPlus`
I will wait for any more input on this and possibly a review before we merge. Thanks for the contribution! 🙂 |
It looks like you edited both Care should be taken that this PR does not introduce inconsistencies with the rest of the book. A priori I do not expect this problem to be localized to a single chapter. |
@MatthijsBlom I ran |
So the |
Yep |
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.
LGTM after these changes.
Afaik, the markdown version isn't used yet, so this change wouldn't be visible as it is rn. cc @xogcox |
Co-authored-by: konsumlamm <[email protected]>
Co-authored-by: konsumlamm <[email protected]>
Co-authored-by: konsumlamm <[email protected]>
Co-authored-by: konsumlamm <[email protected]>
Co-authored-by: konsumlamm <[email protected]>
Hi, thanks for pinging me! It seems like I haven't fully communicated the status of the markdown files, and there's some confusion to clear up. As you say, @konsumlamm, the markdown isn't in use yet. Not only that, nothing below the /markdown directory is in use yet. At the moment the markdown and generated html in this directory are a proof of concept, and there are lots of subtle differences from the live site, which is in the /docs directory. So all of the edits in this PR are edits to a work in progress that doesn't reflect the actual live website. For clarity, maybe I should explain the course of events. As you probably know, the original Learn You A Haskell website didn't have markdown, only raw HTML. So I wrote a markdown version that was as close as possible to the original version (while using HTML tags inside the markdown only when necessary). But as I said above, there are lots of little differences between the markdown-generated HTML and the original. Most of the differences can't be seen when viewing the website in the browser, but lots of tag names are different, and of course the line breaks aren't the same, so there are changes to almost every line. Normally, if there's a markdown version available, it makes sense for the markdown to be the single source of truth. There's not much point editing the markdown if you have to edit the HTML by hand anyway. But I didn't feel like I could just say "Hey, delete all the original HTML and use my markdown version instead, it's much better!" Firstly, I don't have the authority to make that decision. Also, what if the HTML got replaced, and it broke the CSS layouts, for example? There could be new bugs that would be difficult to track down. So I thought that, to clear the way to potentially adopting the markdown, I should document all the changes that would be required, step by step, to turn the current website's HTML into the HTML generated by the markdown, in the form of a commit history. That way there'd be less risk of sudden surprise bugs, and if there were any bugs or unintended differences, they could be traced back to the change that caused them more easily. It would also show exactly what changes would be caused by moving to the markdown version, which would hopefully reassure everyone that the change would be painless. The result of this work is PR #41, which is quietly waiting to be looked at. It hasn't had much attention... maybe nobody was sure what its purpose was? I thought maybe people had lost interest or were busy with other things. But if you want to use the markdown, please take a look at that PR. It makes all the step-by-step changes necessary for the HTML in the /docs directory to match the markdown-generated HTML and also makes a few necessary edits to script files, CSS and Javascript so that they work properly with the HTML. TL;DR I highly recommend that you consider pull request #41 before merging PRs that edit the markdown data. It should be easier to merge #46 on top of #41 than the other way around. Also, after #41 you can just copy the generated HTML files into the /docs folder, since it makes the HTML files in /markdown/generated-html and /docs identical. I'm glad to see that this project is active! Please feel free to give me feedback or ask if anything is unclear. |
functor, even if the <code>Monad</code> class declaration doesn’t say | ||
so.</p> | ||
<code>class Applicative => Monad m where</code> which means that if | ||
we want to create an instance of Monad for some time, we must have an |
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 "for some time" be "for some type"?
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.
Yes, this was already updated in the .md
source, but the HTML hasn't been regenerated.
and wrap it in a <code>Just</code>.</p> | ||
Just x >>= f = f x</code></pre> | ||
<p>Both <code>return</code> and <code>(>>)</code> have <em>default | ||
implementations</em>, so we omit them in instances. <code>return</code> |
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.
Pandoc converts _italic text markdown_
to <em>
tags, but unfortunately, the current /docs/style.css
file redefines em
to be bold. That means that to include italic text, you might have to change the CSS.
so.</p> | ||
<code>class Applicative => Monad m where</code> which means that if | ||
we want to create an instance of Monad for some time, we must have an | ||
instance of Applicative for that type.</p> | ||
<p>The first function that the <code>Monad</code> type class defines is | ||
<code>return</code>. It’s the same as <code>pure</code>, only with a | ||
different name. Its type is <code>(Monad m) => a -> m a</code>. It |
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.
Because the line return = pure
was added, maybe change this part to say not just that return
always does the same as pure
but that it's explicitly defined to be pure
?
@@ -305,22 +296,16 @@ <h2 id="the-monad-type-class">The Monad type class</h2> | |||
attention to it for now because it comes with a default implementation | |||
and we pretty much never implement it when making <code>Monad</code> | |||
instances.</p> | |||
<p>The final function of the <code>Monad</code> type class is |
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.
Maybe change the preceding paragraph to say that >>
is the final function of the type class, instead of "next up", since the paragraph on fail
has been deleted?
… on the current website
… on the current website
@konsumlamm @xogcox would you be interested in getting collaborator rights? I see you are fairly active. |
Sure! I already accepted the invitation. |
Thank you for the invitation! I've joined too. I'll try to be useful. |
No pressure. It's a fun project. |
Signed-off-by: xogcox <[email protected]>
… on the current website
Is this still ongoing? |
Resolves #5