Documentation and new parsers rest,take,eof#140
Conversation
jamesdbrock
force-pushed
the
jbrock-docs
branch
2 times, most recently
from
dd1b086 to
624ba68
Compare
|
@JordanMartinez @robertdp @thomashoneyman would one of you please review? |
jamesdbrock
force-pushed
the
jbrock-docs
branch
from
624ba68 to
6d186aa
Compare
- `Parser.String.rest` - `Parser.String.take` - `Parser.Token.eof` Bugfixes: - `Parser.String.eof` Set consumed on success so that this parser combines correctly with `notFollowedBy eof`. Added a test for this.
jamesdbrock
force-pushed
the
jbrock-docs
branch
from
6d186aa to
736f863
Compare
jamesdbrock
requested review from
JordanMartinez,
robertdp and
thomashoneyman
robertdp
left a comment
robertdp
left a comment
There was a problem hiding this comment.
Looks good, thanks James.
| Parsing starts at the beginning of the input string. | ||
|
|
||
| Parsing requires two more capabilities: *choice* and *failure*. | ||
| Parsing requires two more capabilities: __alternative__ and __failure__. |
There was a problem hiding this comment.
The "choice" -> "alternative" change makes the language fairly awkward. What was the reason for changing it?
There was a problem hiding this comment.
Because “alternative” is the term used by the Alt typeclass. For clarity I like to try to always use the same word when I'm talking about the same thing.
There was a problem hiding this comment.
I think "choice" is a more familiar word to use here because you clarify what you mean by "choice" in the next few paragraphs: Alt.
| To run a parser, call the function `runParser :: s -> Parser s a -> Either ParseError a` in the `Text.Parsing.Parser` module, and supply it with an input string and a parser. | ||
| To run a parser, call the function `runParser :: s -> Parser s a -> Either ParseError a` in | ||
| the `Text.Parsing.Parser` module, and supply it with an input string and a parser. | ||
| If the parse succeeds then the result is `Right a` and if the parse fails then the |
There was a problem hiding this comment.
How come there are all these extra line breaks in this file?
There was a problem hiding this comment.
Just for editing convenience.
There was a problem hiding this comment.
Yeah, I'm curious why this was done too because it messes up the git blame. What does "editing convenience" mean?
|
Thanks for the review @robertdp |
|
The added docs look good and definitely help clarify things. I also didn't know about the 'try considered harmful' paper, so I'd like to read that on my own time. So, in all, this is a net gain. That being said, I agree with the concerns @robertdp raised. But since this has already been merged, 🤷♂️ 😄 |
3 participants
Description of the change
Add parsers:
Parser.String.restParser.String.takeNParser.Token.eofBugfixes:
Parser.String.eofSet consumed on success so that this parser combinescorrectly with
notFollowedBy eof. Added a test for this.Also added a lot of documentation.
Resolves #25 #110 #100 #99 #68
Checklist: