From 4bba66e93d3d710c6824a45b8879b89e5beb0ed1 Mon Sep 17 00:00:00 2001 From: itowlson Date: Fri, 14 Jun 2024 10:26:24 +1200 Subject: [PATCH 1/2] Restore cursor if dialoguer Ctrl+C-ed Signed-off-by: itowlson --- crates/templates/src/interaction.rs | 1 + src/commands/doctor.rs | 5 +++++ src/commands/external.rs | 5 +++++ src/commands/new.rs | 5 +++++ src/commands/plugins.rs | 10 ++++++++++ src/commands/templates.rs | 15 +++++++++++++++ 6 files changed, 41 insertions(+) diff --git a/crates/templates/src/interaction.rs b/crates/templates/src/interaction.rs index a8d36abaec..6091044f06 100644 --- a/crates/templates/src/interaction.rs +++ b/crates/templates/src/interaction.rs @@ -123,6 +123,7 @@ pub(crate) fn prompt_parameter(parameter: &TemplateParameter) -> Option }, Err(e) => { println!("Invalid value: {}", e); + return None; } } } diff --git a/src/commands/doctor.rs b/src/commands/doctor.rs index 51f4a1e010..c3740f95cd 100644 --- a/src/commands/doctor.rs +++ b/src/commands/doctor.rs @@ -25,6 +25,11 @@ pub struct DoctorCommand { impl DoctorCommand { pub async fn run(self) -> Result<()> { + // This may use dialoguer so we work around https://github.com/console-rs/dialoguer/issues/294 + _ = ctrlc::set_handler(|| { + _ = dialoguer::console::Term::stderr().show_cursor(); + }); + let manifest_file = spin_common::paths::resolve_manifest_file_path(&self.app_source)?; println!("{icon}The Spin Doctor is in.", icon = Emoji("📟 ", "")); diff --git a/src/commands/external.rs b/src/commands/external.rs index 6ad91cb718..155795e39e 100644 --- a/src/commands/external.rs +++ b/src/commands/external.rs @@ -56,6 +56,11 @@ pub async fn execute_external_subcommand( cmd: Vec, app: clap::App<'_>, ) -> anyhow::Result<()> { + // This may use dialoguer so we work around https://github.com/console-rs/dialoguer/issues/294 + _ = ctrlc::set_handler(|| { + _ = dialoguer::console::Term::stderr().show_cursor(); + }); + let (plugin_name, args, override_compatibility_check) = parse_subcommand(cmd)?; let plugin_store = PluginStore::try_default()?; let plugin_version = ensure_plugin_available( diff --git a/src/commands/new.rs b/src/commands/new.rs index 728ae396d9..d6d5e5580a 100644 --- a/src/commands/new.rs +++ b/src/commands/new.rs @@ -125,6 +125,11 @@ impl AddCommand { impl TemplateNewCommandCore { pub async fn run(&self, variant: TemplateVariantInfo) -> Result<()> { + // work around https://github.com/console-rs/dialoguer/issues/294 + _ = ctrlc::set_handler(|| { + _ = dialoguer::console::Term::stderr().show_cursor(); + }); + let template_manager = TemplateManager::try_default() .context("Failed to construct template directory path")?; diff --git a/src/commands/plugins.rs b/src/commands/plugins.rs index 062e4c0486..b6647fd465 100644 --- a/src/commands/plugins.rs +++ b/src/commands/plugins.rs @@ -108,6 +108,11 @@ pub struct Install { impl Install { pub async fn run(&self) -> Result<()> { + // This may use dialoguer so we work around https://github.com/console-rs/dialoguer/issues/294 + _ = ctrlc::set_handler(|| { + _ = dialoguer::console::Term::stderr().show_cursor(); + }); + let manifest_location = match (&self.local_manifest_src, &self.remote_manifest_src, &self.name) { (Some(path), None, None) => ManifestLocation::Local(path.to_path_buf()), (None, Some(url), None) => ManifestLocation::Remote(url.clone()), @@ -231,6 +236,11 @@ impl Upgrade { /// Also, by default, Spin displays the list of installed plugins that are in /// the catalogue and prompts user to choose which ones to upgrade. pub async fn run(self) -> Result<()> { + // This may use dialoguer so we work around https://github.com/console-rs/dialoguer/issues/294 + _ = ctrlc::set_handler(|| { + _ = dialoguer::console::Term::stderr().show_cursor(); + }); + let manager = PluginManager::try_default()?; let manifests_dir = manager.store().installed_manifests_directory(); diff --git a/src/commands/templates.rs b/src/commands/templates.rs index aa9367d09f..b0ded585fa 100644 --- a/src/commands/templates.rs +++ b/src/commands/templates.rs @@ -117,6 +117,11 @@ pub struct Uninstall { impl Install { pub async fn run(self) -> Result<()> { + // This may use dialoguer so we work around https://github.com/console-rs/dialoguer/issues/294 + _ = ctrlc::set_handler(|| { + _ = dialoguer::console::Term::stderr().show_cursor(); + }); + let template_manager = TemplateManager::try_default() .context("Failed to construct template directory path")?; let source = match (&self.git, &self.dir) { @@ -198,6 +203,11 @@ fn infer_github(raw: &str) -> String { impl Upgrade { pub async fn run(&self) -> Result<()> { + // This may use dialoguer so we work around https://github.com/console-rs/dialoguer/issues/294 + _ = ctrlc::set_handler(|| { + _ = dialoguer::console::Term::stderr().show_cursor(); + }); + if self.git.is_some() { // This is equivalent to `install --update` let install = Install { @@ -478,6 +488,11 @@ pub enum ListFormat { impl List { pub async fn run(self) -> Result<()> { + // This may use dialoguer so we work around https://github.com/console-rs/dialoguer/issues/294 + _ = ctrlc::set_handler(|| { + _ = dialoguer::console::Term::stderr().show_cursor(); + }); + let template_manager = TemplateManager::try_default() .context("Failed to construct template directory path")?; From 23e97a6e2e788249ddd6cb4b4592d365bf3aa7b6 Mon Sep 17 00:00:00 2001 From: itowlson Date: Fri, 14 Jun 2024 13:39:36 +1200 Subject: [PATCH 2/2] Fix "read interrupted" error from passing through Ctrl+C Signed-off-by: itowlson --- crates/common/src/ui.rs | 46 +++++++++++++++++++++++++++++ crates/templates/src/interaction.rs | 20 ++++++++----- src/commands/doctor.rs | 7 +++-- src/commands/external.rs | 5 ++-- src/commands/new.rs | 20 +++++++++---- src/commands/plugins.rs | 10 +++++-- src/commands/registry.rs | 13 ++++++-- src/commands/templates.rs | 7 +++-- 8 files changed, 106 insertions(+), 22 deletions(-) diff --git a/crates/common/src/ui.rs b/crates/common/src/ui.rs index a56022379d..79b25f91dd 100644 --- a/crates/common/src/ui.rs +++ b/crates/common/src/ui.rs @@ -8,3 +8,49 @@ use std::path::Path; pub fn quoted_path(path: impl AsRef) -> impl std::fmt::Display { format!("\"{}\"", path.as_ref().display()) } + +/// An operation that may be interrupted using Ctrl+C. This is usable only +/// when Ctrl+C is handled via the ctrlc crate - otherwise Ctrl+C terminates +/// the program. But in such situations, this trait helps you to convert +/// the interrupt into a 'cancel' value which you can use to gracefully +/// exit just as if the interrupt had been allowed to go through. +pub trait Interruptible { + /// The result type that captures the cancellation. + type Result; + + /// Converts an interrupt error to a value representing cancellation. + fn cancel_on_interrupt(self) -> Self::Result; +} + +impl Interruptible for Result, std::io::Error> { + type Result = Self; + fn cancel_on_interrupt(self) -> Self::Result { + match self { + Ok(opt) => Ok(opt), + Err(e) if e.kind() == std::io::ErrorKind::Interrupted => Ok(None), + Err(e) => Err(e), + } + } +} + +impl Interruptible for Result { + type Result = Self; + fn cancel_on_interrupt(self) -> Self::Result { + match self { + Ok(b) => Ok(b), + Err(e) if e.kind() == std::io::ErrorKind::Interrupted => Ok(false), + Err(e) => Err(e), + } + } +} + +impl Interruptible for Result { + type Result = Result, std::io::Error>; + fn cancel_on_interrupt(self) -> Self::Result { + match self { + Ok(s) => Ok(Some(s)), + Err(e) if e.kind() == std::io::ErrorKind::Interrupted => Ok(None), + Err(e) => Err(e), + } + } +} diff --git a/crates/templates/src/interaction.rs b/crates/templates/src/interaction.rs index 6091044f06..d5939573a0 100644 --- a/crates/templates/src/interaction.rs +++ b/crates/templates/src/interaction.rs @@ -9,6 +9,7 @@ use crate::{ use anyhow::anyhow; // use console::style; use dialoguer::{Confirm, Input}; +use spin_common::ui::Interruptible; pub(crate) trait InteractionStrategy { fn allow_generate_into(&self, target_dir: &Path) -> Cancellable<(), anyhow::Error>; @@ -45,7 +46,7 @@ impl InteractionStrategy for Interactive { "Directory '{}' already contains other files. Generate into it anyway?", target_dir.display() ); - match crate::interaction::confirm(&prompt) { + match crate::interaction::confirm(&prompt).cancel_on_interrupt() { Ok(true) => Cancellable::Ok(()), Ok(false) => Cancellable::Cancelled, Err(e) => Cancellable::Err(anyhow::Error::from(e)), @@ -115,28 +116,33 @@ pub(crate) fn prompt_parameter(parameter: &TemplateParameter) -> Option }; match input { - Ok(text) => match parameter.validate_value(text) { + Cancellable::Ok(text) => match parameter.validate_value(text) { Ok(text) => return Some(text), Err(e) => { println!("Invalid value: {}", e); } }, - Err(e) => { - println!("Invalid value: {}", e); + Cancellable::Cancelled => { return None; } + Cancellable::Err(e) => { + println!("Invalid value: {}", e); + } } } } -fn ask_free_text(prompt: &str, default_value: &Option) -> anyhow::Result { +fn ask_free_text( + prompt: &str, + default_value: &Option, +) -> Cancellable { let mut input = Input::::new(); input.with_prompt(prompt); if let Some(s) = default_value { input.default(s.to_owned()); } - let result = input.interact_text()?; - Ok(result) + let result = input.interact_text().cancel_on_interrupt(); + result.into() } fn is_directory_empty(path: &Path) -> bool { diff --git a/src/commands/doctor.rs b/src/commands/doctor.rs index c3740f95cd..060878afb8 100644 --- a/src/commands/doctor.rs +++ b/src/commands/doctor.rs @@ -3,6 +3,7 @@ use std::{fmt::Debug, path::PathBuf}; use anyhow::Result; use clap::Parser; use dialoguer::{console::Emoji, Confirm, Select}; +use spin_common::ui::Interruptible; use spin_doctor::{Diagnosis, DryRunNotSupported, PatientDiagnosis}; use crate::opts::{APP_MANIFEST_FILE_OPT, DEFAULT_MANIFEST_FILE}; @@ -110,7 +111,8 @@ fn prompt_treatment(summary: String, dry_run: Option) -> Result { .with_prompt(prompt) .items(&items) .default(0) - .interact_opt()?; + .interact_opt() + .cancel_on_interrupt()?; match selection { Some(2) => { @@ -122,7 +124,8 @@ fn prompt_treatment(summary: String, dry_run: Option) -> Result { Ok(Confirm::new() .with_prompt("Would you like to apply this fix?") .default(true) - .interact_opt()? + .interact_opt() + .cancel_on_interrupt()? .unwrap_or_default()) } Some(0) => Ok(true), diff --git a/src/commands/external.rs b/src/commands/external.rs index 155795e39e..e9132cb19c 100644 --- a/src/commands/external.rs +++ b/src/commands/external.rs @@ -2,7 +2,7 @@ use crate::build_info::*; use crate::commands::plugins::{update, Install}; use crate::opts::PLUGIN_OVERRIDE_COMPATIBILITY_CHECK_FLAG; use anyhow::{anyhow, Result}; -use spin_common::ui::quoted_path; +use spin_common::ui::{quoted_path, Interruptible}; use spin_plugins::{ badger::BadgerChecker, error::Error as PluginError, manifest::warn_unsupported_version, PluginStore, @@ -201,7 +201,8 @@ fn offer_install( let choice = dialoguer::Confirm::new() .with_prompt("Would you like to install this plugin and run it now?") .default(false) - .interact_opt()? + .interact_opt() + .cancel_on_interrupt()? .unwrap_or(false); Ok(choice) } diff --git a/src/commands/new.rs b/src/commands/new.rs index d6d5e5580a..6717fd5989 100644 --- a/src/commands/new.rs +++ b/src/commands/new.rs @@ -10,6 +10,7 @@ use itertools::Itertools; use path_absolutize::Absolutize; use tokio; +use spin_common::ui::Interruptible; use spin_templates::{RunOptions, Template, TemplateManager, TemplateVariantInfo}; use crate::opts::{APP_MANIFEST_FILE_OPT, DEFAULT_MANIFEST_FILE}; @@ -165,7 +166,10 @@ impl TemplateNewCommandCore { let name = match &name { Some(name) => name.to_owned(), - None => prompt_name(&variant).await?, + None => match prompt_name(&variant).await? { + Some(name) => name, + None => return Ok(()), + }, }; let output_path = if self.init { @@ -315,7 +319,8 @@ async fn prompt_template( .with_prompt(prompt) .items(&opts) .default(0) - .interact_opt()? + .interact_opt() + .cancel_on_interrupt()? { Some(i) => i, None => return Ok(None), @@ -336,18 +341,23 @@ async fn list_or_install_templates( } } -async fn prompt_name(variant: &TemplateVariantInfo) -> anyhow::Result { +async fn prompt_name(variant: &TemplateVariantInfo) -> anyhow::Result> { let noun = variant.prompt_noun(); let mut prompt = format!("Enter a name for your new {noun}"); loop { let result = dialoguer::Input::::new() .with_prompt(prompt) - .interact_text()?; + .interact_text() + .cancel_on_interrupt()?; + let Some(result) = result else { + return Ok(None); + }; + if result.trim().is_empty() { prompt = format!("Name is required. Try another {noun} name (or Crl+C to exit)"); continue; } else { - return Ok(result); + return Ok(Some(result)); } } } diff --git a/src/commands/plugins.rs b/src/commands/plugins.rs index b6647fd465..27743ba02f 100644 --- a/src/commands/plugins.rs +++ b/src/commands/plugins.rs @@ -4,6 +4,7 @@ use anyhow::{anyhow, Context, Result}; use clap::{Parser, Subcommand}; use semver::Version; +use spin_common::ui::Interruptible; use spin_plugins::{ error::Error, lookup::{fetch_plugins_repo, plugins_repo_url, PluginLookup}, @@ -327,7 +328,11 @@ impl Upgrade { eprintln!( "Select plugins to upgrade. Use Space to select/deselect and Enter to confirm selection." ); - let selected_indexes = match dialoguer::MultiSelect::new().items(&names).interact_opt()? { + let selected_indexes = match dialoguer::MultiSelect::new() + .items(&names) + .interact_opt() + .cancel_on_interrupt()? + { Some(indexes) => indexes, None => return Ok(()), }; @@ -663,7 +668,8 @@ fn prompt_confirm_install(manifest: &PluginManifest, package: &PluginPackage) -> let install = dialoguer::Confirm::new() .with_prompt(prompt) .default(true) - .interact_opt()? + .interact_opt() + .cancel_on_interrupt()? .unwrap_or(false); if !install { println!("Plugin '{}' will not be installed", manifest.name()); diff --git a/src/commands/registry.rs b/src/commands/registry.rs index 49d5839285..6770a37c07 100644 --- a/src/commands/registry.rs +++ b/src/commands/registry.rs @@ -2,7 +2,7 @@ use crate::opts::*; use anyhow::{Context, Result}; use clap::{Parser, Subcommand}; use indicatif::{ProgressBar, ProgressStyle}; -use spin_common::arg_parser::parse_kv; +use spin_common::{arg_parser::parse_kv, ui::Interruptible}; use spin_oci::Client; use std::{io::Read, path::PathBuf, time::Duration}; @@ -155,6 +155,11 @@ pub struct Login { impl Login { pub async fn run(self) -> Result<()> { + // This may use dialoguer so we work around https://github.com/console-rs/dialoguer/issues/294 + _ = ctrlc::set_handler(|| { + _ = dialoguer::console::Term::stderr().show_cursor(); + }); + let username = match self.username { Some(u) => u, None => { @@ -162,7 +167,11 @@ impl Login { loop { let result = dialoguer::Input::::new() .with_prompt(prompt) - .interact_text()?; + .interact_text() + .cancel_on_interrupt()?; + let Some(result) = result else { + return Ok(()); + }; if result.trim().is_empty() { continue; } else { diff --git a/src/commands/templates.rs b/src/commands/templates.rs index b0ded585fa..99a5daa6e1 100644 --- a/src/commands/templates.rs +++ b/src/commands/templates.rs @@ -6,6 +6,7 @@ use comfy_table::Table; use path_absolutize::Absolutize; use serde::Serialize; +use spin_common::ui::Interruptible; use spin_templates::{ InstallOptions, InstallationResults, InstalledTemplateWarning, ListResults, ProgressReporter, SkippedReason, Template, TemplateManager, TemplateSource, @@ -326,7 +327,8 @@ impl Upgrade { eprintln!("Select repos to upgrade. Use Space to select/deselect and Enter to confirm selection."); let selected_indexes = match dialoguer::MultiSelect::new() .items(&sources) - .interact_opt()? + .interact_opt() + .cancel_on_interrupt()? { Some(indexes) => indexes, None => return Ok(None), @@ -617,7 +619,8 @@ pub(crate) async fn prompt_install_default_templates( let should_install = dialoguer::Confirm::new() .with_prompt(DEFAULT_TEMPLATES_INSTALL_PROMPT) .default(true) - .interact_opt()?; + .interact_opt() + .cancel_on_interrupt()?; if should_install == Some(true) { install_default_templates().await?; Ok(Some(template_manager.list().await?.templates))