Open blackheaven opened 2 years ago
Thank you very much for your contributions: I'd be happy to review this PR 🙂.
Is there anything I can do to make it merged?
I can take a look early next week :)
I'll be grateful, I take any feedback.
I am very sorry for having you wait so long.
I was slightly busier than expected and I pushed your PR further back in my mind than I should have.
I mainly ensure contribution to the style guide on the parts of the PR where relevant (my review may seem a little repetitive).
No worries, I may be involved in the security track, and I'll try to be focused on that, sorry for the sudden wake up.
I'll handle your feedback later today, thanks.
Was the recent commit made as the result of some external review (e.g. conducted on Slack) or just on your own?
I think it's extremely useful to know how you desire your articles to be used and it is useful for those learning Haskell to see how library code is written. It's been slightly difficult for me to review any of the other more technical articles, but it has been on my mind this time. Given my main role is reviewing grammar and I don't have much knowledge of parsers, it's quite hard to review an article that's mainly code. However, what I could perhaps do, in absence of this knowledge, is learn about one of the simpler libraries, and then give some advice about any changes that could be made to articles (structurally / gramatically) to have them better serve the purpose you want, to be applied to the other articles in general. Would that work?
I'm also able to call for however long if you want over the next week, but I'd rather learn a bit about the libraries first to make use of a call.
You're right, I had a quick chat with someone (I'm not sure they want to be mentioned) to clarify the targeted audience.
You're also right about the structure of the contribution (mostly code), which might not be useful.
I'll have another look at it, so it will be more helpful for beginners, going more in depth of each piece of code.
I had a small look over the ReadP
markdown file and I think the following would be useful in achieving the goal of showing beginners some common Haskell idioms:
An initial mention of how ReadP
and P
are connected before introducing both of them. You do mention that P
represents the current instruction, but perhaps the design pattern used in the module should be emphasised more. The design pattern, where P
essentially represents a parser expression and ReadP
is a continuation used to build a parser expression, is fairly common (especially in language interpreters).
Similarly, although one can think through the underlying code of the P
instances, some readers may be wondering why they exist (or alternatively, why ReadP
exists at all). This may be substantially difficult to explain (and truly understanding code requires understanding history and the intent of the authors): the parser library can be minimally defined, with some modification, by: P
(with no instances defined for it), (+++)
, (<++)
; ReadP
and its instances, along with its operators and derived operators. I believe the other instances exist because it is idiomatic to implement them if possible, and some code is necessitated by Alternative
being a subclass of Applicative
. One must also consider that many of these typeclasses are rather recent.
Explaining the entirety of why the ReadP
code is written as it is is quite complicated, but highlighting some of the idioms commonly seen in code (such as defining all possible instances) may be useful for beginners.
It would also be useful to talk about the value of these idioms (it is harmful to apply idioms without knowing why they exist).
Removing comments and whitespace in code and then explaining the code and comments yourself: some of the code comments are rather short and aren't meant for beginners. For example, to a beginner, it may not immediately be obvious what the Get
, Look
, Fail
, Result
, and Final
operations mean. Similarly, doing this would be a good opportunity to explain the idioms used in designing (<|>)
efficiently, and the idiomatic way in which evaluators (such as run
) are written.
Showing some of the derived operators, as they're useful examples of how parser combinators are used.
Making more concrete comparisons in the conclusion. It isn't immediately obvious how simplicity corresponds to inefficiency. I think more concrete points can be made by comparing ReadP
to other libraries (such as restating in the conclusion how ReadP
operates on strings and returns all possible parses).
I think the order in which you introduce ReadP
's constructs is quite good: the main thing that's needed is a little more explicit explanation of the idioms for beginners.
Submitter checklist