mikaelmello / inquire

A Rust library for building interactive prompts
https://docs.rs/inquire
MIT License
1.98k stars 76 forks source link

&str Lifetimes with dynamically contructed inquire prompts #105

Open naxgrp-jfulton opened 1 year ago

naxgrp-jfulton commented 1 year ago

First off, very nice library. I'm switching away from a different input library to this one.

Is your feature request related to a problem? Please describe. I am wrapping inquire with a scripting language (Rhai), allowing users to specify and customize prompts through options in the DSL. However, when I attempt to construct prompts from options pulled from Rhai, I get complains about lifetimes.

#[cfg(test)]
mod tests {
    use inquire::Text;

    #[test]
    fn test() {
        let mut text = Text::new("Prompt:");

        if let Some(default) = hypothetical_default_source() {
            text = text.with_default(&default); // default does not live long enough
        }

        if let Some(placeholder) = hypothetical_placeholder_source() {
            text = text.with_placeholder(&placeholder);
        }
    }

    fn hypothetical_default_source() -> Option<String>{
        Some(String::from("default"))
    }

    fn hypothetical_placeholder_source() -> Option<String>{
        Some(String::from("placeholder"))
    }
}

Describe the solution you'd like I'm not sure what the best solution would be. Is there something I'm missing that I could do to get around this? An alternative is to use owned string internally, and take in slices in the API.

Perhaps it might help if there were builder methods that take Option<&str>, allowing one to build up several options within the same lifetime and pass them to the builder all at once:

        let default_option = hypothetical_default_source();
        let placeholder_option = hypothetical_placeholder_source();

        let results = Text::new("Prompt:")
            .with_optional_default(default_option)
            .with_options_placeholder(placeholder_option)
            .prompt();

Describe alternatives you've considered I'm considering vendoring this library into my project in order to refactor to owned strings to make the API more convenient for dynamic construction.

If there isn't some workaround I'm missing, and if it's too much of a breaking change, I'm happy do exactly this.

Thanks!

LeoRiether commented 1 year ago

The issue in your snippet is that default and placeholder are dropped/freed while text still needs to hold a reference to them, which is why you get a lifetime error ("default does not live long enough"):

fn main() {
    let mut text = Text::new("Prompt:");

    if let Some(default) = hypothetical_default_source() {
        // `default` is an owned String
        text = text.with_default(&default);
        // `default` is dropped here 
    }

    if let Some(placeholder) = hypothetical_placeholder_source() {
        // `placeholder` is an owned String
        text = text.with_placeholder(&placeholder);
        // `placeholder` is dropped here
    }

    // `text` holds a reference to `default` and `placeholder`, but they have been dropped!
    // &default and &placeholder are now dangling pointers
    text.prompt().unwrap();
}

Luckily though, if we bind hypothetical_default_source() to a variable instead of a temporary value, it will only be dropped when it goes out of scope, and that extends its lifetime, which solves the issue!

fn main() {
    let mut text = Text::new("Prompt:");

    let default_opt = hypothetical_default_source();
    if let Some(default) = &default_opt {
        // `default` is a &String
        text = text.with_default(default);
    }

    let placeholder_opt = hypothetical_placeholder_source();
    if let Some(placeholder) = &placeholder_opt {
        // `placeholder` is a &String
        text = text.with_placeholder(placeholder);
    }

    text.prompt().unwrap();
    // `default_opt` is dropped here
    // `placeholder_opt` is dropped here
    // `text` is dropped here
}

The bottom line is that you can use owned Strings to dynamically construct prompts, but you need to ensure that you hold on to them for at least as long as the inquire::Text lives (and that might be tricky in some cases).

mikaelmello commented 1 year ago

@LeoRiether should hopefully be enough to unblock you, and also sorry for the long delay!

Your issue has been making me reconsider the need for str references in this library. I'm thinking that owning these Strings would be a small price to pay for improved usability. For example, on MultiSelect prompts we already take ownership of the selectable members.

I'll have to dive a little deeper into how this would affect the API but it is something to consider.

Would love to hear if anyone already has any thoughts on this

LeoRiether commented 1 year ago

A third option is to use Cow<'a, str>! The benefits include:

Cows do have a cost, but I think the tradeoff may be worth it