From: Gabriel Goller <g.goller@proxmox.com>
To: Thomas Lamprecht <t.lamprecht@proxmox.com>,
Proxmox Backup Server development discussion
<pbs-devel@lists.proxmox.com>
Cc: Wolfgang Bumiller <w.bumiller@proxmox.com>,
Max Carrara <m.carrara@proxmox.com>
Subject: Re: [pbs-devel] [PATCH proxmox] router: cli: added `ask_for_confirmation` helper
Date: Fri, 8 Sep 2023 13:44:53 +0200 [thread overview]
Message-ID: <a6ff2395-09c8-7a69-edc7-a3e60e63eb69@proxmox.com> (raw)
In-Reply-To: <3c8da745-512d-4f1b-953b-9f005953082d@proxmox.com>
Submitted a new version of the series.
On 9/7/23 17:26, Thomas Lamprecht wrote:
> On 30/08/2023 14:45, Gabriel Goller wrote:
>> Added `ask_for_confirmation` helper that outputs a prompt and
>> lets the user confirm or deny it. Implemented to close #4763.
>>
>> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
>> ---
>>
>> Note: A dependency bump is needed.
>>
>> proxmox-router/src/cli/mod.rs | 19 ++++++++++++++++++-
>> 1 file changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/proxmox-router/src/cli/mod.rs b/proxmox-router/src/cli/mod.rs
>> index 208df4a..2366b47 100644
>> --- a/proxmox-router/src/cli/mod.rs
>> +++ b/proxmox-router/src/cli/mod.rs
>> @@ -12,11 +12,15 @@
>> //! - Ability to create interactive commands (using ``rustyline``)
>> //! - Supports complex/nested commands
>>
>> -use std::collections::HashMap;
>> +use std::{
>> + collections::HashMap,
>> + io::{self, Write},
>> +};
>>
>> use crate::ApiMethod;
>>
>> mod environment;
>> +use anyhow::bail;
> hmm, anyhow on library code is a slight smell, not sure about our verdict
> here, for API stuff it's widely used anyway – @Wolfgang (or @Max, you checked
> out error handling too here IIRC)
>
>> pub use environment::*;
>>
>> mod shellword;
>> @@ -62,6 +66,19 @@ pub fn init_cli_logger(env_var_name: &str, default_log_level: &str) {
>> .init();
>> }
>>
>> +pub fn ask_for_confirmation(query: String) -> Result<(), anyhow::Error> {
>> + print!("{} [y/N]: ", query);
> here too w.r.t. format strings and params please:
>
> "{query} [y/N]"
>
>> + io::stdout().flush()?;
>> + let stdin = io::stdin();
>> + let mut line = String::new();
>> + stdin.read_line(&mut line)?;
> don't we have already some pty/tty helpers that we use for passwords that
> we could re-use here? Not a hard recommendation or the like, just interest.
>
>> + if line.trim() == "y" {
> Hmm, not locale-aware though. Albeit we do not place that much of an
> emphasis on being so in our CLI tools.
>
> Would be rather just interesting if there are sane wrappers or
> implementations for something like nl_langinfo's [0] YESEXPR or NOEXPR
> fom libc in rust.
>
> [0]: https://manpages.debian.org/bookworm/manpages-dev/nl_langinfo.3.en.html
>
> Anyhow, not blocking this, just stuck out – would at least compare
> the char case-insensitive though.
>
>> + Ok(())
>> + } else {
>> + bail!("Aborted");
>> + }
>> +}
>> +
>> /// Define a simple CLI command.
>> pub struct CliCommand {
>> /// The Schema definition.
next prev parent reply other threads:[~2023-09-08 11:45 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-30 12:45 Gabriel Goller
2023-08-30 12:45 ` [pbs-devel] [PATCH proxmox-backup v2] close #4763: client: added command to forget backup group Gabriel Goller
2023-09-07 15:26 ` [pbs-devel] [PATCH proxmox] router: cli: added `ask_for_confirmation` helper Thomas Lamprecht
2023-09-08 10:39 ` Gabriel Goller
2023-09-08 11:44 ` Gabriel Goller [this message]
-- strict thread matches above, loose matches on Subject: below --
2023-08-29 11:13 [pbs-devel] [PATCH proxmox-backup] close #4763: client: added command to forget backup group Gabriel Goller
2023-08-29 11:13 ` [pbs-devel] [PATCH proxmox] router: cli: added `ask_for_confirmation` helper Gabriel Goller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a6ff2395-09c8-7a69-edc7-a3e60e63eb69@proxmox.com \
--to=g.goller@proxmox.com \
--cc=m.carrara@proxmox.com \
--cc=pbs-devel@lists.proxmox.com \
--cc=t.lamprecht@proxmox.com \
--cc=w.bumiller@proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal