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

Some command line argument parsing with clap #9

Merged

Conversation

george-cosma
Copy link

In this Pull Request, I've implemented some basic command line argument interpretation using clap. This is in no way a full translation from the python version, just some basic building blocks.

The python version uses two base categories of "arguments":

  • app arguments, which are shared by most subcommands which interact with the apps
  • channel arguments, which configure the computer-board communication

Some subcommands do have additional arguments.

I've implemented the two categories (with the exception of the deprecated "jtag" arguments), and I've made a simple proof of concept, to test that clap is indeed parsing correctly.

Some screenshots:
image
image
image

P.S. I attempted to implement the command line arguments with "derive", but I did not manage to find a way for subcommands to inherit arguments. This would've meant explicitly typing out all of the arguments for all of the subcommands.

rust/Cargo.lock Outdated Show resolved Hide resolved
rust/src/cli.rs Outdated Show resolved Hide resolved
@alexandruradovici
Copy link

P.S. I attempted to implement the command line arguments with "derive", but I did not manage to find a way for subcommands to inherit arguments. This would've meant explicitly typing out all of the arguments for all of the subcommands.

I don't understand this, can you show us an example?

@alexandruradovici alexandruradovici linked an issue Jan 24, 2023 that may be closed by this pull request
3 tasks
@george-cosma
Copy link
Author

P.S. I attempted to implement the command line arguments with "derive", but I did not manage to find a way for subcommands to inherit arguments. This would've meant explicitly typing out all of the arguments for all of the subcommands.

I don't understand this, can you show us an example?

Sure!
To create a clap Command, we have the option to either declare the command, with all of its components (subcommands, arguments, etc. ), using a builder or using structs and enums.

The difference between the two can be seen in clap's own examples:

In my own opinion, the derive method is way more straight-forward and cleaner. However, when attempting to learn this method I had come upon an issue when trying to port from the python version.

As I explained before, a lot of subcommands share a lot of arguments. To solve this, the original authors grouped the common arguments into a single "parser" object (one parser object for the apps, and one for the "channel"/board-computer communication). From there, individual subcommands are given 0,1, or both parsers as "parents", inheriting their arguments. From there, the subcommands can then define their own specific arguments.

Here is a code snippet from main.py where the "app" parser is defined: click me
And here is where the the parser is then used, in the "install" command: click me

Through my experimentation, I have not managed to find a way to replicate this behavior using the "derive" method.

@alexandruradovici
Copy link

Thanks for the explanation, what I do not clearly understand is how you use command arguments inheritance right now?

@george-cosma
Copy link
Author

george-cosma commented Jan 24, 2023

The way I did that was to set up two different functions that will build a vector of arguments, which can then be added to each individual command.

Right now, the "listen" subcommand is set up to use both the "app" and "channel" arguments.

    vec![Command::new("listen")
        .about("Open a terminal to receive UART data")
        .args(get_app_args()) // Use the APP arguments
        .args(get_channel_args()) // Use the CHANNEL arguments
        .arg_required_else_help(true)]

The two functions are then defined as such:

fn get_app_args() -> Vec<clap::Arg> {
    vec![<REMOVED FOR BERVITY>]
}

fn get_channel_args() -> Vec<clap::Arg> {
    vec![<REMOVED FOR BREVITY>]
}

If in the future if we want to implement a subcommand which uses only the app arguments, and some subcommand-specific arguments, then we can create the new subcommand like this:

Command::new("my_command")
        .about("My awesome subcommand")
        .args(get_app_args()) // Use the APP arguments
        .args(get_my_command_specific_args())  // Arguments that are only used by this command. (can be defined inline, without helper function)
]

This isn't exactly inheritance, but it does allow re-using the same arguments for multiple subcommands.

@alexandruradovici
Copy link

Ok, got it.

@alexandruradovici alexandruradovici merged commit c3cc608 into UPB-CS-OpenSourceUpstream:rust-port Jan 27, 2023
@alexandruradovici alexandruradovici added the upstream This pull request was opened in the upstream repository label Jan 27, 2023
@alexandruradovici
Copy link

Closes #3

@alexandruradovici
Copy link

@george-cosma Please select a timeslot for the interview https://doodle.com/meeting/participate/id/eZ8gOOEb.

@george-cosma george-cosma deleted the rust_port branch April 9, 2023 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upstream This pull request was opened in the upstream repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for command line options (clap)
2 participants