From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 273A8A18A for ; Thu, 7 Sep 2023 17:27:00 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id EBADC9185 for ; Thu, 7 Sep 2023 17:26:29 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Thu, 7 Sep 2023 17:26:29 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id ACCA34315A for ; Thu, 7 Sep 2023 17:26:28 +0200 (CEST) Message-ID: <3c8da745-512d-4f1b-953b-9f005953082d@proxmox.com> Date: Thu, 7 Sep 2023 17:26:26 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-GB To: Proxmox Backup Server development discussion , Gabriel Goller References: <20230830124509.168412-1-g.goller@proxmox.com> From: Thomas Lamprecht Cc: Wolfgang Bumiller , Max Carrara In-Reply-To: <20230830124509.168412-1-g.goller@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.079 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pbs-devel] [PATCH proxmox] router: cli: added `ask_for_confirmation` helper X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 07 Sep 2023 15:27:00 -0000 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 > --- > > 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.