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

Suppressing static lifetimes from Text (and other modules ?) #241

Open
stormshield-hf opened this issue Mar 29, 2024 · 4 comments · May be fixed by #272
Open

Suppressing static lifetimes from Text (and other modules ?) #241

stormshield-hf opened this issue Mar 29, 2024 · 4 comments · May be fixed by #272

Comments

@stormshield-hf
Copy link

stormshield-hf commented Mar 29, 2024

Is your feature request related to a problem? Please describe.
While working with Inquire to setup a name verification, I look through an HashMap that is contained by a struct and pass that struct through a ref, that looks like this :

struct Container {
   ids: HashMap<String, usize>,
}

fn type_id(container: &Container) -> InquireResult<String> {
   Text::new("type the id:")
       .with_validator(|text:&str| {
           // Validation process...
           if container.ids.contains(text) {
               Validation::Invalid("nope, exists".into())
           } else {
              Validation::Valid
           }
      }).prompt()
}

But I can't pass a simple ref to the validator as it needs a static lifetime, and I rather avoid cloning the hashmap / container (they can get pretty big) or sharing them through an Rc<RefCell<T>>.

Describe the solution you'd like
I tried to implement a solution which is available on my fork : stormshield-guillaumed@2a07dc4 (branch in case I end up doing other changes : https://github.com/stormshield-hf/inquire/tree/work/hf/remove_static_lifetimes)

TLDR: We add a second lifetime to the Text struct which is associated to validators (and the autocompleter) and add a clone implementation that specializes on 'b: static.

The advantage of this solution is that it will keep the existing clone impl.
The main disadvantage is that it will be a breaking change as we need to add a lifetime.

Describe alternatives you've considered
Another solution we tried is specifying that validator must be 'a rather than 'static but we must specialize the Clone as 'a: static, which might be an even greater breaking change depending on the usage of the lib.

Additional context
Add any other context or screenshots about the feature request here.

@NicolasGB
Copy link

Hey!
Ran across the same issue, having a 'static lifetime when wanting to validate something living in the AppState, as in my case, it's not ideal. Ended up cloning as the data is not very big.
If there's a possibility to change that to a simple lifetime it would be great!

@Yingrjimsch
Copy link

Yingrjimsch commented Aug 8, 2024

Hey! Ran across the same issue, having a 'static lifetime when wanting to validate something living in the AppState, as in my case, it's not ideal. Ended up cloning as the data is not very big. If there's a possibility to change that to a simple lifetime it would be great!

@NicolasGB I have the same error how did you clone it`?

My fn looks like this:

add_profile(config.profiles.keys().clone().map(|key| key.as_str()).collect().clone(), config.profiles)


fn add_profile(choices: Vec<&str>, mut profiles: HashMap<String, Profile>) {
    let _name = Text::new("Name:")
        .with_validator(required!("This field is required"))
        .with_validator(
            move |name: &str| {
            if choices.iter().any(|e| name.contains(e)) {
                Ok(Validation::Invalid("not exists".into()))
            } else {
                Ok(Validation::Valid)
            }
        })
        .with_help_message("Optional notes")
        .prompt();

    let _activate = Confirm::new("Do you want to activate the profile?")
        .with_default(false)
        .prompt();
}

@NicolasGB
Copy link

Hey @Yingrjimsch, sorry i was away for the last few weeks.

I did pretty much the same you did, i had to validate if a given data already existed in my appstate, and had to clone my vec. As the data is not very big i didn't bother wrapping in Rc or whatsoever.

You can have a look at my code here: https://github.com/NicolasGB/filecrab/blob/main/filecrab-cli/src/cli/config.rs
On line 125 you'll see the validation closure.

Hope it helps :)

@pronebird
Copy link

I am sharing a certain object via Rc used for validation and then reclaiming it back after validation. Seems a bit hacky that I have to do that and unwrap. Would be great to remove the 'static requirement, also considering that Text has 'a lifetime, maybe we can pin on that?

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 a pull request may close this issue.

4 participants