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

Use of moved value confirm #262

Closed
ShayBox opened this issue May 28, 2023 · 5 comments
Closed

Use of moved value confirm #262

ShayBox opened this issue May 28, 2023 · 5 comments

Comments

@ShayBox
Copy link

ShayBox commented May 28, 2023

Since the last cargo release, changes in the git repo have made with_prompt take ownership of confirm, preventing re-use and requiring constant re-initialization.

@pksunkara
Copy link
Collaborator

Yes, it will be a breaking change to support the builder pattern.

preventing re-use and requiring constant re-initialization.

You wouldn't need it. Can you describe more on the differences? How did it used to be and how did you need to change it?

@ShayBox
Copy link
Author

ShayBox commented May 29, 2023

I used it like this: let mut confirm = Confirm::new(); which let you initialize Confirm, Input, etc once and set the theme once instead of doing it 50 times. The builder pattern is nice but the prompt_with method shouldn't be part of the builder, but on the variable the builder returns so it can be re-used as many times as needed.

@pksunkara
Copy link
Collaborator

Just to make sure I don't misunderstand, I would prefer a more concrete example with code. Please understand that I don't have the full context here compared to you.

@ShayBox
Copy link
Author

ShayBox commented May 29, 2023

let theme = ColorfulTheme::default();

// Stable
let mut confirm = Confirm::with_theme(&theme);
if confirm.with_prompt("Question 1").interact().unwrap() {}
if confirm.with_prompt("Question 2").interact().unwrap() {}
if confirm.with_prompt("Question 3").interact().unwrap() {}

// Git
if Confirm::with_theme(&theme)
    .with_prompt("Question 1")
    .interact()
    .unwrap()
{}
if Confirm::with_theme(&theme)
    .with_prompt("Question 2")
    .interact()
    .unwrap()
{}
if Confirm::with_theme(&theme)
    .with_prompt("Question 3")
    .interact()
    .unwrap()
{}

With stable if the prompt becomes too long and rustfmt wraps the lines you can create a prompt variable for the input prompt which only takes 2 lines, with the git code it always takes 4 lines even with the prompt variable to separate the input prompt. It also requires initializing a new Confirm and borrowing the Theme every time, this ends up with the git code taking 6 lines of code (because open and close braces are on new lines) rather than 2-3 for the stable code, and this stacks the more questions you ask.

@pksunkara
Copy link
Collaborator

All the prompts are now cloneable so that you can reduce the code's duplicity.

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

No branches or pull requests

2 participants