Skip to content

Update to clap 4#1253

Closed
tjquillan wants to merge 9 commits into
dandavison:masterfrom
tjquillan:clap-4
Closed

Update to clap 4#1253
tjquillan wants to merge 9 commits into
dandavison:masterfrom
tjquillan:clap-4

Conversation

@tjquillan

@tjquillan tjquillan commented Dec 4, 2022

Copy link
Copy Markdown
Contributor

This updates delta to clap version 4.0.29. With all the migrations that come along with it. I am leaving this as a draft for now as I still need to make the correct updates to the code. I will list the changes and important parts of 4.0.0 here as I go. I will try to do each change as a single commit so they can be dropped or changed as needed.

Changes:

@dandavison

Copy link
Copy Markdown
Owner

Great, thanks for this work @tjquillan. I'll look forward to the final version.

@tjquillan

Copy link
Copy Markdown
Contributor Author

@dandavison If possible I could use your input on an issue I am encountering. It seems as of 4.0.0 Arg::get_id returns a &Id instead of a &str. Taking the simple approach of replacing mentions in the function of name with name.as_str() as shown below gives the following error:

pub fn get_argument_and_option_names<'a>() -> HashMap<&'a str, &'a str> {
        itertools::chain(Self::command().get_opts(), Self::command().get_arguments())
            .filter_map(|arg| match (arg.get_id(), arg.get_long()) {
                (name, Some(long)) => {
                    if IGNORED_OPTION_NAMES.contains(name.as_str()) {
                        None
                    } else {
                        Some((name.as_str(), long))
                    }
                }
                _ => None,
            })
            .collect()
    }
error[E0515]: cannot return value referencing temporary value
    --> src/cli.rs:1176:9
     |
1176 |           itertools::chain(Self::command().get_opts(), Self::command().get_arguments())
     |           ^                                            --------------- temporary value created here
     |  _________|
     | |
1177 | |             .filter_map(|arg| match (arg.get_id(), arg.get_long()) {
1178 | |                 (name, Some(long)) => {
1179 | |                     if IGNORED_OPTION_NAMES.contains(name.as_str()) {
...    |
1186 | |             })
1187 | |             .collect()
     | |______________________^ returns a value referencing data owned by the current function

I am very much new to rust so I don't exactly know what a solution to this would look like (it is very possible the answer is obvious to an experienced rust programmer). An alternative I see would be to change the HashMap to HashMap<&'a Id, &'a str>.

@tjquillan

Copy link
Copy Markdown
Contributor Author

@dandavison

Copy link
Copy Markdown
Owner

Thanks very much for this work @tjquillan. Your commits were included in @nickelc's work #1322 which has merged.

@dandavison dandavison closed this Mar 3, 2023
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.

2 participants