winnow-rs / winnow

Making parsing a breeze
https://docs.rs/winnow
Other
525 stars 40 forks source link

docs: Try to clear up some confusion #485

Closed epage closed 7 months ago

epage commented 7 months ago

Feedback

And every time the docs say "this is simple" or "this is easy" etc., it's really discouraging if you are struggling to understand.

"simple" is meant to convey "basic". To avoid the mismatch between the language and people's potential experience, I've tried to find other words.

One example is take_till vs take_until. Semantically that means the same thing, and their descriptions on the doc's list of combinators, seem to say the same thing just with different words.

The naming has always bothered me but I still don't have a better idea. We can at least cross-reference and clarify the distinctions

Another is the phrase "the longest list of bytes/chars"; it feels ambiguous. Is there a chance it'll return some kind of shorter list? "longest" is a comparison, but it's not clear what the alternative outcome there is.

This is meant to convey "greedy". For take_while, this is true. However, take_til isn't greedy, so this was clarified

Briefly looking at it again, chapters 0 and 1 made sense. Chapter 2 mostly made sense, but it started to become very code-heavy, with minimal explanation. Like for example, https://docs.rs/winnow/latest/winnow/_tutorial/chapter_2/index.html#tokens, the first code block is showing an example of how to process a single token, as noted in the text above it. But then in the next block, any and verify are introduced without any details of what they are or how they are relevant.

We're trying to step through the transformation from lower level code to higher level. I've tried to adjust that section to be more explicitly about that transformation. This also opened up an opportunity to call out the value of using a Parser is the helpers you can then use.

codecov[bot] commented 7 months ago

Codecov Report

Attention: Patch coverage is 81.51659% with 39 lines in your changes are missing coverage. Please review.

Project coverage is 41.83%. Comparing base (18a319a) to head (55700b4).

Files Patch % Lines
src/token/mod.rs 66.66% 16 Missing :warning:
src/combinator/multi.rs 59.25% 11 Missing :warning:
src/combinator/core.rs 69.23% 8 Missing :warning:
src/binary/mod.rs 94.82% 3 Missing :warning:
src/ascii/mod.rs 95.83% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #485 +/- ## ========================================== - Coverage 41.86% 41.83% -0.04% ========================================== Files 18 18 Lines 3036 3043 +7 ========================================== + Hits 1271 1273 +2 - Misses 1765 1770 +5 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

coveralls commented 7 months ago

Pull Request Test Coverage Report for Build 8071627425

Details


Changes Missing Coverage Covered Lines Changed/Added Lines %
src/ascii/mod.rs 23 24 95.83%
src/binary/mod.rs 55 58 94.83%
src/combinator/core.rs 18 26 69.23%
src/combinator/multi.rs 16 27 59.26%
src/token/mod.rs 32 48 66.67%
<!-- Total: 172 211 81.52% -->
Files with Coverage Reduction New Missed Lines %
src/ascii/mod.rs 1 55.75%
src/combinator/parser.rs 1 41.53%
<!-- Total: 2 -->
Totals Coverage Status
Change from base Build 8052977810: -0.03%
Covered Lines: 1273
Relevant Lines: 3043

💛 - Coveralls