bpaf icon indicating copy to clipboard operation
bpaf copied to clipboard

bpaf_derive: support `help` annotation on enum branches

Open pacak opened this issue 8 months ago • 13 comments

#[derive(Debug, Clone, Bpaf)]
enum Foo {
    #[bpaf(help("custom help"))] // <- this
    Kospi200
}

pacak avatar Mar 29 '25 13:03 pacak

How should it map?

lu-zero avatar Oct 27 '25 18:10 lu-zero

How should it map?

It depends. In the example I gave it should behave the same way as doc comment would behave, so something like this:

fn foo() -> impl ::bpaf::Parser<Foo> {
    #[allow(unused_imports)]
    use ::bpaf::Parser;
    ::bpaf::long("kospi200")
        .help("custom help")
        .req_flag(Foo::Kospi200)
}

If enum is not a unit type and contains stuff inside - it should map into group_help

#[derive(Debug, Clone, Bpaf)]
enum Foo {
    /// custom help
    #[bpaf(help("custom help"))]
    Kospi200(
        /// Show results from a binary
        #[bpaf(long("bin"), argument("BIN"))]
        String,
    ),
    Bar
}

Either "custom help" annotation should produce something like. If both are present - help takes priority.

fn foo() -> impl ::bpaf::Parser<Foo> {
    #[allow(unused_imports)]
    use ::bpaf::Parser;
    {
        let alt0 = {
            let f0 = ::bpaf::long("bin")
                .help("Show results from a binary")
                .argument::<String>("BIN");
            ::bpaf::construct!(Foo::Kospi200(f0,))
        }.group_help("custom help");
        let alt1 = ::bpaf::long("bar").req_flag(Foo::Bar);
        ::bpaf::construct!([alt0, alt1,])
    }
}

pacak avatar Oct 27 '25 18:10 pacak

It's more like a TODO for myself for the next big version, if it's bothering you - I should be able to fix it sooner in the current one.

pacak avatar Oct 27 '25 18:10 pacak

Just I saw less activity and I might have another slice of time to help a little.

lu-zero avatar Oct 27 '25 18:10 lu-zero

I see. I was thinking about cleaning up the derive macro anyway - it was one of the earlier derives I made - some things might be suboptimal :) This specific change might not survive, but the test would be useful.

Actually if you have some time - I would appreciate a closer look at the whole derive crate and just a list of your thoughts on how it can be improved. If you have more experience writing deriving code than I had back then when I made this thing.

pacak avatar Oct 27 '25 18:10 pacak

From a quick look it seems fairly nice. I would just change the expect( to a compiler_error.

Do you want to try using unsynn for the next version ?

lu-zero avatar Oct 27 '25 19:10 lu-zero

Do you want to try using unsynn for the next version ?

Unfortunately on average this will only make compilation slower. Even if bpaf doesn't pull in syn - something else will.

pacak avatar Oct 27 '25 19:10 pacak

I ended up implementing bpaf_unsynn as experiment, I can send you a PR with the initial impl or you'd prefer if I can keep it as a stand alone project.

lu-zero avatar Nov 30 '25 06:11 lu-zero

I'm definitely interested, will try to check it out soon.

As for where it goes - might switch to it in 0.10. Need to check a few things.

0.10 is a big rewrite anyway.

Current (0.9) version runs parsers in order and tracks what was parsed, but in order to deal with parsing both positional and named items - it requires to put positional items at the end, recursively - this is mildly annoying even for me and confusing for newcomers. An approach I'm trying is to run parsers in order to consume the items with some async magic. First attempt is in bpaf-core branch, mostly working but I didn't like the organization at the end.

Right now I'm trying a slightly different approach, with using async only to create state machines and it works surprisingly well. I think I've solved all the blockers and API will stay 99% the same, so unless you used type names it should be a drop in replacement as well.

Currently I'm working on porting error message generation logic. Parts missing are help/markdown/man generation - I think I'll reuse them from the bpaf-core branch.

Before the release I also want to clean up the documentation system. Right now it's a separate crate that compiles the examples, generates files to be included - confusing to contributors and I made it before rust learned how to run all the doc tests in a single binary.

At least that's my plan of action.

pacak avatar Nov 30 '25 13:11 pacak

I'm definitely interested, will try to check it out soon.

As for where it goes - might switch to it in 0.10. Need to check a few things.

I didn't check how many changes would be needed to get there, since it is an experiment we can play there.

0.10 is a big rewrite anyway.

Current (0.9) version runs parsers in order and tracks what was parsed, but in order to deal with parsing both positional and named items - it requires to put positional items at the end, recursively - this is mildly annoying even for me and confusing for newcomers. An approach I'm trying is to run parsers in order to consume the items with some async magic. First attempt is in bpaf-core branch, mostly working but I didn't like the organization at the end.

I added a compile time error for that in the proc macro for the basic pitfall since it also annoyed me :)

Right now I'm trying a slightly different approach, with using async only to create state machines and it works surprisingly well. I think I've solved all the blockers and API will stay 99% the same, so unless you used type names it should be a drop in replacement as well.

Currently I'm working on porting error message generation logic. Parts missing are help/markdown/man generation - I think I'll reuse them from the bpaf-core branch.

Before the release I also want to clean up the documentation system. Right now it's a separate crate that compiles the examples, generates files to be included - confusing to contributors and I made it before rust learned how to run all the doc tests in a single binary.

As end-user, the documentation is very nice, probably if it could be made so all the examples double as examples would be cool.

One of the outcomes of the bpaf_unsynn experiment is that now we have a nice corpus that gives nearly 100% coverage for what the proc-macro should do.

lu-zero avatar Nov 30 '25 14:11 lu-zero

I added a compile time error for that in the proc macro for the basic pitfall since it also annoyed me :)

I didn't do it because it covers only the basic case. If parser is external - it won't work.

As end-user, the documentation is very nice, probably if it could be made so all the examples double as examples would be cool.

Right. That's the plan. Just thinking if I want to make any changes to the organization. Separate crate and generated files is good but a bit too much steps. Plus I no longer need to combine tests manually. Maybe make something that could extract doc test examples/cases into files and splice the results back? Or have them as standalone examples and scrape/plug them into documentation? Not sure yet.

pacak avatar Nov 30 '25 14:11 pacak

Sadly scraped-examples is still unstable. so it is not immediately straightforward yet :/

lu-zero avatar Nov 30 '25 15:11 lu-zero

Sadly scraped-examples is still unstable.

This is actually fine and I'm already using it. You can just pass whatever unstable flags docs.rs will build it with. Problem is that you can easily specify passed inputs and expected outputs.

pacak avatar Nov 30 '25 15:11 pacak