Tutorial / Documentation Improvements

[nskobelevs]

Starting a more organised discussion in relation to tutorial improvements, moved over from my original comment on #196 and abridged in some ways and expanded in others.

As of writing, everything below is based on the tutorial as of 0.6.24.

Wasn't quite sure on the best way to split everything so lemme know if any of the threads need to be combined / split further!

[nskobelevs]

Lack of concrete goal

While the tutorial is loosely around parsing hex numbers there's ultimately no goal of having written a specific parser by the end of it. Not having this leads to things like chapter 5 being out of place of the bin, oct and dec parsing aspect which is in some way ignored / not utilised.

I think it would be beneficial of having a target parser that each chapter can build upon, something that can demonstrate usage of various common combinators / parsers in winnow (which the tutorial already does presently well)

If staying on the number literals theme this could be something like parsing a nested list / tree of number literals that may be in any of those cases. The list / tree aspect would allow the repeated aspect to fit better into the tutorial, provide more examples of custom return types if the output is a more context type as well as allowing the tutorial to mention seq! macro (which is amazing).

[epage]

I can fully understand the value of it. This does come with a large editorial cost to find ways to make it go through the entire tutorial and that cost carries forward to every change made from there on out.

I don't think I have the time to commit to that at this moment.

[nskobelevs]

[Chapter 2/3] Imperative parsers / core functions

I can understand having some imperative examples of some parsers for comparison to help the reader better understand the declarative combinator approach but I'm not sure if this is the best way to do it.

Chapter 2 shows 4 ways to match on a token and 3 on a tag and since they're in some way ordered from "worst" to "best" (or at least from least useful to most useful practical examples) the chapter felt like a repetition of feeling like you've learned something just for the next snippet to make that obsolete.

Imperative parsers might have their uses but I'm not sure if they're the most to be shown that early on in the tutorial. The first snippet is 7 lines to parse 1 token - an overcomplicated example that the reader would never use in practice and not actually representative of how winnow would generally be used which in some way can give a bad first impression.

Similarly, the core / low-level Steam functions like next_token, next_slice, reset and the Checkpoint trait also do not feel as relevant this early given winnow provides a lot of common parsers and combinators where for a lot of things you'd never have to reach for those functions. I think they'd be more appropriate for a way later chapter going through the low-lever usage of Steam and how one might use those function in writing their own combinator. (Which relates to #583 and #588).

[epage]

There are two approaches to teaching

• Glossing over the details and getting people up and running quickly
• Starting with a solid foundation of principles and build up from there

They both have their places both for different ways people learn and for how they help people. I don't quite remember my original motivation for the latter but I do think its important

• Support for imperative parsing is one of our core values
• When people need to do things off the beaten path, I've found they tend to hack something up without understanding the underlying principles which can easily lead to bugs down the road.

[nskobelevs]

[Chapter 4/7, General] dispatch + peek usage not properly explained

Chapter 3 has the follow note under sample usage of dispatch:

Note: peek may be useful when dispatching from hints from each case’s parser.

I think the usage of "from X from Y" makes this note hard to parse and understand. Alternative suggestion would be to rephrase it to get rid of the double from such as:

Note: peek may be useful when dispatching based on hints for each (case|branch)'s parser.

 

This note (and the dispatch docs themselves) do not explain why peek would be useful with dispatch or provide any examples.
The sample usage of dispatch above this note uses take(2) to match on the number literal prefix - this is a prime example of where peek would be appropriate since the in the _ => fail case the prefix chars would still end up being consumed (potentially leading to inaccurate error location).

I think this could be worth calling out here since using dispatch with a parser that consumes the input could be considered a footgun in some cases (of the top of my head at least it feels like majority of &str parsers that would use this would not be guaranteed to succeed for every prefix)

[epage]

The error reporting aspects of this would be best for the error chapter. While important, I also don't know if its a general enough principle to call out there.

This comment is mostly to focus on if 0b was part of parse_bin_digits, etc and you didn't want to break that encapsulation, then you peek the prefix and have it re-parsed in parse_bin_digits.

Trying to think of a good way to talk about this without detracting from the flow.

[nskobelevs]

[Chapter 5] Out of place

Repetition is definitely an important aspect to cover in the tutorial but it felt out of place since the repetition aspect was not used in any subsequent chapters.

[epage]

I feel like this is a restating of the concern in #698 (comment) ?

Without a full through-thread narrative, we have to put this somewhere and imo it belongs before we get to app integration, errors, etc.

[nskobelevs]

[Chapter 7] parse_digits return type not in line with Chapter 6

The parser now returns PResult<(&str, &str)> without discarding the prefix or parsing the digits into a number which is relied upon by Hex::from_str from chapter 6.

This change isn't called out and Hex::from_str in the annotate-snippets example does not show usage of this new parser

[epage]

However, this does align with the alt version from earlier, as discussed in #698 (comment)

For myself, I feel like those details aren't relevant to the point being discussed and would instead bloat the examples, making it harder to see whats trying to be highlighted.

[nskobelevs]

[Chapter 7] parse_digits implementation unrelated to implementation from previous chapters

Chapter 4 has the previous full parse_digits implementation which uses dispatch! to match on the prefix. Chapter 7 instead uses the alt approach and while it is shown in chapter 3, it's not the most recent implementation.

Shouldn't this snippet instead built upon the previous dispatch! implementation? (assuming peek has been introduced)

[epage]

I intentionally went back to an alt form because that helps clarify cut_err

[nskobelevs]

[Chapter 8] Visually parsing trace output

trace is a lovely feature but the debugging chapter doesn't really explain how to visually parse the output of it. While it's not overly complex it could still be beneficial to go over the format a bit.

[epage]

Maybe its the curse of knowledge but I'm having a hard time figuring out what needs describing and how. Maybe you could go into what you felt was not clear.

I did make some minor tweaks in #700.

[epage]

For some context, the tutorial was made as a fork of https://tfpk.github.io/nominomicon and has slowly been iterated on from there, including adding more of a connection between sections.

[epage]

Thank you for providing specific, clear feedback!

Something else that would be helpful is looking at it from the lens of what problems you had as you went through. This would help in better prioritizing the feedback and seeing if there are other ways of improving things.