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

Rc 0.6.0 #63

Merged
merged 8 commits into from
Sep 20, 2022
Merged

Rc 0.6.0 #63

merged 8 commits into from
Sep 20, 2022

Conversation

pacak
Copy link
Owner

@pacak pacak commented Sep 18, 2022

Actual changes

What's new in 0.6.0

  • adjacent restriction to parse things in a tighter context
  • catch for many, some and optional to handle parse errors
  • any positional like, to consume pretty much anything from a command line
  • improved documentation with more detailed examples
  • cosmetic improvements
  • a sneaky reminder to use to_options on Parser before trying to run it
  • a way to make boxed parsers with single item construct! macro for making dynamic parsers
  • a horrible way to reduce Yoda-talk coding by placing primitive definitions inside the construct! macro

With new additions you should be able to parse pretty much anything and then some more :)

Migration guide 0.5.x -> 0.6.x

  1. Replace any uses of positional_os and argument_os with positional and argument plus turbofish:
    -let file = positional_os("FILE").help("File to use").map(PathBuf::from);
    +let file = positional::<PathBuf>("FILE").help("File to use");
  2. Replace any uses of from_str with either turbofish on the consumer or with parse, if String is generated inside the parser:
    -let number = short('n').argument("N").from_str::<usize>();
    +let number = short('n').argument::<usize>("N");
    You can still use it for your own types if you implement FromOsStr, alternatively parse still works:
    -let my = long("my-type").argument("MAGIC").from_str::<MyType>();
    +let my = long("my-type").argument::<String>("MAGIC").parse(|s| MyType::from_str(s));
  3. You shouldn't be using those names directly in your code, but there are some renames
    • Positional -> ParsePositional
    • BuildArgument -> ParseArgument
    • Command -> ParseCommand
    • Named -> NamedArg

@pacak
Copy link
Owner Author

pacak commented Sep 19, 2022

@epage I think I'm done with rc 0.6.0 as far as new features and docs are concerned.

You should be able to find up to date docs here: https://pacak.github.io/bpaf/bpaf/index.html

Still open to suggestions on renames. One way would be to rename OptionParser into Parser - now that Parser is a trait they can coexist, and with hidden deprecated .run() in Parser trait to serve as a reminder - it should be harder to confuse those two I think. to_options is still a thing, with OptionParser renamed it can probably be changed to done/finish/make or add_info/with_info/and_info?

Next translating terms into open group base spec language

The utility in the example is named utility_name. It is followed by options, option-arguments, and operands.

  • flag/switch = option, but spec allows flag as a historical name, plus bpaf needs a distinction between those two - I think I'll leave it as is
  • argument = option-argument - not sure if adding option_ in front makes things cleaner in any way, I think I'll stick with that one too
  • positional = operand - Google search for "shell positional" returns a bunch of matches, search for "shell operand" suggest "shell operator", probably positional is close enough. In practice it's "front unconsumed word in current context" though.

Copy link
Contributor

@epage epage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did a quick pass through so you could get this sooner than when i have time to be more thorough.

Comment on lines +33 to +36
- `Positional` -> `ParsePositional`
- `BuildArgument` -> `ParseArgument`
- `Command` -> `ParseCommand`
- `Named` -> `NamedArg`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why Parse prefixes for the first three but not for NamedArg?

Personally, I would do

  • PositionalArg
  • Argument
  • Command
  • NamedArg

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why Parse prefixes for the first three but not for NamedArg?

First three are actually parsers that can extract something, NamedArg needs .flag, .switch or .argument to become one. At the moment any primitive parser starts with Parse so naming is consistent.

For Command - when messing with passing arguments to cargo I found it confusing when there's several things named Command sitting in one namespace: one from bpaf and one from std::process::Command. Having distinct names helps to reduce this kind of confusion. I guess I'll poke my coworkers as well for the input.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I renamed clap::App to clap::Command back in 3.1.0 and no one has brought up concerns over naming conflicts. Granted, it was only deprecated and in clap 3.2 we made deprecations opt-in. We'll see when clap v4 comes out.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well. For that to be reported you need a bunch of factors:

  1. One needs to try to pass some stuff to a child process which might be a problem in general: Partially parse args, capturing unknown args into a vec, rather than error clap-rs/clap#1404
  2. To do this in the same module as the argument parsing - I'm lazy and it's a prototype.
  3. Be actually bothered enough to report. I probably wouldn't be.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(1) seems irrelevant. You just need to deal with Command in the same mod. This can happen because of that, because of how cargo run -- works, or just because

(2) depends on if you do use clap::Command. Personally, I don't. I only like using use for traits.

Changelog.md Show resolved Hide resolved
bpaf_cauwugo/Cargo.toml Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
With new additions you should be able to parse pretty much anything and then some more :)

# Migration guide 0.5.x -> 0.6.x

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One way would be to rename OptionParser into Parser - now that Parser is a trait they can coexist, and with hidden deprecated .run() in Parser trait to serve as a reminder - it should be harder to confuse those two I think. to_options is still a thing, with OptionParser renamed it can probably be changed to done/finish/make or add_info/with_info/and_info?

I would recommend against Parser and Parser as that can be confusing even if they do operate in different namespaces.

As for to_options:

  • done, et all implies it is built but it isn't
  • add_info focuses purely on adding context but there is more going on

I would focus on the core of what it is. It is taking the lower level parsers and wrapping / building / turning them into a full application / user-facing / std::env::args parser.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You also go though OptionParser when implementing commands and subcommands, making tests for your code or parsing things from sources other than std::env::args_os. I think of it as a parser with some extra meta information attached. Interfacing with the lower level is done inside the run method only - and that's the part that needs to be pluggable in order to support windows style flags FWIW.

Changelog.md Show resolved Hide resolved
@pacak pacak merged commit 649303a into master Sep 20, 2022
@pacak pacak deleted the rc-0.6.0 branch September 20, 2022 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants