From 6f1ae83fbe1433a785a579f285f9887ac34fed45 Mon Sep 17 00:00:00 2001 From: Filip Tibell Date: Tue, 14 Feb 2023 21:14:50 +0100 Subject: [PATCH] Make stdio.prompt async to let background tasks run --- CHANGELOG.md | 4 +- Cargo.lock | 105 +++++++++++++++ packages/lib/Cargo.toml | 1 + packages/lib/src/globals/stdio.rs | 164 +++++++---------------- packages/lib/src/lua/mod.rs | 1 + packages/lib/src/lua/stdio/mod.rs | 3 + packages/lib/src/lua/stdio/prompt.rs | 192 +++++++++++++++++++++++++++ tests/stdio/prompt.luau | 18 ++- 8 files changed, 371 insertions(+), 117 deletions(-) create mode 100644 packages/lib/src/lua/stdio/mod.rs create mode 100644 packages/lib/src/lua/stdio/prompt.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index dc1b795..7b7a944 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,10 +12,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- Improve error messages when `net.serve` fails +- Improve error handling and messages for `net.serve` +- Improve error handling and messages for `stdio.prompt` ### Fixed +- Fixed `stdio.prompt` blocking all other lua threads while prompting for input - Fixed `task.delay` keeping the script running even if it was cancelled using `task.cancel` ## `0.4.0` - February 11th, 2023 diff --git a/Cargo.lock b/Cargo.lock index 174772d..9bb8664 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -17,6 +17,33 @@ version = "1.0.69" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "224afbd727c3d6e4b90103ece64b8d1b67fbb1973b1046c2281eed3f3803f800" +[[package]] +name = "async-channel" +version = "1.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cf46fee83e5ccffc220104713af3292ff9bc7c64c7de289f66dae8e38d826833" +dependencies = [ + "concurrent-queue", + "event-listener", + "futures-core", +] + +[[package]] +name = "async-lock" +version = "2.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c8101efe8695a6c17e02911402145357e718ac92d3ff88ae8419e84b1707b685" +dependencies = [ + "event-listener", + "futures-lite", +] + +[[package]] +name = "async-task" +version = "4.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7a40729d2133846d9ed0ea60a8b9541bccddab49cd30f0715a1da672fe9a2524" + [[package]] name = "async-trait" version = "0.1.64" @@ -28,6 +55,12 @@ dependencies = [ "syn", ] +[[package]] +name = "atomic-waker" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "debc29dde2e69f9e47506b525f639ed42300fc014a3e007832592448fa8e4599" + [[package]] name = "autocfg" version = "1.1.0" @@ -67,6 +100,20 @@ dependencies = [ "generic-array", ] +[[package]] +name = "blocking" +version = "1.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3c67b173a56acffd6d2326fb7ab938ba0b00a71480e14902b2591c87bc5741e8" +dependencies = [ + "async-channel", + "async-lock", + "async-task", + "atomic-waker", + "fastrand", + "futures-lite", +] + [[package]] name = "bstr" version = "0.2.17" @@ -149,6 +196,15 @@ dependencies = [ "os_str_bytes", ] +[[package]] +name = "concurrent-queue" +version = "2.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c278839b831783b70278b14df4d45e1beb1aad306c07bb796637de9a0e323e8e" +dependencies = [ + "crossbeam-utils", +] + [[package]] name = "console" version = "0.15.5" @@ -177,6 +233,15 @@ dependencies = [ "libc", ] +[[package]] +name = "crossbeam-utils" +version = "0.8.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4fb766fa798726286dbbb842f174001dab8abc7b627a1dd86e0b7222a95d929f" +dependencies = [ + "cfg-if", +] + [[package]] name = "crypto-common" version = "0.1.6" @@ -287,6 +352,12 @@ dependencies = [ "libc", ] +[[package]] +name = "event-listener" +version = "2.5.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0206175f82b8d6bf6652ff7d71a1e27fd2e4efde587fd368662814d6ec1d9ce0" + [[package]] name = "fastrand" version = "1.8.0" @@ -354,6 +425,27 @@ version = "0.3.26" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ec90ff4d0fe1f57d600049061dc6bb68ed03c7d2fbd697274c41805dcb3f8608" +[[package]] +name = "futures-io" +version = "0.3.26" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bfb8371b6fb2aeb2d280374607aeabfc99d95c72edfe51692e42d3d7f0d08531" + +[[package]] +name = "futures-lite" +version = "1.12.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7694489acd39452c77daa48516b894c153f192c3578d5a839b62c58099fcbf48" +dependencies = [ + "fastrand", + "futures-core", + "futures-io", + "memchr", + "parking", + "pin-project-lite", + "waker-fn", +] + [[package]] name = "futures-macro" version = "0.3.26" @@ -690,6 +782,7 @@ version = "0.4.0" dependencies = [ "anyhow", "async-trait", + "blocking", "console", "dialoguer", "directories", @@ -799,6 +892,12 @@ dependencies = [ "memchr", ] +[[package]] +name = "parking" +version = "2.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "427c3892f9e783d91cc128285287e70a59e206ca452770ece88a76f7a3eddd72" + [[package]] name = "parking_lot" version = "0.12.1" @@ -1512,6 +1611,12 @@ version = "0.9.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "49874b5167b65d7193b8aba1567f5c7d93d001cafc34600cee003eda787e483f" +[[package]] +name = "waker-fn" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9d5b2c62b4012a3e1eca5a7e077d13b3bf498c4073e33ccd58626607748ceeca" + [[package]] name = "want" version = "0.3.0" diff --git a/packages/lib/Cargo.toml b/packages/lib/Cargo.toml index 61b1c8f..274a8ea 100644 --- a/packages/lib/Cargo.toml +++ b/packages/lib/Cargo.toml @@ -23,6 +23,7 @@ tokio.workspace = true reqwest.workspace = true async-trait = "0.1.64" +blocking = "1.3.0" dialoguer = "0.10.3" directories = "4.0.1" futures-util = "0.3.26" diff --git a/packages/lib/src/globals/stdio.rs b/packages/lib/src/globals/stdio.rs index bd8cffb..ca0c59d 100644 --- a/packages/lib/src/globals/stdio.rs +++ b/packages/lib/src/globals/stdio.rs @@ -1,11 +1,15 @@ +use blocking::unblock; use dialoguer::{theme::ColorfulTheme, Confirm, Input, MultiSelect, Select}; use mlua::prelude::*; -use crate::utils::{ - formatting::{ - format_style, pretty_format_multi_value, style_from_color_str, style_from_style_str, +use crate::{ + lua::stdio::{PromptKind, PromptOptions, PromptResult}, + utils::{ + formatting::{ + format_style, pretty_format_multi_value, style_from_color_str, style_from_style_str, + }, + table::TableBuilder, }, - table::TableBuilder, }; pub fn create(lua: &'static Lua) -> LuaResult { @@ -29,7 +33,9 @@ pub fn create(lua: &'static Lua) -> LuaResult { eprint!("{s}"); Ok(()) })? - .with_function("prompt", prompt)? + .with_async_function("prompt", |_, options: PromptOptions| { + unblock(move || prompt(options)) + })? .build_readonly() } @@ -37,116 +43,48 @@ fn prompt_theme() -> ColorfulTheme { ColorfulTheme::default() } -fn prompt<'a>( - lua: &'static Lua, - (kind, message, options): (Option, Option, LuaValue<'a>), -) -> LuaResult> { - match kind.map(|k| k.trim().to_ascii_lowercase()).as_deref() { - None | Some("text") => { - let theme = prompt_theme(); - let mut prompt = Input::with_theme(&theme); - if let Some(message) = message { - prompt.with_prompt(message); - }; - if let LuaValue::String(s) = options { - let txt = String::from_lua(LuaValue::String(s), lua)?; - prompt.with_initial_text(&txt); - }; - let input: String = prompt.allow_empty(true).interact_text()?; - Ok(LuaValue::String(lua.create_string(&input)?)) +fn prompt(options: PromptOptions) -> LuaResult { + let theme = prompt_theme(); + match options.kind { + PromptKind::Text => { + let input: String = Input::with_theme(&theme) + .allow_empty(true) + .with_prompt(options.text.unwrap_or("".to_string())) + .with_initial_text(options.default_string.unwrap_or("".to_string())) + .interact_text()?; + Ok(PromptResult::String(input)) } - Some("confirm") => { - if let Some(message) = message { - let theme = prompt_theme(); - let mut prompt = Confirm::with_theme(&theme); - if let LuaValue::Boolean(b) = options { - prompt.default(b); - }; - let result = prompt.with_prompt(&message).interact()?; - Ok(LuaValue::Boolean(result)) - } else { - Err(LuaError::RuntimeError( - "Argument #2 missing or nil".to_string(), - )) - } - } - Some(s) if matches!(s, "select" | "multiselect") => { - let options = match options { - LuaValue::Table(t) => { - let v: Vec = Vec::from_lua(LuaValue::Table(t), lua)?; - if v.len() < 2 { - return Err(LuaError::RuntimeError( - "Options table must contain at least 2 options".to_string(), - )); - } - v - } - LuaValue::Nil => { - return Err(LuaError::RuntimeError( - "Argument #3 missing or nil".to_string(), - )) - } - value => { - return Err(LuaError::RuntimeError(format!( - "Argument #3 must be a table, got '{}'", - value.type_name() - ))) - } + PromptKind::Confirm => { + let mut prompt = Confirm::with_theme(&theme); + if let Some(b) = options.default_bool { + prompt.default(b); }; - if let Some(message) = message { - match s { - "select" => { - let chosen = Select::with_theme(&prompt_theme()) - .with_prompt(&message) - .items(&options) - .interact_opt()?; - Ok(match chosen { - Some(idx) => LuaValue::Number((idx + 1) as f64), - None => LuaValue::Nil, - }) - } - "multiselect" => { - let chosen = MultiSelect::with_theme(&prompt_theme()) - .with_prompt(&message) - .items(&options) - .interact_opt()?; - Ok(match chosen { - Some(indices) => indices - .iter() - .map(|idx| (*idx + 1) as f64) - .collect::>() - .to_lua(lua)?, - None => LuaValue::Nil, - }) - } - _ => unreachable!(), - } - } else { - match s { - "select" => { - let chosen = Select::new().items(&options).interact_opt()?; - Ok(match chosen { - Some(idx) => LuaValue::Number((idx + 1) as f64), - None => LuaValue::Nil, - }) - } - "multiselect" => { - let chosen = MultiSelect::new().items(&options).interact_opt()?; - Ok(match chosen { - Some(indices) => indices - .iter() - .map(|idx| (*idx + 1) as f64) - .collect::>() - .to_lua(lua)?, - None => LuaValue::Nil, - }) - } - _ => unreachable!(), - } - } + let result = prompt + .with_prompt(&options.text.expect("Missing text in prompt options")) + .interact()?; + Ok(PromptResult::Boolean(result)) + } + PromptKind::Select => { + let chosen = Select::with_theme(&prompt_theme()) + .with_prompt(&options.text.unwrap_or("".to_string())) + .items(&options.options.expect("Missing options in prompt options")) + .interact_opt()?; + Ok(match chosen { + Some(idx) => PromptResult::Index(idx + 1), + None => PromptResult::None, + }) + } + PromptKind::MultiSelect => { + let chosen = MultiSelect::with_theme(&prompt_theme()) + .with_prompt(&options.text.unwrap_or("".to_string())) + .items(&options.options.expect("Missing options in prompt options")) + .interact_opt()?; + Ok(match chosen { + None => PromptResult::None, + Some(indices) => { + PromptResult::Indices(indices.iter().map(|idx| *idx + 1).collect()) + } + }) } - Some(s) => Err(LuaError::RuntimeError(format!( - "Invalid stdio prompt kind: '{s}'" - ))), } } diff --git a/packages/lib/src/lua/mod.rs b/packages/lib/src/lua/mod.rs index 3be432e..820f81e 100644 --- a/packages/lib/src/lua/mod.rs +++ b/packages/lib/src/lua/mod.rs @@ -1,2 +1,3 @@ pub mod net; +pub mod stdio; pub mod task; diff --git a/packages/lib/src/lua/stdio/mod.rs b/packages/lib/src/lua/stdio/mod.rs new file mode 100644 index 0000000..41d4c14 --- /dev/null +++ b/packages/lib/src/lua/stdio/mod.rs @@ -0,0 +1,3 @@ +mod prompt; + +pub use prompt::{PromptKind, PromptOptions, PromptResult}; diff --git a/packages/lib/src/lua/stdio/prompt.rs b/packages/lib/src/lua/stdio/prompt.rs new file mode 100644 index 0000000..ed940cc --- /dev/null +++ b/packages/lib/src/lua/stdio/prompt.rs @@ -0,0 +1,192 @@ +use std::fmt; + +use mlua::prelude::*; + +#[derive(Debug, Clone, Copy)] +pub enum PromptKind { + Text, + Confirm, + Select, + MultiSelect, +} + +impl PromptKind { + fn get_all() -> Vec { + vec![Self::Text, Self::Confirm, Self::Select, Self::MultiSelect] + } +} + +impl Default for PromptKind { + fn default() -> Self { + Self::Text + } +} + +impl fmt::Display for PromptKind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "{}", + match self { + Self::Text => "Text", + Self::Confirm => "Confirm", + Self::Select => "Select", + Self::MultiSelect => "MultiSelect", + } + ) + } +} + +impl<'lua> FromLua<'lua> for PromptKind { + fn from_lua(value: LuaValue<'lua>, _: &'lua Lua) -> LuaResult { + if let LuaValue::Nil = value { + Ok(Self::default()) + } else if let LuaValue::String(s) = value { + let s = s.to_str()?; + /* + If the user only typed the prompt kind slightly wrong, meaning + it has some kind of space in it, a weird character, or an uppercase + character, we should try to be permissive as possible and still work + + Not everyone is using an IDE with proper Luau type definitions + installed, and Luau is still a permissive scripting language + even though it has a strict (but optional) type system + */ + let s = s + .chars() + .filter_map(|c| { + if c.is_ascii_alphabetic() { + Some(c.to_ascii_lowercase()) + } else { + None + } + }) + .collect::(); + // If the prompt kind is still invalid we will + // show the user a descriptive error message + match s.as_ref() { + "text" => Ok(Self::Text), + "confirm" => Ok(Self::Confirm), + "select" => Ok(Self::Select), + "multiselect" => Ok(Self::MultiSelect), + s => Err(LuaError::FromLuaConversionError { + from: "string", + to: "PromptKind", + message: Some(format!( + "Invalid prompt kind '{s}', valid kinds are:\n{}", + PromptKind::get_all() + .iter() + .map(ToString::to_string) + .collect::>() + .join(", ") + )), + }), + } + } else { + Err(LuaError::FromLuaConversionError { + from: "nil", + to: "PromptKind", + message: None, + }) + } + } +} + +pub struct PromptOptions { + pub kind: PromptKind, + pub text: Option, + pub default_string: Option, + pub default_bool: Option, + pub options: Option>, +} + +impl<'lua> FromLuaMulti<'lua> for PromptOptions { + fn from_lua_multi(mut values: LuaMultiValue<'lua>, lua: &'lua Lua) -> LuaResult { + // Argument #1 - prompt kind (optional) + let kind = values + .pop_front() + .map(|value| PromptKind::from_lua(value, lua)) + .transpose()? + .unwrap_or_default(); + // Argument #2 - prompt text (optional) + let text = values + .pop_front() + .map(|text| String::from_lua(text, lua)) + .transpose()?; + // Argument #3 - default value / options, + // this is different per each prompt kind + let (default_bool, default_string, options) = match values.pop_front() { + None => (None, None, None), + Some(options) => match options { + LuaValue::Nil => (None, None, None), + LuaValue::Boolean(b) => (Some(b), None, None), + LuaValue::String(s) => ( + None, + Some(String::from_lua(LuaValue::String(s), lua)?), + None, + ), + LuaValue::Table(t) => ( + None, + None, + Some(Vec::::from_lua(LuaValue::Table(t), lua)?), + ), + value => { + return Err(LuaError::FromLuaConversionError { + from: value.type_name(), + to: "PromptOptions", + message: Some("Argument #3 must be a boolean, table, or nil".to_string()), + }) + } + }, + }; + /* + Make sure we got the required values for the specific prompt kind: + + - "Confirm" requires a message to be present so the user knows what they are confirming + - "Select" and "MultiSelect" both require a table of options to choose from + */ + if matches!(kind, PromptKind::Confirm) && text.is_none() { + return Err(LuaError::FromLuaConversionError { + from: "nil", + to: "PromptOptions", + message: Some("Argument #2 missing or nil".to_string()), + }); + } + if matches!(kind, PromptKind::Select | PromptKind::MultiSelect) && options.is_none() { + return Err(LuaError::FromLuaConversionError { + from: "nil", + to: "PromptOptions", + message: Some("Argument #3 missing or nil".to_string()), + }); + } + // All good, return the prompt options + Ok(Self { + kind, + text, + default_bool, + default_string, + options, + }) + } +} + +#[derive(Debug, Clone)] +pub enum PromptResult { + String(String), + Boolean(bool), + Index(usize), + Indices(Vec), + None, +} + +impl<'lua> ToLua<'lua> for PromptResult { + fn to_lua(self, lua: &'lua Lua) -> LuaResult> { + Ok(match self { + Self::String(s) => LuaValue::String(lua.create_string(&s)?), + Self::Boolean(b) => LuaValue::Boolean(b), + Self::Index(i) => LuaValue::Number(i as f64), + Self::Indices(v) => v.to_lua(lua)?, + Self::None => LuaValue::Nil, + }) + } +} diff --git a/tests/stdio/prompt.luau b/tests/stdio/prompt.luau index fb068ee..e8cc91e 100644 --- a/tests/stdio/prompt.luau +++ b/tests/stdio/prompt.luau @@ -1,16 +1,28 @@ -- NOTE: This test is intentionally not included in the -- automated tests suite since it requires user input +local passed = false +task.delay(0.2, function() + if passed then + task.spawn(error, "Prompt must not block other lua threads") + process.exit(1) + else + -- stdio.ewrite("Hello from concurrent task!") + end +end) + -- Text prompt local text = stdio.prompt("text", "Type some text") assert(#text > 0, "Did not get any text") print(`Got text '{text}'\n`) +passed = true + -- Confirmation prompt local confirmed = stdio.prompt("confirm", "Please confirm", true) -assert(confirmed == true, "Did not get true as result") +assert(type(confirmed) == "boolean", "Did not get a boolean as result") print(if confirmed then "Confirmed\n" else "Did not confirm\n") -- Selection prompt @@ -21,7 +33,7 @@ local option = stdio.prompt( { "one", "two", "three", "four" } ) assert(option == 1, "Did not get the first option as result") -print(if option then `Got option #{option}\n` else "Got no option\n") +print(`Got option #{option}\n`) -- Multi-selection prompt @@ -34,4 +46,4 @@ assert( options ~= nil and table.find(options, 2) and table.find(options, 4), "Did not get options 2 and 4 as result" ) -print(if options then `Got option(s) {stdio.format(options)}\n` else "Got no option\n") +print(`Got option(s) {stdio.format(options)}\n`)