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

Have seq! trace the field name when parsing each field #403

Open
epage opened this issue Dec 24, 2023 · 2 comments
Open

Have seq! trace the field name when parsing each field #403

epage opened this issue Dec 24, 2023 · 2 comments
Labels
A-combinator Area: combinators C-enhancement Category: Raise on the bar on expectations

Comments

@epage
Copy link
Collaborator

epage commented Dec 24, 2023

seq! will trace the struct name for the whole thing but we can also trace the name of the individual fields (maybe also the tuple index?). This would automatically provide more context for the user as they go which can help with complex seq! calls like https://github.com/MIDAS-rs/midasio/blob/cc2ed1e54f8fc40eb7bfabd79d490ec4e8ba38b2/src/event.rs#L115-L139

@epage epage added A-combinator Area: combinators C-enhancement Category: Raise on the bar on expectations labels Dec 24, 2023
@DJDuque
Copy link
Contributor

DJDuque commented Dec 27, 2023

I don't think I understand what you are suggesting here. How would the seq! you quoted look like? Is this to help avoiding all the chained flat_maps?

In case it is to help with the chained flat_maps, a nice thing to have could be _temporary variables (prefixed with _ that don't make it into the struct, but can be referenced in later fields).

So the above seq! could look something like:

 pub(crate) fn event_view<'a>(
    endian: Endianness,
) -> impl Parser<&'a [u8], EventView<'a>, ContextError> {
    seq! {EventView {
        event_id: u16(endian),
        trigger_mask: u16(endian),
        serial_number: u32(endian),
        timestamp: u32(endian),
        _event_size: u32(endian).verify(|&n| n >= 8),
        _banks_size: u32(endian).verify(&n| n == _event_size - 8),
        bank_views: dispatch! {u32(endian);
                1 => length_and_then(success(_banks_size), repeat_till0(padded_bank(bank_16_view(endian)), eof)),
                17 => length_and_then(success(_banks_size), repeat_till0(padded_bank(bank_32_view(endian)), eof)),
                49 => length_and_then(success(_banks_size), repeat_till0(padded_bank(bank_32a_view(endian)), eof)),
                _ => fail,
            }.map(|(bank_views, _)| bank_views),
    }}
}

@epage
Copy link
Collaborator Author

epage commented Dec 27, 2023

That is a separate concern and if you want to find a way to solve it, I would recommend opening a separate issue.

winnow has debug tracing for parsers built in. You can do cargo test -F winnow/debug -- test foo -- bar and it will show you each parse operation (that has a trace combinator call) attempt the parse with the result.

See https://docs.rs/winnow/latest/winnow/trace/index.html for an example of what this looks like.

seq! inserts a trace for the struct as a whole but the proposal here is to add a trace on each field so the user automatically gets names for the operations, making it easier to follow what is happening.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-combinator Area: combinators C-enhancement Category: Raise on the bar on expectations
Projects
None yet
Development

No branches or pull requests

2 participants