search

console-rs / dialoguer

Rust utility library for nice command line prompts and similar things
MIT License
1.31k stars 143 forks source link

Add 'interact_text_opt' and 'interact_text_on_opt' for 'Input' prompts #278

Open tbergerd opened 1 year ago

tbergerd commented 1 year ago

Hello, This adds 'interact_text_opt' and 'interact_text_on_opt' for 'Input' prompts (see #160)

Note I did not do the same for 'interact' methods, I don't see how this would work with escape sequences being accepted. Additionally, unlike selection prompts, I only allowed cancellation via the 'Esc' key, considering 'q' is a valid input

P.S: Is there a specific process to contribute or is opening a pull request fine ?

Gordon01 commented 1 year ago

Oh, I see why you took this approach. Other prompt implementations are like this. Personally, I think that the builder pattern is cleaner. Single-level Result is easier to handle in the user's code, Error variants are easily matchable and directly compatible with crates like anyhow.

@pksunkara what do you think? Should "cancel" implementations be identical in all prompts?

pksunkara commented 1 year ago

@pksunkara what do you think? Should "cancel" implementations be identical in all prompts?

Yes, but IIRC there were some complications on the Input prompt, and I need to review this PR closely this weekend.

tbergerd commented 1 year ago

Thank you for the review.

Yes I basically copied what was done for other prompts. If not for consistency, I agree with your points : builder methods + an extra error variant and a single 'interact' would be better (for the impl complexity, the API surface and the calling code)

One thing to note is this wouldn't work with interact and interact_on (as is currently the case with 'completion_with' and 'history_with') though. I'm not quite sure what the use case for those methods is actually (unicode input probably ?)

If it were up to me I'd say consistency should prevail, I'll wait for both your feedbacks.