Skip to content
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

Improve docs by adding parse earlier #529

Closed
2 tasks done
woile opened this issue May 30, 2024 · 4 comments
Closed
2 tasks done

Improve docs by adding parse earlier #529

woile opened this issue May 30, 2024 · 4 comments
Labels
C-enhancement Category: Raise on the bar on expectations

Comments

@woile
Copy link

woile commented May 30, 2024

Please complete the following tasks

winnow version

1.65

Describe your use case

The docs don't make enough use of parse till Tutorial 7. I think introducing earlier would help people close the gap between the initial parser and the finished app.

On top of that, it makes winnow "less scary", because the &mut str is only required while parsing, you can still use your &str with it, which doesn't have to be mutable (does it make sense?)

The way I usually approach it, is to write a simple parser and then I want to see it working.

Describe the solution you'd like

I propose adding parse earlier on.
On:

  • chapter 1 (either change the existing example, which currently uses parse_next, but at that point I think it should be used the user facing parse, or at least mention it)
  • The main example in libs.rs I think it should be changed, the parse inside the Color::from_str IMO is a very important point, but it's a bit lost. And it's not used on css/main.rs as far as I can see, I propose keeping it as simple as possible.

Something like this, would highlight the most important parts of the parser:

use winnow::combinator::seq;
use winnow::prelude::*;
use winnow::token::take_while;

#[derive(Debug, Eq, PartialEq)]
pub(crate) struct Color {
    pub(crate) red: u8,
    pub(crate) green: u8,
    pub(crate) blue: u8,
}

fn hex_primary(input: &mut &str) -> PResult<u8> {
    take_while(2, |c: char| c.is_ascii_hexdigit())
        .try_map(|input| u8::from_str_radix(input, 16))
        .parse_next(input)
}

pub(crate) fn hex_color(input: &mut &str) -> PResult<Color> {
    seq!(Color {
        _: '#',
        red: hex_primary,
        green: hex_primary,
        blue: hex_primary
    })
    .parse_next(input)
}

fn main() {
    let input = "#AAAAAA";

    match hex_color.parse(input) {
        Ok(result) => {
            println!("  {:?}", result);
        }
        Err(err) => {
            println!("  {}", err);
        }
    }
}

Thoughts? I don't mind sending some PRs

Alternatives, if applicable

No response

Additional Context

No response

@woile woile added the C-enhancement Category: Raise on the bar on expectations label May 30, 2024
@epage
Copy link
Collaborator

epage commented May 30, 2024

For the tutorial,

  • I'm not seeing how to fit parse in. We need to be using parse_next as part of teaching what the parser is doing. Understanding it is incremental is an important concept.
  • The tutorial is focused on people writing parsers. The top-level parse operation is a small piece to the overall structure.

As far as the Color example, I get where you are coming from but it is also important to idiomatic code and that is how it would be written in an idiomtic way.

@woile
Copy link
Author

woile commented May 30, 2024

Maybe for the tutorial, in chapter 1, it could be a small mention right after the example.

Do not confuse parse_next with parse, the latter is used as the entry point for the top level parse, while parse_next is used when parsers call other parsers

What do you think?

As far as the Color example, I get where you are coming from but it is also important to idiomatic code and that is how it would be written in an idiomtic way.

I think the example you see the moment you go to https://docs.rs/winnow/latest/winnow/ can be a bit hard for beginners. But I agree with your point.

Feel free to close the issue if it doesn't apply, I think my main wish would be to see the parse highlighted somewhere early on

@epage
Copy link
Collaborator

epage commented Jun 4, 2024

What are your thoughts on #536?

@woile
Copy link
Author

woile commented Jun 6, 2024 via email

@epage epage closed this as completed Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: Raise on the bar on expectations
Projects
None yet
Development

No branches or pull requests

2 participants