From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 0D1131FF385 for ; Wed, 24 Apr 2024 21:03:43 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 0B8CC13B70; Wed, 24 Apr 2024 21:03:47 +0200 (CEST) Message-ID: <4e3d511c-252f-4480-bcf2-f3c281d08107@proxmox.com> Date: Wed, 24 Apr 2024 21:03:42 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta To: Proxmox Backup Server development discussion , Gabriel Goller References: <20240418114957.186561-1-g.goller@proxmox.com> <20240418114957.186561-2-g.goller@proxmox.com> Content-Language: en-GB, de-AT From: Thomas Lamprecht Autocrypt: addr=t.lamprecht@proxmox.com; keydata= xsFNBFsLjcYBEACsaQP6uTtw/xHTUCKF4VD4/Wfg7gGn47+OfCKJQAD+Oyb3HSBkjclopC5J uXsB1vVOfqVYE6PO8FlD2L5nxgT3SWkc6Ka634G/yGDU3ZC3C/7NcDVKhSBI5E0ww4Qj8s9w OQRloemb5LOBkJNEUshkWRTHHOmk6QqFB/qBPW2COpAx6oyxVUvBCgm/1S0dAZ9gfkvpqFSD 90B5j3bL6i9FIv3YGUCgz6Ue3f7u+HsEAew6TMtlt90XV3vT4M2IOuECG/pXwTy7NtmHaBQ7 UJBcwSOpDEweNob50+9B4KbnVn1ydx+K6UnEcGDvUWBkREccvuExvupYYYQ5dIhRFf3fkS4+ wMlyAFh8PQUgauod+vqs45FJaSgTqIALSBsEHKEs6IoTXtnnpbhu3p6XBin4hunwoBFiyYt6 YHLAM1yLfCyX510DFzX/Ze2hLqatqzY5Wa7NIXqYYelz7tXiuCLHP84+sV6JtEkeSUCuOiUY virj6nT/nJK8m0BzdR6FgGtNxp7RVXFRz/+mwijJVLpFsyG1i0Hmv2zTn3h2nyGK/I6yhFNt dX69y5hbo6LAsRjLUvZeHXpTU4TrpN/WiCjJblbj5um5eEr4yhcwhVmG102puTtuCECsDucZ jpKpUqzXlpLbzG/dp9dXFH3MivvfuaHrg3MtjXY1i+/Oxyp5iwARAQABzTNUaG9tYXMgTGFt cHJlY2h0IChBdXRoLTQpIDx0LmxhbXByZWNodEBwcm94bW94LmNvbT7CwY4EEwEIADgWIQQO R4qbEl/pah9K6VrTZCM6gDZWBgUCWwuNxgIbAwULCQgHAgYVCAkKCwIEFgIDAQIeAQIXgAAK CRDTZCM6gDZWBm/jD/4+6JB2s67eaqoP6x9VGaXNGJPCscwzLuxDTCG90G9FYu29VcXtubH/ bPwsyBbNUQpqTm/s4XboU2qpS5ykCuTjqavrcP33tdkYfGcItj2xMipJ1i3TWvpikQVsX42R G64wovLs/dvpTYphRZkg5DwhgTmy3mRkmofFCTa+//MOcNOORltemp984tWjpR3bUJETNWpF sKGZHa3N4kCNxb7A+VMsJZ/1gN3jbQbQG7GkJtnHlWkw9rKCYqBtWrnrHa4UAvSa9M/XCIAB FThFGqZI1ojdVlv5gd6b/nWxfOPrLlSxbUo5FZ1i/ycj7/24nznW1V4ykG9iUld4uYUY86bB UGSjew1KYp9FmvKiwEoB+zxNnuEQfS7/Bj1X9nxizgweiHIyFsRqgogTvLh403QMSGNSoArk tqkorf1U+VhEncIn4H3KksJF0njZKfilrieOO7Vuot1xKr9QnYrZzJ7m7ZxJ/JfKGaRHXkE1 feMmrvZD1AtdUATZkoeQtTOpMu4r6IQRfSdwm/CkppZXfDe50DJxAMDWwfK2rr2bVkNg/yZI tKLBS0YgRTIynkvv0h8d9dIjiicw3RMeYXyqOnSWVva2r+tl+JBaenr8YTQw0zARrhC0mttu cIZGnVEvQuDwib57QLqMjQaC1gazKHvhA15H5MNxUhwm229UmdH3KM7BTQRbC43GARAAyTkR D6KRJ9Xa2fVMh+6f186q0M3ni+5tsaVhUiykxjsPgkuWXWW9MbLpYXkzX6h/RIEKlo2BGA95 QwG5+Ya2Bo3g7FGJHAkXY6loq7DgMp5/TVQ8phsSv3WxPTJLCBq6vNBamp5hda4cfXFUymsy HsJy4dtgkrPQ/bnsdFDCRUuhJHopnAzKHN8APXpKU6xV5e3GE4LwFsDhNHfH/m9+2yO/trcD txSFpyftbK2gaMERHgA8SKkzRhiwRTt9w5idOfpJVkYRsgvuSGZ0pcD4kLCOIFrer5xXudk6 NgJc36XkFRMnwqrL/bB4k6Pi2u5leyqcXSLyBgeHsZJxg6Lcr2LZ35+8RQGPOw9C0ItmRjtY ZpGKPlSxjxA1WHT2YlF9CEt3nx7c4C3thHHtqBra6BGPyW8rvtq4zRqZRLPmZ0kt/kiMPhTM 8wZAlObbATVrUMcZ/uNjRv2vU9O5aTAD9E5r1B0dlqKgxyoImUWB0JgpILADaT3VybDd3C8X s6Jt8MytUP+1cEWt9VKo4vY4Jh5vwrJUDLJvzpN+TsYCZPNVj18+jf9uGRaoK6W++DdMAr5l gQiwsNgf9372dbMI7pt2gnT5/YdG+ZHnIIlXC6OUonA1Ro/Itg90Q7iQySnKKkqqnWVc+qO9 GJbzcGykxD6EQtCSlurt3/5IXTA7t6sAEQEAAcLBdgQYAQgAIBYhBA5HipsSX+lqH0rpWtNk IzqANlYGBQJbC43GAhsMAAoJENNkIzqANlYGD1sP/ikKgHgcspEKqDED9gQrTBvipH85si0j /Jwu/tBtnYjLgKLh2cjv1JkgYYjb3DyZa1pLsIv6rGnPX9bH9IN03nqirC/Q1Y1lnbNTynPk IflgvsJjoTNZjgu1wUdQlBgL/JhUp1sIYID11jZphgzfDgp/E6ve/8xE2HMAnf4zAfJaKgD0 F+fL1DlcdYUditAiYEuN40Ns/abKs8I1MYx7Yglu3RzJfBzV4t86DAR+OvuF9v188WrFwXCS RSf4DmJ8tntyNej+DVGUnmKHupLQJO7uqCKB/1HLlMKc5G3GLoGqJliHjUHUAXNzinlpE2Vj C78pxpwxRNg2ilE3AhPoAXrY5qED5PLE9sLnmQ9AzRcMMJUXjTNEDxEYbF55SdGBHHOAcZtA kEQKub86e+GHA+Z8oXQSGeSGOkqHi7zfgW1UexddTvaRwE6AyZ6FxTApm8wq8NT2cryWPWTF BDSGB3ujWHMM8ERRYJPcBSjTvt0GcEqnd+OSGgxTkGOdufn51oz82zfpVo1t+J/FNz6MRMcg 8nEC+uKvgzH1nujxJ5pRCBOquFZaGn/p71Yr0oVitkttLKblFsqwa+10Lt6HBxm+2+VLp4Ja 0WZNncZciz3V3cuArpan/ZhhyiWYV5FD0pOXPCJIx7WS9PTtxiv0AOS4ScWEUmBxyhFeOpYa DrEx In-Reply-To: <20240418114957.186561-2-g.goller@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL -0.055 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 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [mod.rs] Subject: Re: [pbs-devel] [PATCH proxmox v5 1/2] 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: , Reply-To: Proxmox Backup Server development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" Am 18/04/2024 um 13:49 schrieb Gabriel Goller: > 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 > --- > proxmox-router/Cargo.toml | 1 + > proxmox-router/src/cli/mod.rs | 46 ++++++++++++++++++++++++++++++++++- > 2 files changed, 46 insertions(+), 1 deletion(-) > > diff --git a/proxmox-router/Cargo.toml b/proxmox-router/Cargo.toml > index dcd71a4..0b9d361 100644 > --- a/proxmox-router/Cargo.toml > +++ b/proxmox-router/Cargo.toml > @@ -19,6 +19,7 @@ percent-encoding.workspace = true > serde_json.workspace = true > serde.workspace = true > unicode-width ="0.1.8" > +regex.workspace = true > > # cli: > tokio = { workspace = true, features = [], optional = true } > diff --git a/proxmox-router/src/cli/mod.rs b/proxmox-router/src/cli/mod.rs > index 7df94ad..7f87284 100644 > --- a/proxmox-router/src/cli/mod.rs > +++ b/proxmox-router/src/cli/mod.rs > @@ -12,7 +12,10 @@ > //! - 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; > > @@ -61,6 +64,47 @@ pub fn init_cli_logger(env_var_name: &str, default_log_level: &str) { > .init(); > } > > +pub enum DefaultAnswer { The enumeration is not really tied to being a _default_ answer, you just use it that way now in this patch, I'd rather use just Answer and then also use that as return value. See for a replacement proposal at the end. > + Yes, > + No, > +} > + > +/// Prints a prompt to ask for confirmation > +pub fn ask_for_confirmation(query: String, default: DefaultAnswer) -> Result { > + let yesnoprompt: (char, char) = match default { > + DefaultAnswer::Yes => ('Y', 'n'), > + DefaultAnswer::No => ('y', 'N'), > + }; > + print!("{query} [{}/{}]: ", yesnoprompt.0, yesnoprompt.1); > + > + io::stdout().flush()?; > + let stdin = io::stdin(); > + let mut line = String::new(); > + stdin.read_line(&mut line)?; > + > + use regex::Regex; > + match default { > + DefaultAnswer::Yes => { > + // unwrap is okay, because this regex is correct > + let no_regex: Regex = Regex::new("^[nN]$").unwrap(); > + if no_regex.is_match(line.trim()) { > + Ok(false) > + } else { > + Ok(true) So, a caller passes DefaultAnswer::Yes, the user enter the full word "no", your logic then makes it a Yes like it would for all other unrecognized input... I do not like such dangerous interfaces, so NACK. The default must only be taken if the user did not make any input, but just pressed enter. > + } > + } > + DefaultAnswer::No => { > + // unwrap is okay, because this regex is coorrect > + let yes_regex: Regex = Regex::new("^[yY]$").unwrap(); > + if yes_regex.is_match(line.trim()) { > + Ok(true) > + } else { > + Ok(false) same here > + } > + } I'd rather prefer something like the following interface, it's mostly boilerplate and docs, but it should allow some safer usage. Could also do s/Confirmation/Confirm/ but no hard feelings there. The code from patch 2/2 could look the roughly like: let destroy_group_answer = Confirmation:query_with_default( format!("Delete group \"{backup_group}\" with {snapshots} snapshot(s)?"), Confirmation:No, )?; if destroy_group_answer.is_yes() { // .... } ----8<---- >From 6287a4dd3f092e2413356798a41e3a7423536f98 Mon Sep 17 00:00:00 2001 From: Thomas Lamprecht Date: Wed, 24 Apr 2024 20:55:43 +0200 Subject: [PATCH] router: cli: add yes/no confirmation enum with prompt query helper Signed-off-by: Thomas Lamprecht --- proxmox-router/src/cli/mod.rs | 115 +++++++++++++++++++++++++++++++++- 1 file changed, 114 insertions(+), 1 deletion(-) diff --git a/proxmox-router/src/cli/mod.rs b/proxmox-router/src/cli/mod.rs index 7df94ad9..7e561b33 100644 --- a/proxmox-router/src/cli/mod.rs +++ b/proxmox-router/src/cli/mod.rs @@ -12,7 +12,10 @@ //! - 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; @@ -61,6 +64,116 @@ pub fn init_cli_logger(env_var_name: &str, default_log_level: &str) { .init(); } +#[derive(PartialEq, Debug)] +/// Use for simple yes or no questions, where booleans can be confusing, especially if there's a +/// default response to consider. The implementation provides query helper for the CLI. +pub enum Confirmation { + Yes, + No, +} + +impl Confirmation { + /// get the formatted choice for the query prompt, with self being the highlighted (default) + /// one displayed as upper case. + pub fn default_choice_str(&self) -> &'static str { + match self { + Self::Yes => "Y/n", + Self::No => "y/N", + } + } + + /// Returns true if the answer is Yes + pub fn is_yes(&self) -> bool { + *self == Self::Yes + } + + /// Returns true if the answer is No + pub fn is_no(&self) -> bool { + *self == Self::No + } + + /// Parse an input string reference as yes or no confirmation. + /// + /// The input string is checked verbatim if it is exactly one of the single chars 'y', 'Y', + /// 'n', or 'N'. You must trim the string before calling, if needed, or use one of the query + /// helper functions. + /// + /// ``` + /// use proxmox_router::cli::Confirmation; + /// + /// let answer = Confirmation::from_str("y"); + /// assert!(answer.expect("valid").is_yes()); + /// + /// let answer = Confirmation::from_str("N"); + /// assert!(answer.expect("valid").is_no()); + /// + /// let answer = Confirmation::from_str("bogus"); + /// assert!(answer.is_err()); + /// ``` + pub fn from_str(input: &str) -> Result { + match input.trim() { + "y" | "Y" => Ok(Self::Yes), + "n" | "N" => Ok(Self::No), + _ => Err(io::Error::new( + io::ErrorKind::InvalidInput, + "invalid input, enter one of y, Y, n, or N", + )), + } + } + + /// Parse a input string reference as yes or no confirmation, allowing a fallback default + /// answer if the user enters an empty choice. + /// + /// The input string is checked verbatim if it is exactly one of the single chars 'y', 'Y', + /// 'n', or 'N'. The empty string maps to the default. You must trim the string before calling, + /// if needed, or use one of the query helper functions. + /// + /// ``` + /// use proxmox_router::cli::Confirmation; + /// + /// let answer = Confirmation::from_str_with_default("", Confirmation::No); + /// assert!(answer.expect("valid").is_no()); + /// + /// let answer = Confirmation::from_str_with_default("n", Confirmation::Yes); + /// assert!(answer.expect("valid").is_no()); + /// + /// let answer = Confirmation::from_str_with_default("yes", Confirmation::Yes); + /// assert!(answer.is_err()); // full-word answer not allowed for now. + /// ``` + pub fn from_str_with_default(input: &str, default: Self) -> Result { + match input.trim() { + "y" | "Y" => Ok(Self::Yes), + "n" | "N" => Ok(Self::No), + "" => Ok(default), + _ => Err(io::Error::from(io::ErrorKind::InvalidInput)), + } + } + + /// Print a query prompt with available yes no choices and returns the String the user enters. + fn read_line(query: &str, choices: &str) -> Result { + print!("{query} [{choices}]: "); + + io::stdout().flush()?; + let stdin = io::stdin(); + let mut line = String::new(); + stdin.read_line(&mut line)?; + Ok(line) + } + + /// Print a query prompt and parse the white-space trimmed answer using from_str. + pub fn query(query: &str) -> Result { + let line = Self::read_line(query, "y/n")?; + Confirmation::from_str(line.trim()) + } + + /// Print a query prompt and parse the answer using from_str_with_default, falling back to the + /// default_answer if the user provided an empty string. + pub fn query_with_default(query: &str, default_answer: Self) -> Result { + let line = Self::read_line(query, default_answer.default_choice_str())?; + Confirmation::from_str_with_default(line.trim(), default_answer) + } +} + /// Define a simple CLI command. pub struct CliCommand { /// The Schema definition. -- 2.39.2 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel