CLI - Refactor yes/no prompts into a YesOrNo builder

crates/cli/src/subcommands/delete.rs

@@ -3,7 +3,7 @@ use std::io;
 use crate::common_args;
 use crate::config::Config;
 use crate::subcommands::db_arg_resolution::{load_config_db_targets, resolve_database_arg};
-use crate::util::{add_auth_header_opt, database_identity, get_auth_header, y_or_n, AuthHeader};
+use crate::util::{add_auth_header_opt, database_identity, get_auth_header, AuthHeader, YesOrNo};
 use clap::{Arg, ArgMatches};
 use http::StatusCode;
 use itertools::Itertools as _;
@@ -51,10 +51,13 @@ pub async fn exec(mut config: Config, args: &ArgMatches) -> Result<(), anyhow::E
         format!("{} ({identity})", resolved.database)
     };
 
-    if !y_or_n(
-        force,
-        &format!("Are you sure you want to delete database {delete_target}? This action cannot be undone."),
-    )? {
+    if !YesOrNo::new(format!(
+        "Are you sure you want to delete database {delete_target}? This action cannot be undone."
+    ))
+    .force(force)
+    .default(false)
+    .prompt()?
+    {
         println!("Aborting");
         return Ok(());
     }
@@ -69,10 +72,12 @@ pub async fn exec(mut config: Config, args: &ArgMatches) -> Result<(), anyhow::E
         StatusCode::PRECONDITION_REQUIRED => {
             let confirm = response.json::<DatabaseDeleteConfirmationResponse>().await?;
             println!("WARNING: Deleting the database {identity} will also delete its children!");
-            if !force {
-                print_database_tree_info(&confirm.database_tree).await?;
-            }
-            if force || y_or_n(false, "Do you want to proceed deleting above databases?")? {
+            print_database_tree_info(&confirm.database_tree).await?;
+            if YesOrNo::new("Do you want to proceed deleting above databases?")
+                .force(force)
+                .default(false)
+                .prompt()?
+            {
                 send_request(&client, &request_path, &auth_header, Some(confirm.confirmation_token))
                     .await?
                     .error_for_status()?;

crates/cli/src/subcommands/generate.rs

@@ -21,7 +21,7 @@ use crate::spacetime_config::{
 };
 use crate::tasks::csharp::dotnet_format;
 use crate::tasks::rust::rustfmt;
-use crate::util::y_or_n;
+use crate::util::YesOrNo;
 use crate::Config;
 use crate::{build, common_args};
 use clap::builder::PossibleValue;
@@ -591,7 +591,11 @@ pub async fn run_prepared_generate_configs(
                 println!("  {}", path.to_str().unwrap());
             }
 
-            if y_or_n(force, "Are you sure you want to delete these files?")? {
+            if YesOrNo::new("Are you sure you want to delete these files?")
+                .force(force)
+                .default(false)
+                .prompt()?
+            {
                 for path in files_to_delete {
                     fs::remove_file(path)?;
                 }

crates/cli/src/subcommands/publish.rs

@@ -24,7 +24,7 @@ use crate::spacetime_config::{
     FlatTarget, Key, LoadedConfig, SpacetimeConfig,
 };
 use crate::util::{add_auth_header_opt, get_auth_header, strip_verbatim_prefix, AuthHeader, ResponseExt};
-use crate::util::{decode_identity, y_or_n};
+use crate::util::{decode_identity, YesOrNo};
 use crate::{build, common_args};
 
 /// Individual prompts that `--yes` can suppress. `All` is a shorthand for every category below.
@@ -406,10 +406,14 @@ fn confirm_and_clear(
         "This will DESTROY the current {} module, and ALL corresponding data.",
         name_or_identity
     );
-    if !y_or_n(
-        skip_prompt,
-        format!("Are you sure you want to proceed? [deleting {}]", name_or_identity).as_str(),
-    )? {
+    if !YesOrNo::new(format!(
+        "Are you sure you want to proceed? [deleting {}]",
+        name_or_identity
+    ))
+    .force(skip_prompt)
+    .default(false)
+    .prompt()?
+    {
         anyhow::bail!("Aborted.");
     }
 
@@ -680,7 +684,11 @@ async fn execute_publish_configs<'a>(
         };
         if server_address != "localhost" && server_address != "127.0.0.1" {
             println!("You are about to publish to a non-local server: {server_address}");
-            if !y_or_n(yes.publish_to_remote, "Are you sure you want to proceed?")? {
+            if !YesOrNo::new("Are you sure you want to proceed?")
+                .force(yes.publish_to_remote)
+                .default(false)
+                .prompt()?
+            {
                 anyhow::bail!("Publish aborted by user.");
             }
         }
@@ -927,10 +935,10 @@ async fn apply_pre_publish_if_needed(
                 // We only arrive here if you have not specified ClearMode::Always AND there was no
                 // conflict that required manual migration.
                 if auto.break_clients
-                    && !y_or_n(
-                        force_break_clients || yes.break_clients,
-                        "The above changes will BREAK existing clients. Do you want to proceed?",
-                    )?
+                    && !YesOrNo::new("The above changes will BREAK existing clients. Do you want to proceed?")
+                        .force(force_break_clients || yes.break_clients)
+                        .default(false)
+                        .prompt()?
                 {
                     println!("Aborting");
                     // Early exit: return an error or a special signal. Here we bail out by returning Err.

crates/cli/src/subcommands/publish/environment.rs

@@ -91,7 +91,7 @@ pub(super) async fn publish_only(
     input: Option<&Value>,
     options: &super::EnvironmentOptions,
 ) -> anyhow::Result<()> {
-    use crate::util::{add_auth_header_opt, get_auth_header, y_or_n};
+    use crate::util::{add_auth_header_opt, get_auth_header, YesOrNo};
     use spacetimedb_client_api_messages::publish::EnvironmentMetadata;
 
     let host = config.get_host_url(server)?;
@@ -100,7 +100,10 @@ pub(super) async fn publish_only(
     if hostname != "localhost" && hostname != "127.0.0.1" {
         println!("You are about to publish environment values to a non-local server: {hostname}");
         anyhow::ensure!(
-            y_or_n(yes.publish_to_remote, "Are you sure you want to proceed?")?,
+            YesOrNo::new("Are you sure you want to proceed?")
+                .force(yes.publish_to_remote)
+                .default(false)
+                .prompt()?,
             "Publish aborted by user"
         );
     }

crates/cli/src/subcommands/server.rs

@@ -1,6 +1,8 @@
 use crate::{
     common_args,
-    util::{host_or_url_to_host_and_protocol, spacetime_server_fingerprint, y_or_n, UNSTABLE_WARNING, VALID_PROTOCOLS},
+    util::{
+        host_or_url_to_host_and_protocol, spacetime_server_fingerprint, YesOrNo, UNSTABLE_WARNING, VALID_PROTOCOLS,
+    },
     Config,
 };
 use anyhow::Context;
@@ -271,7 +273,7 @@ pub async fn exec_fingerprint(mut config: Config, args: &ArgMatches) -> Result<(
     let force = args.get_flag("force");
 
     if update_server_fingerprint(&mut config, Some(server)).await? {
-        if !y_or_n(force, "Continue?")? {
+        if !YesOrNo::new("Continue?").force(force).default(false).prompt()? {
             anyhow::bail!("Aborted");
         }
 
@@ -348,7 +350,7 @@ pub async fn exec_edit(mut config: Config, args: &ArgMatches) -> Result<(), anyh
         }
     }
 
-    if !y_or_n(force, "Continue?")? {
+    if !YesOrNo::new("Continue?").force(force).default(false).prompt()? {
         anyhow::bail!("Aborted");
     }
 
@@ -364,10 +366,11 @@ async fn exec_clear(_config: Config, paths: &SpacetimePaths, args: &ArgMatches)
     if data_dir.0.exists() {
         println!("Database path: {}", data_dir.display());
 
-        if !y_or_n(
-            force,
-            "Are you sure you want to delete all data from the local database?",
-        )? {
+        if !YesOrNo::new("Are you sure you want to delete all data from the local database?")
+            .force(force)
+            .default(false)
+            .prompt()?
+        {
             println!("Aborting");
             return Ok(());
         }

crates/cli/src/util.rs

@@ -329,20 +329,53 @@ pub fn host_or_url_to_host_and_protocol(host_or_url: &str) -> (&str, Option<&str
     }
 }
 
-/// Prompt the user for `y` or `n` from stdin.
-///
-/// Return `false` unless the input is `y`.
-pub fn y_or_n(force: bool, prompt: &str) -> anyhow::Result<bool> {
-    if force {
-        println!("Skipping confirmation due to --yes");
-        return Ok(true);
+/// Prompt the user for `n` or `n` from stdin
+pub struct YesOrNo {
+    message: String,
+    force: Option<bool>,
+    default: Option<bool>,
+}
+
+impl YesOrNo {
+    pub fn new(message: impl Into<String>) -> Self {
+        Self {
+            message: message.into(),
+            force: None,
+            default: None,
+        }
+    }
+
+    pub fn force(mut self, force: bool) -> Self {
+        self.force = Some(force);
+        self
+    }
+
+    /// Set the response when Enter is pressed without an answer.
+    pub fn default(mut self, default: bool) -> Self {
+        self.default = Some(default);
+        self
+    }
+
+    pub fn prompt(self) -> anyhow::Result<bool> {
+        assert!(!self.message.trim().is_empty(), "YesOrNo requires a message");
+        let force = self.force.expect("YesOrNo requires an explicit force setting");
+        let default = self.default.expect("YesOrNo requires an explicit default");
+        if force {
+            println!("Skipping confirmation due to --yes");
+            return Ok(true);
+        }
+        let choices = if default { "Y/n" } else { "y/N" };
+        print!("{} [{choices}]", self.message);
+        std::io::stdout().flush()?;
+        let mut input = String::new();
+        let num_read = std::io::stdin().read_line(&mut input)?;
+        anyhow::ensure!(num_read != 0, "Unexpected EOF while reading response");
+        let input = input.trim().to_lowercase();
+        if input.is_empty() {
+            return Ok(default);
+        }
+        Ok(input == "y" || input == "yes")
     }
-    let mut input = String::new();
-    print!("{prompt} [y/N]");
-    std::io::stdout().flush()?;
-    std::io::stdin().read_line(&mut input)?;
-    let input = input.trim().to_lowercase();
-    Ok(input == "y" || input == "yes")
 }
 
 pub fn decode_identity(token: &String) -> anyhow::Result<String> {
@@ -371,14 +404,16 @@ pub async fn get_login_token_or_log_in(
         return Ok(token.clone());
     }
 
-    // Note: We pass `force: false` to `y_or_n` because if we're running non-interactively we want to default to "no", not yes!
+    // Note: We leave `force` false because if we're running non-interactively we want to default to "no", not yes!
     let full_login = interactive
-        && y_or_n(
-            false,
+        && YesOrNo::new(
             // It would be "ideal" if we could print the `spacetimedb.com` by deriving it from the `default_auth_host` constant,
             // but this will change _so_ infrequently that it's not even worth the time to write that code and test it.
             "You are not logged in. Would you like to log in with spacetimedb.com?",
-        )?;
+        )
+        .force(false)
+        .default(false)
+        .prompt()?;
 
     if full_login {
         let host = Url::parse(DEFAULT_AUTH_HOST)?;

crates/smoketests/tests/cluster/cli/publish.rs

@@ -23,7 +23,7 @@ fn cli_can_publish_spacetimedb_on_disk() {
             "--server",
             &test.server_url,
             // Needed when running smoketests against a remote server.
-            "--yes=remote",
+            "--yes=remote,skip-login",
             "foobar",
         ])
         .unwrap();
@@ -37,7 +37,7 @@ fn cli_can_publish_spacetimedb_on_disk() {
             "--server",
             &test.server_url,
             // Needed when running smoketests against a remote server.
-            "--yes=remote",
+            "--yes=remote,skip-login",
             "foobar",
         ])
         .unwrap();
@@ -62,7 +62,7 @@ fn migration_test(module_name: &str, republish_args: &[&str], expect_success: bo
             &dir,
             "--server",
             &test.server_url,
-            "--yes=remote",
+            "--yes=remote,skip-login",
             &module_name,
         ])
         .unwrap();
@@ -74,7 +74,7 @@ fn migration_test(module_name: &str, republish_args: &[&str], expect_success: bo
         &dir,
         "--server",
         &test.server_url,
-        "--yes=remote",
+        "--yes=remote,skip-login",
         &module_name,
     ];
     args.extend(republish_args);
@@ -252,7 +252,7 @@ fn cli_publish_with_config_but_no_match_uses_cli_args() {
         "--server",
         &test.server_url,
         // Needed when running smoketests against a remote server.
-        "--yes=remote",
+        "--yes=remote,skip-login",
         "cli-db-name",
         "--module-path",
         module_dir.to_str().unwrap(),

crates/smoketests/tests/cluster/delete_database.rs

@@ -24,7 +24,7 @@ fn test_delete_database_aborts_without_confirmation() {
     test.publish().name(&name).run().unwrap();
 
     let output = test
-        .spacetime(&["delete", "--server", &test.server_url, &name])
+        .spacetime_with_stdin(&["delete", "--server", &test.server_url, &name], "\n")
         .unwrap();
     assert_delete_prompt(&output, &name);
     assert!(output.contains("Aborting"), "expected abort message:\n{output}");