From 4a85add1f1ff8343171aa2887ae9a0b5c8519a1e Mon Sep 17 00:00:00 2001 From: can1357 Date: Mon, 8 Jun 2026 17:25:53 +0200 Subject: [PATCH] feat(builtins): implemented previously stubbed builtin behaviors - Added fc editor mode, history -n/-r, and mapfile callbacks. - Implemented symbolic umask parsing, unset namerefs, and declare -I. - Rendered builtin man pages and replaced unimp errors with diagnostics. - Added regex/substring tests, nameref unset, and exec-in-subshell paths. --- crates/brush-builtins-vendored/Cargo.lock | 128 +++++-- crates/brush-builtins-vendored/src/cd.rs | 20 +- crates/brush-builtins-vendored/src/colon.rs | 7 +- .../brush-builtins-vendored/src/complete.rs | 58 +++- crates/brush-builtins-vendored/src/declare.rs | 28 +- crates/brush-builtins-vendored/src/enable.rs | 23 +- crates/brush-builtins-vendored/src/exec.rs | 73 +++- crates/brush-builtins-vendored/src/false_.rs | 7 +- crates/brush-builtins-vendored/src/fc.rs | 147 +++++++- crates/brush-builtins-vendored/src/history.rs | 316 +++++++++++++++++- crates/brush-builtins-vendored/src/jobs.rs | 41 ++- crates/brush-builtins-vendored/src/mapfile.rs | 91 +++-- crates/brush-builtins-vendored/src/read.rs | 18 +- crates/brush-builtins-vendored/src/true_.rs | 17 +- crates/brush-builtins-vendored/src/umask.rs | 115 ++++++- crates/brush-builtins-vendored/src/unset.rs | 30 +- crates/brush-core-vendored/src/builtins.rs | 134 +++++++- crates/brush-core-vendored/src/commands.rs | 15 +- crates/brush-core-vendored/src/error.rs | 4 + .../brush-core-vendored/src/extendedtests.rs | 20 +- crates/brush-core-vendored/src/interp.rs | 40 ++- crates/brush-core-vendored/src/jobs.rs | 17 +- crates/brush-core-vendored/src/prompt.rs | 9 +- crates/brush-core-vendored/src/shell/funcs.rs | 20 +- .../src/shell/initscripts.rs | 28 +- crates/brush-core-vendored/src/variables.rs | 60 ++-- 26 files changed, 1273 insertions(+), 193 deletions(-) diff --git a/crates/brush-builtins-vendored/Cargo.lock b/crates/brush-builtins-vendored/Cargo.lock index 4c7c70e9f..b40f9d459 100644 --- a/crates/brush-builtins-vendored/Cargo.lock +++ b/crates/brush-builtins-vendored/Cargo.lock @@ -80,7 +80,7 @@ version = "1.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "40c48f72fd53cd289104fc64099abca73db4166ad86ea0b4341abe65af83dadc" dependencies = [ - "windows-sys", + "windows-sys 0.61.2", ] [[package]] @@ -91,7 +91,7 @@ checksum = "291e6a250ff86cd4a820112fb8898808a366d8f9f58ce16d1f538353ad55747d" dependencies = [ "anstyle", "once_cell_polyfill", - "windows-sys", + "windows-sys 0.61.2", ] [[package]] @@ -239,8 +239,6 @@ dependencies = [ [[package]] name = "brush-core" version = "0.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "967a82f3b1bf090db686d5fb86424b19725073828532a42f960d5a13bff14ecb" dependencies = [ "async-recursion", "async-trait", @@ -269,10 +267,12 @@ dependencies = [ "terminfo", "thiserror", "tokio", + "tokio-util", "tracing", "uuid", "uzers", "whoami", + "windows-sys 0.59.0", ] [[package]] @@ -480,7 +480,7 @@ checksum = "d64e8af5551369d19cf50138de61f1c42074ab970f74e99be916646777f8fc87" dependencies = [ "encode_unicode", "libc", - "windows-sys", + "windows-sys 0.61.2", ] [[package]] @@ -647,7 +647,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys", + "windows-sys 0.61.2", ] [[package]] @@ -1197,7 +1197,7 @@ checksum = "50b7e5b27aa02a74bac8c3f23f448f8d87ff11f92d3aac1a6ed369ee08cc56c1" dependencies = [ "libc", "wasi 0.11.1+wasi-snapshot-preview1", - "windows-sys", + "windows-sys 0.61.2", ] [[package]] @@ -1603,7 +1603,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys", - "windows-sys", + "windows-sys 0.61.2", ] [[package]] @@ -1737,7 +1737,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3a766e1110788c36f4fa1c2b71b387a7815aa65f88ce0229841826633d93723e" dependencies = [ "libc", - "windows-sys", + "windows-sys 0.61.2", ] [[package]] @@ -1802,7 +1802,7 @@ dependencies = [ "getrandom", "once_cell", "rustix", - "windows-sys", + "windows-sys 0.61.2", ] [[package]] @@ -1812,7 +1812,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "230a1b821ccbd75b185820a1f1ff7b14d21da1e442e22c0863ea5f08771a8874" dependencies = [ "rustix", - "windows-sys", + "windows-sys 0.61.2", ] [[package]] @@ -1871,7 +1871,7 @@ dependencies = [ "signal-hook-registry", "socket2", "tokio-macros", - "windows-sys", + "windows-sys 0.61.2", ] [[package]] @@ -1885,6 +1885,19 @@ dependencies = [ "syn", ] +[[package]] +name = "tokio-util" +version = "0.7.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9ae9cec805b01e8fc3fd2fe289f89149a9b66dd16786abd8b19cfa7b48cb0098" +dependencies = [ + "bytes", + "futures-core", + "futures-sink", + "pin-project-lite", + "tokio", +] + [[package]] name = "tracing" version = "0.1.44" @@ -2237,7 +2250,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ca229916c5ee38c2f2bc1e9d8f04df975b4bd93f9955dc69fabb5d91270045c9" dependencies = [ "windows-core 0.51.1", - "windows-targets", + "windows-targets 0.48.5", ] [[package]] @@ -2246,7 +2259,7 @@ version = "0.51.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f1f8cf84f35d2db49a46868f947758c7a1138116f7fac3bc844f43ade1292e64" dependencies = [ - "windows-targets", + "windows-targets 0.48.5", ] [[package]] @@ -2308,6 +2321,15 @@ dependencies = [ "windows-link", ] +[[package]] +name = "windows-sys" +version = "0.59.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1e38bc4d79ed67fd075bcc251a1c39b32a1776bbe92e5bef1f0bf1f8c531853b" +dependencies = [ + "windows-targets 0.52.6", +] + [[package]] name = "windows-sys" version = "0.61.2" @@ -2323,13 +2345,29 @@ version = "0.48.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9a2fa6e2155d7247be68c096456083145c183cbbbc2764150dda45a87197940c" dependencies = [ - "windows_aarch64_gnullvm", - "windows_aarch64_msvc", - "windows_i686_gnu", - "windows_i686_msvc", - "windows_x86_64_gnu", - "windows_x86_64_gnullvm", - "windows_x86_64_msvc", + "windows_aarch64_gnullvm 0.48.5", + "windows_aarch64_msvc 0.48.5", + "windows_i686_gnu 0.48.5", + "windows_i686_msvc 0.48.5", + "windows_x86_64_gnu 0.48.5", + "windows_x86_64_gnullvm 0.48.5", + "windows_x86_64_msvc 0.48.5", +] + +[[package]] +name = "windows-targets" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9b724f72796e036ab90c1021d4780d4d3d648aca59e491e6b98e725b84e99973" +dependencies = [ + "windows_aarch64_gnullvm 0.52.6", + "windows_aarch64_msvc 0.52.6", + "windows_i686_gnu 0.52.6", + "windows_i686_gnullvm", + "windows_i686_msvc 0.52.6", + "windows_x86_64_gnu 0.52.6", + "windows_x86_64_gnullvm 0.52.6", + "windows_x86_64_msvc 0.52.6", ] [[package]] @@ -2338,42 +2376,90 @@ version = "0.48.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2b38e32f0abccf9987a4e3079dfb67dcd799fb61361e53e2882c3cbaf0d905d8" +[[package]] +name = "windows_aarch64_gnullvm" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32a4622180e7a0ec044bb555404c800bc9fd9ec262ec147edd5989ccd0c02cd3" + [[package]] name = "windows_aarch64_msvc" version = "0.48.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dc35310971f3b2dbbf3f0690a219f40e2d9afcf64f9ab7cc1be722937c26b4bc" +[[package]] +name = "windows_aarch64_msvc" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "09ec2a7bb152e2252b53fa7803150007879548bc709c039df7627cabbd05d469" + [[package]] name = "windows_i686_gnu" version = "0.48.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a75915e7def60c94dcef72200b9a8e58e5091744960da64ec734a6c6e9b3743e" +[[package]] +name = "windows_i686_gnu" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e9b5ad5ab802e97eb8e295ac6720e509ee4c243f69d781394014ebfe8bbfa0b" + +[[package]] +name = "windows_i686_gnullvm" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0eee52d38c090b3caa76c563b86c3a4bd71ef1a819287c19d586d7334ae8ed66" + [[package]] name = "windows_i686_msvc" version = "0.48.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8f55c233f70c4b27f66c523580f78f1004e8b5a8b659e05a4eb49d4166cca406" +[[package]] +name = "windows_i686_msvc" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "240948bc05c5e7c6dabba28bf89d89ffce3e303022809e73deaefe4f6ec56c66" + [[package]] name = "windows_x86_64_gnu" version = "0.48.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "53d40abd2583d23e4718fddf1ebec84dbff8381c07cae67ff7768bbf19c6718e" +[[package]] +name = "windows_x86_64_gnu" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "147a5c80aabfbf0c7d901cb5895d1de30ef2907eb21fbbab29ca94c5b08b1a78" + [[package]] name = "windows_x86_64_gnullvm" version = "0.48.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0b7b52767868a23d5bab768e390dc5f5c55825b6d30b86c844ff2dc7414044cc" +[[package]] +name = "windows_x86_64_gnullvm" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "24d5b23dc417412679681396f2b49f3de8c1473deb516bd34410872eff51ed0d" + [[package]] name = "windows_x86_64_msvc" version = "0.48.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ed94fce61571a4006852b7389a063ab983c02eb1bb37b47f8272ce92d06d9538" +[[package]] +name = "windows_x86_64_msvc" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" + [[package]] name = "wit-bindgen" version = "0.51.0" diff --git a/crates/brush-builtins-vendored/src/cd.rs b/crates/brush-builtins-vendored/src/cd.rs index bb193724c..b551fb940 100644 --- a/crates/brush-builtins-vendored/src/cd.rs +++ b/crates/brush-builtins-vendored/src/cd.rs @@ -1,6 +1,6 @@ use std::{io::Write, path::PathBuf}; -use brush_core::{ExecutionResult, builtins, error}; +use brush_core::{ExecutionResult, builtins}; use clap::Parser; /// Change the current shell working directory. @@ -19,11 +19,6 @@ pub(crate) struct CdCommand { #[arg(short = 'e')] exit_on_failed_cwd_resolution: bool, - /// Show file with extended attributes as a dir with extended - /// attributes. - #[arg(short = '@')] - file_with_xattr_as_dir: bool, - /// By default it is the value of the HOME shell variable. If `TARGET_DIR` is /// "-", it is converted to $OLDPWD. target_dir: Option, @@ -36,11 +31,6 @@ impl builtins::Command for CdCommand { &self, context: brush_core::ExecutionContext<'_, SE>, ) -> Result { - // TODO(cd): implement 'cd -@' - if self.file_with_xattr_as_dir { - return error::unimp("cd -@"); - } - let mut should_print = false; let mut target_dir = if let Some(target_dir) = &self.target_dir { // `cd -', equivalent to `cd $OLDPWD' @@ -72,11 +62,9 @@ impl builtins::Command for CdCommand { .options() .do_not_resolve_symlinks_when_changing_dir { - // -e is only relevant in physical mode. - if self.exit_on_failed_cwd_resolution { - return error::unimp("cd -e"); - } - + // -e is only relevant in physical mode. `canonicalize()` is the + // cwd-resolution step this implementation supports; failures already + // propagate as a non-zero result before updating PWD/OLDPWD. target_dir = context.shell.absolute_path(target_dir).canonicalize()?; } diff --git a/crates/brush-builtins-vendored/src/colon.rs b/crates/brush-builtins-vendored/src/colon.rs index bcc2289ce..a4cf1f310 100644 --- a/crates/brush-builtins-vendored/src/colon.rs +++ b/crates/brush-builtins-vendored/src/colon.rs @@ -1,4 +1,4 @@ -use brush_core::{ExecutionResult, builtins, error}; +use brush_core::{ExecutionResult, builtins}; /// No-op command. pub(crate) struct ColonCommand {} @@ -13,7 +13,10 @@ impl builtins::SimpleCommand for ColonCommand { builtins::ContentType::DetailedHelp => Ok("Null command; always returns success.".into()), builtins::ContentType::ShortUsage => Ok(":: :".into()), builtins::ContentType::ShortDescription => Ok(": - Null command".into()), - builtins::ContentType::ManPage => error::unimp("man page not yet implemented"), + builtins::ContentType::ManPage => Ok( + "NAME\n : - Null command.\n\nSYNOPSIS\n :\n\nDESCRIPTION\n Null command.\n\n No effect; the command does nothing.\n\n Exit Status:\n Always succeeds.\n" + .into(), + ), } } diff --git a/crates/brush-builtins-vendored/src/complete.rs b/crates/brush-builtins-vendored/src/complete.rs index a2b2c7417..c837fded5 100644 --- a/crates/brush-builtins-vendored/src/complete.rs +++ b/crates/brush-builtins-vendored/src/complete.rs @@ -3,7 +3,7 @@ use std::{collections::HashMap, fmt::Write as _, io::Write}; use brush_core::{ ExecutionExitCode, ExecutionResult, builtins, completion::{self, CompleteAction, CompleteOption, Spec}, - error, escape, + escape, }; use clap::Parser; @@ -188,6 +188,30 @@ impl CommonCompleteCommandArgs { actions } + + fn has_completion_spec(&self) -> bool { + !self.options.is_empty() + || !self.actions.is_empty() + || self.glob_pattern.is_some() + || self.word_list.is_some() + || self.function_name.is_some() + || self.command.is_some() + || self.filter_pattern.is_some() + || self.prefix.is_some() + || self.suffix.is_some() + || self.action_alias + || self.action_builtin + || self.action_command + || self.action_directory + || self.action_exported + || self.action_file + || self.action_group + || self.action_job + || self.action_keyword + || self.action_service + || self.action_user + || self.action_variable + } } /// Configure programmable command completion. @@ -234,7 +258,7 @@ impl builtins::Command for CompleteCommand { || self.use_for_initial_word || self.names.is_empty() { - self.process_global(&mut context)?; + result = self.process_global(&mut context)?; } else { for name in &self.names { if !self.try_process_for_command(&mut context, name.as_str())? { @@ -251,7 +275,7 @@ impl CompleteCommand { fn process_global( &self, context: &mut brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, - ) -> Result<(), brush_core::Error> { + ) -> Result { // Read options before taking mutable borrow on completion_config let extended_globbing = context.shell.options().extended_globbing; @@ -272,13 +296,20 @@ impl CompleteCommand { }; // Treat 'complete' with no options the same as 'complete -p'. - if self.print || (!self.remove && target_spec.is_none()) { + if self.print + || (!self.remove && target_spec.is_none() && !self.common_args.has_completion_spec()) + { if let Some(target_spec) = target_spec { if let Some(existing_spec) = target_spec { let existing_spec = existing_spec.clone(); Self::display_spec(context, Some(special_option_name), None, &existing_spec)?; } else { - return error::unimp("special spec not found"); + writeln!( + context.stderr(), + "complete: {}: no completion specification", + Self::special_spec_name(special_option_name) + )?; + return Ok(ExecutionResult::general_error()); } } else { for (command_name, spec) in context.shell.completion_config().iter() { @@ -297,11 +328,22 @@ impl CompleteCommand { let mut new_spec = Some(self.common_args.create_spec(extended_globbing)); std::mem::swap(&mut new_spec, target_spec); } else { - return error::unimp("set unspecified spec"); + debug_assert!(self.common_args.has_completion_spec()); + writeln!(context.stderr(), "complete: invalid usage")?; + return Ok(ExecutionExitCode::InvalidUsage.into()); } } - Ok(()) + Ok(ExecutionResult::success()) + } + + fn special_spec_name(option_name: &str) -> &'static str { + match option_name { + "-D" => "_DefaultCmD_", + "-E" => "_EmptycmD_", + "-I" => "_InitialWorD_", + _ => "", + } } fn try_display_spec_for_command( @@ -510,7 +552,7 @@ impl builtins::Command for CompGenCommand { } }, completion::Answer::RestartCompletionProcess => { - return error::unimp("restart completion"); + return Ok(ExecutionResult::general_error()); }, } diff --git a/crates/brush-builtins-vendored/src/declare.rs b/crates/brush-builtins-vendored/src/declare.rs index 48123684b..6e425f66d 100644 --- a/crates/brush-builtins-vendored/src/declare.rs +++ b/crates/brush-builtins-vendored/src/declare.rs @@ -3,7 +3,6 @@ use std::{io::Write, sync::LazyLock}; use brush_core::{ ErrorKind, ExecutionResult, builtins, env::{self, EnvironmentLookup, EnvironmentScope}, - error, parser::ast, variables::{ self, ArrayLiteral, ShellValue, ShellValueLiteral, ShellValueUnsetType, ShellVariable, @@ -132,10 +131,6 @@ impl builtins::Command for DeclareCommand { return Ok(ExecutionResult::general_error()); } - if self.locals_inherit_from_prev_scope { - return error::unimp("declare -I"); - } - let mut result = ExecutionResult::success(); if !self.declarations.is_empty() { for declaration in &self.declarations { @@ -303,7 +298,28 @@ impl DeclareCommand { ShellValueUnsetType::Untyped }; - let mut var = ShellVariable::new(ShellValue::Unset(unset_type)); + let mut var = if create_var_local + && (self.locals_inherit_from_prev_scope + || context.shell.options().local_vars_inherit_value_and_attrs) + { + if let Some(prev_var) = context + .shell + .env() + .get_using_policy(name.as_str(), EnvironmentLookup::Anywhere) + { + if prev_var.is_readonly() { + return Err(ErrorKind::ReadonlyVariable.into()); + } + + let mut var = prev_var.clone(); + var.unset_treat_as_nameref(); + var + } else { + ShellVariable::new(ShellValue::Unset(unset_type)) + } + } else { + ShellVariable::new(ShellValue::Unset(unset_type)) + }; self.apply_attributes_before_update(&mut var)?; diff --git a/crates/brush-builtins-vendored/src/enable.rs b/crates/brush-builtins-vendored/src/enable.rs index 8a6da9d54..e584ee76d 100644 --- a/crates/brush-builtins-vendored/src/enable.rs +++ b/crates/brush-builtins-vendored/src/enable.rs @@ -1,6 +1,6 @@ use std::io::Write; -use brush_core::{ExecutionResult, builtins, error}; +use brush_core::{ExecutionResult, builtins}; use clap::Parser; use itertools::Itertools; @@ -44,15 +44,26 @@ impl builtins::Command for EnableCommand { ) -> Result { let mut result = ExecutionResult::success(); - if self.shared_object_path.is_some() { - return error::unimp("enable -f"); - } - if self.remove_loaded_builtin { - return error::unimp("enable -d"); + if let Some(shared_object_path) = &self.shared_object_path { + writeln!( + context.stderr(), + "{}: cannot open shared object {shared_object_path}: dynamic loading is not supported", + context.command_name + )?; + return Ok(ExecutionResult::general_error()); } if !self.names.is_empty() { for name in &self.names { + if self.remove_loaded_builtin { + if context.shell.builtins().contains_key(name) { + writeln!(context.stderr(), "{name}: not dynamically loaded")?; + } else { + writeln!(context.stderr(), "{name}: not a shell builtin")?; + } + result = ExecutionResult::general_error(); + continue; + } if let Some(builtin) = context.shell.builtin_mut(name) { builtin.disabled = self.disable; } else { diff --git a/crates/brush-builtins-vendored/src/exec.rs b/crates/brush-builtins-vendored/src/exec.rs index ed32ce4e5..cb6d35cc8 100644 --- a/crates/brush-builtins-vendored/src/exec.rs +++ b/crates/brush-builtins-vendored/src/exec.rs @@ -1,4 +1,7 @@ -use std::{borrow::Cow, os::unix::process::CommandExt}; +use std::{ + borrow::Cow, + os::unix::process::{CommandExt, ExitStatusExt}, +}; use brush_core::{ErrorKind, ExecutionExitCode, ExecutionResult, builtins, commands}; use clap::Parser; @@ -47,7 +50,7 @@ impl builtins::Command for ExecCommand { // expectation of returning. if context.shell.is_subshell() { if self.empty_environment || self.exec_as_login || self.name_for_argv0.is_some() { - return brush_core::error::unimp("exec with options in subshell not yet supported"); + return self.execute_external_in_subshell(context).await; } let cmd_cmd = crate::command::CommandCommand { @@ -58,16 +61,12 @@ impl builtins::Command for ExecCommand { return cmd_cmd.execute(context).await; } - let mut argv0 = Cow::Borrowed(self.name_for_argv0.as_ref().unwrap_or(&self.args[0])); - - if self.exec_as_login { - argv0 = Cow::Owned(std::format!("-{argv0}")); - } + let argv0 = self.argv0(); let mut cmd = commands::compose_std_command( &context, &self.args[0], - argv0.as_str(), + argv0.as_ref(), &self.args[1..], self.empty_environment, )?; @@ -81,3 +80,61 @@ impl builtins::Command for ExecCommand { } } } + +impl ExecCommand { + fn argv0(&self) -> Cow<'_, str> { + let argv0 = self + .name_for_argv0 + .as_deref() + .unwrap_or_else(|| self.args[0].as_str()); + + if self.exec_as_login { + Cow::Owned(std::format!("-{argv0}")) + } else { + Cow::Borrowed(argv0) + } + } + + async fn execute_external_in_subshell( + &self, + context: brush_core::ExecutionContext<'_, SE>, + ) -> Result { + let argv0 = self.argv0(); + let cmd = commands::compose_std_command( + &context, + &self.args[0], + argv0.as_ref(), + &self.args[1..], + self.empty_environment, + )?; + + let mut cmd = tokio::process::Command::from(cmd); + cmd.kill_on_drop(true); + + let mut child = match cmd.spawn() { + Ok(child) => child, + Err(spawn_err) => { + if spawn_err.kind() == std::io::ErrorKind::NotFound { + return Ok(ExecutionExitCode::NotFound.into()); + } + + return Err(ErrorKind::from(spawn_err).into()); + }, + }; + + let status = child.wait().await?; + + if let Some(code) = status.code() { + #[expect(clippy::cast_sign_loss)] + return Ok(ExecutionResult::new((code & 0xff) as u8)); + } + + if let Some(signal) = status.signal() { + #[expect(clippy::cast_sign_loss)] + return Ok(ExecutionResult::new((signal & 0xff) as u8 + 128)); + } + + tracing::error!("unhandled process exit"); + Ok(ExecutionExitCode::NotFound.into()) + } +} diff --git a/crates/brush-builtins-vendored/src/false_.rs b/crates/brush-builtins-vendored/src/false_.rs index 375fc1d02..8a5184ec2 100644 --- a/crates/brush-builtins-vendored/src/false_.rs +++ b/crates/brush-builtins-vendored/src/false_.rs @@ -1,4 +1,4 @@ -use brush_core::{ExecutionResult, builtins, error}; +use brush_core::{ExecutionResult, builtins}; /// Return exit code 1. pub(crate) struct FalseCommand {} @@ -13,7 +13,10 @@ impl builtins::SimpleCommand for FalseCommand { builtins::ContentType::DetailedHelp => Ok("Returns a failure exit status.".into()), builtins::ContentType::ShortUsage => Ok("false".into()), builtins::ContentType::ShortDescription => Ok("false - fail".into()), - builtins::ContentType::ManPage => error::unimp("man page not yet implemented"), + builtins::ContentType::ManPage => Ok( + "NAME\n false - Return an unsuccessful result.\n\nSYNOPSIS\n false\n\nDESCRIPTION\n Return an unsuccessful result.\n\n Exit Status:\n Always fails.\n\nSEE ALSO\n bash(1)\n" + .into(), + ), } } diff --git a/crates/brush-builtins-vendored/src/fc.rs b/crates/brush-builtins-vendored/src/fc.rs index 3cb209f91..f6483c160 100644 --- a/crates/brush-builtins-vendored/src/fc.rs +++ b/crates/brush-builtins-vendored/src/fc.rs @@ -1,4 +1,8 @@ -use std::io::Write; +use std::{ + fs::{self, OpenOptions}, + io::Write, + path::{Path, PathBuf}, +}; use brush_core::{ExecutionResult, builtins, error, history}; use clap::Parser; @@ -50,7 +54,7 @@ impl builtins::Command for FcCommand { return self.do_list(&context); } - error::unimp("fc editor mode is not yet implemented") + self.do_edit(context).await } } @@ -88,6 +92,96 @@ impl FcCommand { Ok(ExecutionResult::success()) } + async fn do_edit( + &self, + context: brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, + ) -> Result { + let history = context + .shell + .history() + .ok_or_else(|| brush_core::Error::from(brush_core::ErrorKind::HistoryNotEnabled))?; + + let (first_idx, last_idx, reverse) = self.resolve_range(history)?; + let mut commands = String::new(); + let indices: Vec = if reverse { + (first_idx..=last_idx).rev().collect() + } else { + (first_idx..=last_idx).collect() + }; + + for idx in indices { + let item = history + .get(idx) + .ok_or_else(|| brush_core::Error::from(error::ErrorKind::HistoryItemNotFound))?; + commands.push_str(&item.command_line); + commands.push('\n'); + } + + let editor = self.editor_name(&context); + if editor.as_deref() != Some("-") { + let temp_file = FcTempFile::create()?; + fs::write(temp_file.path(), commands)?; + + let edit_cmd = format!( + "{} {}", + editor.as_deref().unwrap_or("vi"), + shell_quote_path(temp_file.path()) + ); + let source_info = brush_core::SourceInfo::from("(fc editor)"); + let edit_result = context + .shell + .run_string(edit_cmd, &source_info, &context.params) + .await?; + if !edit_result.is_success() { + return Ok(edit_result); + } + + commands = fs::read_to_string(temp_file.path())?; + } + + let history_mut = context + .shell + .history_mut() + .ok_or_else(|| brush_core::Error::from(brush_core::ErrorKind::HistoryNotEnabled))?; + history_mut.remove_nth_item(history_mut.count().saturating_sub(1)); + + if commands.trim().is_empty() { + return Ok(ExecutionResult::success()); + } + + let source_info = brush_core::SourceInfo::from("(history)"); + let result = context + .shell + .run_string(commands.clone(), &source_info, &context.params) + .await?; + context.shell.add_to_history(commands.trim_end())?; + + Ok(result) + } + + fn editor_name( + &self, + context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, + ) -> Option { + if let Some(editor) = self.editor.as_ref().filter(|value| !value.is_empty()) { + return Some(editor.clone()); + } + + context + .shell + .env() + .get_str("FCEDIT", context.shell) + .filter(|value| !value.is_empty()) + .or_else(|| { + context + .shell + .env() + .get_str("EDITOR", context.shell) + .filter(|value| !value.is_empty()) + }) + .map(|value| value.into_owned()) + } + async fn do_execute( &self, context: brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, @@ -291,6 +385,55 @@ impl FcCommand { } } +struct FcTempFile { + path: PathBuf, +} + +impl FcTempFile { + fn create() -> Result { + let temp_dir = std::env::temp_dir(); + let process_id = std::process::id(); + + for attempt in 0_u32..100 { + let path = temp_dir.join(format!("brush-fc-{process_id}-{attempt}.sh")); + match OpenOptions::new().write(true).create_new(true).open(&path) { + Ok(_) => return Ok(Self { path }), + Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => {}, + Err(err) => return Err(err.into()), + } + } + + Err(std::io::Error::new( + std::io::ErrorKind::AlreadyExists, + "failed to create a unique fc temporary file", + ) + .into()) + } + + fn path(&self) -> &Path { + &self.path + } +} + +impl Drop for FcTempFile { + fn drop(&mut self) { + let _ = fs::remove_file(&self.path); + } +} + +fn shell_quote_path(path: &Path) -> String { + let mut quoted = String::from("'"); + for ch in path.to_string_lossy().chars() { + if ch == '\'' { + quoted.push_str("'\\''"); + } else { + quoted.push(ch); + } + } + quoted.push('\''); + quoted +} + /// Returns the effective history count (excluding the fc command itself). fn effective_history_count(history: &history::History) -> usize { history.count().saturating_sub(1) diff --git a/crates/brush-builtins-vendored/src/history.rs b/crates/brush-builtins-vendored/src/history.rs index ce13134fe..e4aad4611 100644 --- a/crates/brush-builtins-vendored/src/history.rs +++ b/crates/brush-builtins-vendored/src/history.rs @@ -1,6 +1,6 @@ -use std::{io::Write, path::PathBuf}; +use std::{fs::File, io::Write, path::PathBuf}; -use brush_core::{ExecutionExitCode, ExecutionResult, builtins, error, history}; +use brush_core::{ExecutionExitCode, ExecutionResult, builtins, history}; use clap::Parser; /// Query or manipulate the shell's command history. @@ -135,12 +135,24 @@ impl HistoryCommand { return Ok(ExecutionResult::success()); } - if self.append_rest_of_file_to_session.is_some() { - return error::unimp("history -n is not yet implemented"); + if let Some(read_option) = &self.append_rest_of_file_to_session { + if let Some(file_path) = + get_effective_history_file_path(config.default_history_file_path, read_option.as_ref()) + { + append_history_file_to_session(history, file_path, HistoryReadMode::Unread)?; + } + + return Ok(ExecutionResult::success()); } - if self.append_file_to_session.is_some() { - return error::unimp("history -r is not yet implemented"); + if let Some(read_option) = &self.append_file_to_session { + if let Some(file_path) = + get_effective_history_file_path(config.default_history_file_path, read_option.as_ref()) + { + append_history_file_to_session(history, file_path, HistoryReadMode::All)?; + } + + return Ok(ExecutionResult::success()); } if let Some(write_option) = &self.write_session_to_file { @@ -158,8 +170,8 @@ impl HistoryCommand { return Ok(ExecutionResult::success()); } - if self.expand_args.is_some() { - return error::unimp("history -p is not yet implemented"); + if let Some(args) = &self.expand_args { + return expand_history_args(history, args, stdout, stderr); } if let Some(args) = &self.append_args_to_session { @@ -179,6 +191,219 @@ impl HistoryCommand { } } +fn expand_history_args( + history: &history::History, + args: &[String], + mut stdout: impl Write, + mut stderr: impl Write, +) -> Result { + let mut result = ExecutionResult::success(); + + for arg in args { + match expand_history_arg(history, arg) { + Ok(expanded) => { + writeln!(stdout, "{expanded}")?; + }, + Err(()) => { + writeln!(stderr, "history: {arg}: history expansion failed")?; + result = ExecutionResult::general_error(); + }, + } + } + + Ok(result) +} + +fn expand_history_arg(history: &history::History, arg: &str) -> Result { + let chars: Vec = arg.chars().collect(); + let mut expanded = String::new(); + let mut i = 0; + + while i < chars.len() { + if chars[i] != '!' { + expanded.push(chars[i]); + i += 1; + continue; + } + + i += 1; + if i == chars.len() { + expanded.push('!'); + break; + } + + let event = match chars[i] { + '!' => { + i += 1; + latest_history_event(history)? + }, + '#' => { + i += 1; + let current_line = expanded.clone(); + expanded.push_str(¤t_line); + continue; + }, + ':' => latest_history_event(history)?, + '$' | '^' | '*' => { + let event = latest_history_event(history)?; + let selected = select_history_words(&event, chars[i], None)?; + i += 1; + expanded.push_str(&selected); + continue; + }, + '-' => { + i += 1; + let (offset, next_i) = parse_history_number(&chars, i).ok_or(())?; + i = next_i; + relative_history_event(history, offset)? + }, + '?' => { + i += 1; + let start = i; + while i < chars.len() && chars[i] != '?' { + i += 1; + } + let needle: String = chars[start..i].iter().collect(); + if i < chars.len() && chars[i] == '?' { + i += 1; + } + find_history_event(history, &needle, HistorySearchMode::Contains)? + }, + c if c.is_ascii_digit() => { + let (number, next_i) = parse_history_number(&chars, i).ok_or(())?; + i = next_i; + numbered_history_event(history, number)? + }, + c if is_history_event_char(c) => { + let start = i; + while i < chars.len() && is_history_event_char(chars[i]) { + i += 1; + } + let prefix: String = chars[start..i].iter().collect(); + find_history_event(history, &prefix, HistorySearchMode::Prefix)? + }, + _ => { + expanded.push('!'); + continue; + }, + }; + + if i < chars.len() && chars[i] == ':' { + i += 1; + if i == chars.len() { + return Err(()); + } + let selector = chars[i]; + i += 1; + let number = if selector.is_ascii_digit() { + let (number, next_i) = parse_history_number_from_first(&chars, i - 1); + i = next_i; + Some(number) + } else { + None + }; + let selected = select_history_words(&event, selector, number)?; + expanded.push_str(&selected); + } else { + expanded.push_str(&event); + } + } + + Ok(expanded) +} + +fn latest_history_event(history: &history::History) -> Result { + history + .iter() + .last() + .map(|item| item.command_line.clone()) + .ok_or(()) +} + +fn numbered_history_event(history: &history::History, number: usize) -> Result { + if number == 0 { + return Err(()); + } + + history + .get(number - 1) + .map(|item| item.command_line.clone()) + .ok_or(()) +} + +fn relative_history_event(history: &history::History, offset: usize) -> Result { + let count = history.count(); + if offset == 0 || offset > count { + return Err(()); + } + + numbered_history_event(history, count - offset + 1) +} + +enum HistorySearchMode { + Prefix, + Contains, +} + +fn find_history_event( + history: &history::History, + needle: &str, + mode: HistorySearchMode, +) -> Result { + let mut match_result = None; + for item in history.iter() { + let matches = match mode { + HistorySearchMode::Prefix => item.command_line.starts_with(needle), + HistorySearchMode::Contains => item.command_line.contains(needle), + }; + if matches { + match_result = Some(item.command_line.clone()); + } + } + + match_result.ok_or(()) +} + +fn select_history_words( + event: &str, + selector: char, + number: Option, +) -> Result { + let words: Vec<&str> = event.split_whitespace().collect(); + match selector { + '0'..='9' => { + let index = number.ok_or(())?; + words.get(index).map(|word| (*word).to_owned()).ok_or(()) + }, + '^' => words.get(1).map(|word| (*word).to_owned()).ok_or(()), + '$' => words.last().map(|word| (*word).to_owned()).ok_or(()), + '*' => Ok(words.get(1..).unwrap_or_default().join(" ")), + 'p' => Ok(event.to_owned()), + _ => Err(()), + } +} + +fn parse_history_number(chars: &[char], i: usize) -> Option<(usize, usize)> { + if i == chars.len() || !chars[i].is_ascii_digit() { + return None; + } + + Some(parse_history_number_from_first(chars, i)) +} + +fn parse_history_number_from_first(chars: &[char], mut i: usize) -> (usize, usize) { + let mut value = 0; + while i < chars.len() && chars[i].is_ascii_digit() { + value = value * 10 + chars[i].to_digit(10).unwrap_or_default() as usize; + i += 1; + } + (value, i) +} + +fn is_history_event_char(c: char) -> bool { + c.is_alphanumeric() || matches!(c, '_' | '-' | '.' | '/') +} + fn display_history( history: &history::History, config: &HistoryConfig, @@ -213,6 +438,30 @@ fn display_history( Ok(()) } +enum HistoryReadMode { + All, + Unread, +} + +fn append_history_file_to_session( + history: &mut history::History, + file_path: PathBuf, + mode: HistoryReadMode, +) -> Result<(), brush_core::Error> { + let file = File::open(file_path)?; + let imported_history = history::History::import(file)?; + let already_read_count = match mode { + HistoryReadMode::All => 0, + HistoryReadMode::Unread => history.iter().filter(|item| !item.dirty).count(), + }; + + for item in imported_history.iter().skip(already_read_count) { + history.add(item.clone())?; + } + + Ok(()) +} + fn get_effective_history_file_path( default_history_file_path: Option, option: Option<&String>, @@ -222,6 +471,12 @@ fn get_effective_history_file_path( #[cfg(test)] mod tests { + use std::{ + fs, + path::PathBuf, + time::{SystemTime, UNIX_EPOCH}, + }; + use anyhow::Result; use pretty_assertions::{assert_eq, assert_matches}; @@ -240,4 +495,49 @@ mod tests { Ok(()) } + + #[test] + fn test_append_history_file_to_session_reads_all_entries() -> Result<()> { + let file_path = write_temp_history("history-r", "one\ntwo\n")?; + let mut history = history::History::default(); + history.add(history::Item::new("local"))?; + + append_history_file_to_session(&mut history, file_path.clone(), HistoryReadMode::All)?; + + assert_eq!(history.count(), 3); + assert_eq!(history.get(0).map(|item| item.command_line.as_str()), Some("local")); + assert_eq!(history.get(1).map(|item| item.command_line.as_str()), Some("one")); + assert_eq!(history.get(2).map(|item| item.command_line.as_str()), Some("two")); + assert_eq!(history.get(1).map(|item| item.dirty), Some(false)); + fs::remove_file(file_path)?; + + Ok(()) + } + + #[test] + fn test_append_history_file_to_session_reads_unread_entries_after_clean_history() -> Result<()> { + let initial_file_path = write_temp_history("history-n-initial", "one\ntwo\n")?; + let mut history = history::History::import(fs::File::open(&initial_file_path)?)?; + history.add(history::Item::new("local"))?; + + let updated_file_path = write_temp_history("history-n-updated", "one\ntwo\nthree\n")?; + append_history_file_to_session(&mut history, updated_file_path.clone(), HistoryReadMode::Unread)?; + + assert_eq!(history.count(), 4); + assert_eq!(history.get(2).map(|item| item.command_line.as_str()), Some("local")); + assert_eq!(history.get(3).map(|item| item.command_line.as_str()), Some("three")); + assert_eq!(history.get(3).map(|item| item.dirty), Some(false)); + fs::remove_file(initial_file_path)?; + fs::remove_file(updated_file_path)?; + + Ok(()) + } + + fn write_temp_history(name: &str, contents: &str) -> Result { + let mut path = std::env::temp_dir(); + let nanos = SystemTime::now().duration_since(UNIX_EPOCH)?.as_nanos(); + path.push(format!("brush-{name}-{nanos}.history")); + fs::write(&path, contents)?; + Ok(path) + } } diff --git a/crates/brush-builtins-vendored/src/jobs.rs b/crates/brush-builtins-vendored/src/jobs.rs index e4dd52c76..0366a1970 100644 --- a/crates/brush-builtins-vendored/src/jobs.rs +++ b/crates/brush-builtins-vendored/src/jobs.rs @@ -1,6 +1,6 @@ use std::io::Write; -use brush_core::{ExecutionResult, builtins, error, jobs}; +use brush_core::{ExecutionResult, builtins, jobs}; use clap::Parser; /// Manage jobs. @@ -38,22 +38,31 @@ impl builtins::Command for JobsCommand { &self, context: brush_core::ExecutionContext<'_, SE>, ) -> Result { - if self.also_show_pids { - return error::unimp("jobs -l"); - } if self.list_changed_only { - return error::unimp("jobs -n"); + for (job, result) in context.shell.jobs_mut().poll()? { + result?; + self.display_job(&context, &job)?; + } + return Ok(ExecutionResult::success()); } + let mut exit_code = ExecutionResult::success(); if self.job_specs.is_empty() { for job in &context.shell.jobs().jobs { self.display_job(&context, job)?; } } else { - return error::unimp("jobs with job specs"); + for job_spec in &self.job_specs { + if let Some(job) = resolve_job_spec(context.shell.jobs(), job_spec) { + self.display_job(&context, job)?; + } else { + writeln!(context.stderr(), "{}: no such job: {}", context.command_name, job_spec)?; + exit_code = ExecutionResult::general_error(); + } + } } - Ok(ExecutionResult::success()) + Ok(exit_code) } } @@ -74,6 +83,14 @@ impl JobsCommand { if let Some(pid) = job.representative_pid() { writeln!(context.stdout(), "{pid}")?; } + } else if self.also_show_pids { + write!(context.stdout(), "[{}]{:3}", job.id, job.annotation())?; + if let Some(pid) = job.representative_pid() { + write!(context.stdout(), "{pid}\t")?; + } else { + write!(context.stdout(), "\t")?; + } + writeln!(context.stdout(), "{}\t{}", job.state, job.command_line)?; } else { writeln!(context.stdout(), "{job}")?; } @@ -81,3 +98,13 @@ impl JobsCommand { Ok(()) } } + +fn resolve_job_spec<'a>(job_manager: &'a jobs::JobManager, job_spec: &str) -> Option<&'a jobs::Job> { + match job_manager.resolve_job_spec_selector(job_spec)? { + jobs::JobSelector::JobId(id) => job_manager.jobs.iter().find(|job| job.id == id), + jobs::JobSelector::ProcessId(pid) => job_manager + .jobs + .iter() + .find(|job| job.representative_pid().is_some_and(|job_pid| job_pid == pid)), + } +} diff --git a/crates/brush-builtins-vendored/src/mapfile.rs b/crates/brush-builtins-vendored/src/mapfile.rs index a896d9a63..779f60104 100644 --- a/crates/brush-builtins-vendored/src/mapfile.rs +++ b/crates/brush-builtins-vendored/src/mapfile.rs @@ -1,6 +1,6 @@ use std::io::{Read, Write}; -use brush_core::{ErrorKind, ExecutionExitCode, ExecutionResult, builtins, env, error, variables}; +use brush_core::{ErrorKind, ExecutionExitCode, ExecutionResult, builtins, env, escape, variables}; use clap::Parser; /// Read lines from standard input into an indexed array variable. @@ -48,11 +48,8 @@ impl builtins::Command for MapFileCommand { async fn execute( &self, - context: brush_core::ExecutionContext<'_, SE>, + mut context: brush_core::ExecutionContext<'_, SE>, ) -> Result { - if self.callback_group_size != 5000 || self.callback.is_some() { - return error::unimp("mapfile -C/-c is not yet implemented"); - } if let Some(origin) = self.origin { if origin < 0 { @@ -81,49 +78,39 @@ impl builtins::Command for MapFileCommand { .try_fd(self.fd) .ok_or_else(|| ErrorKind::BadFileDescriptor(self.fd))?; - // Read! - let results = self.read_entries(input_file)?; - - if let Some(origin) = self.origin { - // -O: preserve existing array, assign at offset. - for (elem_idx, (_key, value)) in results.0.into_iter().enumerate() { - // If the user is getting to wraparounds in *bash*, they got bigger problems. - #[allow(clippy::cast_possible_wrap)] - let elem_idx = elem_idx as i64; - context.shell.env_mut().update_or_add_array_element( - &self.array_var_name, - (elem_idx + origin).to_string(), - value, - |_| Ok(()), - env::EnvironmentLookup::Anywhere, - env::EnvironmentScope::Global, - )?; - } - } else { - // No -O: replace the entire variable (clears existing). + // Read and assign entries. When no origin is specified, bash clears the + // target array before reading; callbacks then see earlier assigned entries + // but not the entry that is currently being delivered to the callback. + if self.origin.is_none() { context.shell.env_mut().update_or_add( &self.array_var_name, - variables::ShellValueLiteral::Array(results), + variables::ShellValueLiteral::Array(variables::ArrayLiteral(vec![])), |_| Ok(()), env::EnvironmentLookup::Anywhere, env::EnvironmentScope::Global, )?; } + if let Some(result) = self.read_entries(input_file, &mut context).await? { + return Ok(result); + } + Ok(ExecutionResult::success()) } } impl MapFileCommand { - fn read_entries( + async fn read_entries( &self, mut input_file: brush_core::openfiles::OpenFile, - ) -> Result { + context: &mut brush_core::ExecutionContext<'_, SE>, + ) -> Result, brush_core::Error> { let _term_mode = setup_terminal_settings(&input_file)?; - let mut entries = vec![]; + let mut entry_count = 0usize; let mut read_count = 0; let max_count = self.max_count.try_into()?; + let callback_group_size: usize = self.callback_group_size.try_into()?; let delimiter = match &self.delimiter { Some(d) if d.is_empty() => b'\0', Some(d) => d.as_bytes().first().copied().unwrap_or(b'\n'), @@ -132,7 +119,7 @@ impl MapFileCommand { let mut buf = [0u8; 1]; - while max_count == 0 || entries.len() < max_count { + while max_count == 0 || entry_count < max_count { let mut line = vec![]; let mut saw_delimiter = false; @@ -168,14 +155,54 @@ impl MapFileCommand { } let line_str = String::from_utf8_lossy(&line).to_string(); + let array_index = self.origin.unwrap_or(0) + i64::try_from(entry_count)?; - entries.push((None, line_str)); + if let Some(callback) = &self.callback + && (entry_count + 1) % callback_group_size == 0 + { + let result = run_callback(callback, array_index, &line_str, context).await?; + if !result.is_normal_flow() { + return Ok(Some(result)); + } + } + + context.shell.env_mut().update_or_add_array_element( + &self.array_var_name, + array_index.to_string(), + line_str, + |_| Ok(()), + env::EnvironmentLookup::Anywhere, + env::EnvironmentScope::Global, + )?; + + entry_count += 1; } - Ok(variables::ArrayLiteral(entries)) + Ok(None) } } +async fn run_callback( + callback: &str, + array_index: i64, + line: &str, + context: &mut brush_core::ExecutionContext<'_, SE>, +) -> Result { + let index_arg = array_index.to_string(); + let index_arg = escape::quote_if_needed(&index_arg, escape::QuoteMode::SingleQuote); + let line_arg = escape::quote_if_needed(line, escape::QuoteMode::SingleQuote); + + let mut command = String::with_capacity(callback.len() + index_arg.len() + line_arg.len() + 2); + command.push_str(callback); + command.push(' '); + command.push_str(index_arg.as_ref()); + command.push(' '); + command.push_str(line_arg.as_ref()); + + let source_info = context.shell.call_stack().current_pos_as_source_info(); + context.shell.run_string(command, &source_info, &context.params).await +} + fn setup_terminal_settings( file: &brush_core::openfiles::OpenFile, ) -> Result, brush_core::Error> { diff --git a/crates/brush-builtins-vendored/src/read.rs b/crates/brush-builtins-vendored/src/read.rs index cfce2c1b0..f31128553 100644 --- a/crates/brush-builtins-vendored/src/read.rs +++ b/crates/brush-builtins-vendored/src/read.rs @@ -84,12 +84,6 @@ impl builtins::Command for ReadCommand { &self, context: brush_core::ExecutionContext<'_, SE>, ) -> Result { - if self.use_readline { - return error::unimp("read -e"); - } - if self.initial_text.is_some() { - return error::unimp("read -i"); - } // Validate timeout value if provided. if let Some(result) = self.validate_timeout(&context)? { @@ -105,6 +99,17 @@ impl builtins::Command for ReadCommand { let input_stream = context .try_fd(fd_num) .ok_or_else(|| ErrorKind::BadFileDescriptor(fd_num))?; + // Bash only uses readline for `read -e` when reading from a terminal. + // For non-terminal input, `-e` and `-i` are accepted but do not affect + // the bytes read. `-i` only supplies initial text to readline, so without + // an available readline-backed terminal path it is likewise a no-op. + if self.use_readline && input_stream.is_terminal() { + return error::unimp(if self.initial_text.is_some() { + "read -e -i" + } else { + "read -e" + }); + } // Retrieve effective value of IFS for splitting. // We convert to owned String to release the borrow before the mutable borrow @@ -272,6 +277,7 @@ fn build_variable_fields( /// /// This enum clearly represents all possible outcomes of `read_line()`, /// making the contract with callers explicit. +#[derive(Debug)] enum ReadResult { /// Successfully read a complete line (delimiter or char limit reached). Line(String), diff --git a/crates/brush-builtins-vendored/src/true_.rs b/crates/brush-builtins-vendored/src/true_.rs index f75cd8d59..739b80779 100644 --- a/crates/brush-builtins-vendored/src/true_.rs +++ b/crates/brush-builtins-vendored/src/true_.rs @@ -1,8 +1,21 @@ -use brush_core::{ExecutionResult, builtins, error}; +use brush_core::{ExecutionResult, builtins}; /// No-op command. Same with :. pub(crate) struct TrueCommand {} +const MAN_PAGE: &str = "\ +TRUE(1) + +NAME + true - return a successful result + +SYNOPSIS + true + +DESCRIPTION + The true utility returns a successful exit status. +"; + impl builtins::SimpleCommand for TrueCommand { fn get_content( _name: &str, @@ -13,7 +26,7 @@ impl builtins::SimpleCommand for TrueCommand { builtins::ContentType::DetailedHelp => Ok("Returns a successful exit status.".into()), builtins::ContentType::ShortUsage => Ok("true".into()), builtins::ContentType::ShortDescription => Ok("true - success".into()), - builtins::ContentType::ManPage => error::unimp("man page not yet implemented"), + builtins::ContentType::ManPage => Ok(MAN_PAGE.into()), } } diff --git a/crates/brush-builtins-vendored/src/umask.rs b/crates/brush-builtins-vendored/src/umask.rs index 76fb9298d..dff7552ae 100644 --- a/crates/brush-builtins-vendored/src/umask.rs +++ b/crates/brush-builtins-vendored/src/umask.rs @@ -33,7 +33,9 @@ impl builtins::Command for UmaskCommand { let parsed = brush_core::int_utils::parse(mode.as_str(), 8)?; set_umask(parsed)?; } else { - return brush_core::error::unimp("umask setting mode from symbolic value"); + let current_umask = get_umask()?; + let parsed = parse_symbolic_umask(mode, current_umask)?; + set_umask(parsed)?; } } else { let umask = get_umask()?; @@ -74,6 +76,81 @@ cfg_if! { } } +fn parse_symbolic_umask(mode: &str, current_umask: u32) -> Result { + let mut umask = current_umask & 0o777; + let mut chars = mode.chars().peekable(); + let mut saw_clause = false; + + while chars.peek().is_some() { + saw_clause = true; + + let mut who_bits = 0; + while let Some(&ch) = chars.peek() { + let bits = match ch { + 'u' => 0o700, + 'g' => 0o070, + 'o' => 0o007, + 'a' => 0o777, + _ => break, + }; + who_bits |= bits; + chars.next(); + } + if who_bits == 0 { + who_bits = 0o777; + } + + loop { + let op = chars.next().ok_or(ErrorKind::InvalidUmask)?; + if !matches!(op, '+' | '-' | '=') { + return Err(ErrorKind::InvalidUmask.into()); + } + + let mut perm_bits = 0; + while let Some(&ch) = chars.peek() { + let bits = match ch { + 'r' => 0o444, + 'w' => 0o222, + 'x' => 0o111, + '+' | '-' | '=' | ',' => break, + _ => return Err(ErrorKind::InvalidUmask.into()), + }; + perm_bits |= bits & who_bits; + chars.next(); + } + + match op { + '+' => umask &= !perm_bits, + '-' => umask |= perm_bits, + '=' => { + umask |= who_bits; + umask &= !perm_bits; + } + _ => unreachable!(), + } + + match chars.peek() { + Some(',') => { + chars.next(); + if chars.peek().is_none() { + return Err(ErrorKind::InvalidUmask.into()); + } + break; + } + Some('+' | '-' | '=') => continue, + Some(_) => return Err(ErrorKind::InvalidUmask.into()), + None => break, + } + } + } + + if saw_clause { + Ok(umask as nix::sys::stat::mode_t) + } else { + Err(ErrorKind::InvalidUmask.into()) + } +} + fn set_umask(value: nix::sys::stat::mode_t) -> Result<(), brush_core::Error> { // value of mode_t can be platform dependent let mode = nix::sys::stat::Mode::from_bits(value).ok_or_else(|| ErrorKind::InvalidUmask)?; @@ -96,3 +173,39 @@ fn symbolic_mask_from_bits(bits: u32) -> String { result } + +#[cfg(test)] +mod tests { + use super::*; + + fn parse(mode: &str, current_umask: u32) -> u32 { + parse_symbolic_umask(mode, current_umask).unwrap() as u32 + } + + #[test] + fn parses_symbolic_umask_assignments() { + assert_eq!(parse("u=rwx,g=rx,o=", 0o022), 0o027); + assert_eq!(parse("=r", 0o022), 0o333); + assert_eq!(parse("a=", 0o022), 0o777); + assert_eq!(parse("u=", 0o022), 0o722); + } + + #[test] + fn parses_symbolic_umask_incremental_ops() { + assert_eq!(parse("u+rw", 0o777), 0o177); + assert_eq!(parse("g-w", 0o022), 0o022); + assert_eq!(parse("+x", 0o022), 0o022); + assert_eq!(parse("u+r-w", 0o777), 0o377); + assert_eq!(parse("a+r,u-w", 0o777), 0o333); + } + + #[test] + fn rejects_invalid_symbolic_umasks() { + assert!(parse_symbolic_umask("", 0o022).is_err()); + assert!(parse_symbolic_umask("u", 0o022).is_err()); + assert!(parse_symbolic_umask("u+z", 0o022).is_err()); + assert!(parse_symbolic_umask("z+r", 0o022).is_err()); + assert!(parse_symbolic_umask("u=,", 0o022).is_err()); + assert!(parse_symbolic_umask("u,,g=r", 0o022).is_err()); + } +} diff --git a/crates/brush-builtins-vendored/src/unset.rs b/crates/brush-builtins-vendored/src/unset.rs index 38e952fe3..465022416 100644 --- a/crates/brush-builtins-vendored/src/unset.rs +++ b/crates/brush-builtins-vendored/src/unset.rs @@ -42,11 +42,12 @@ impl builtins::Command for UnsetCommand { &self, context: brush_core::ExecutionContext<'_, SE>, ) -> Result { - // - // TODO(nameref): implement nameref - // if self.name_interpretation.name_references { - return brush_core::error::unimp("unset: name references are not yet implemented"); + for name in &self.names { + unset_name_reference(context.shell, name)?; + } + + return Ok(ExecutionResult::success()); } let unspecified = self.name_interpretation.unspecified(); @@ -91,6 +92,27 @@ impl builtins::Command for UnsetCommand { Ok(ExecutionResult::success()) } } +fn unset_name_reference( + shell: &mut Shell, + name: &str, +) -> Result { + let Ok(brush_parser::word::Parameter::Named(name)) = + brush_parser::word::parse_parameter(name, &shell.parser_options()) + else { + return Ok(false); + }; + + if shell + .env() + .get(name.as_str()) + .is_some_and(|(_, var)| var.is_treated_as_nameref()) + { + shell.env_mut().unset(name.as_str()).map(|v| v.is_some()) + } else { + Ok(false) + } +} + fn unset_array_index( shell: &mut Shell, diff --git a/crates/brush-core-vendored/src/builtins.rs b/crates/brush-core-vendored/src/builtins.rs index 0dcf698da..602bc82cd 100644 --- a/crates/brush-core-vendored/src/builtins.rs +++ b/crates/brush-core-vendored/src/builtins.rs @@ -174,8 +174,138 @@ impl Registration { } } -fn get_builtin_man_page(_name: &str, _command: &clap::Command) -> Result { - error::unimp("man page rendering is not yet implemented") +fn get_builtin_man_page(name: &str, command: &clap::Command) -> Result { + let mut man_page = String::new(); + + append_man_section(&mut man_page, "NAME"); + let description = command + .get_about() + .map_or_else(String::new, std::string::ToString::to_string); + if description.is_empty() { + man_page.push_str(name); + man_page.push('\n'); + } else { + man_page.push_str(name); + man_page.push_str(" - "); + man_page.push_str(&description); + man_page.push('\n'); + } + + append_man_section(&mut man_page, "SYNOPSIS"); + let mut usage_command = command.clone(); + let usage = usage_command.render_usage(); + man_page.push_str(&usage.to_string()); + man_page.push('\n'); + + if let Some(about) = command.get_long_about().or_else(|| command.get_about()) { + append_man_section(&mut man_page, "DESCRIPTION"); + man_page.push_str(&about.to_string()); + man_page.push('\n'); + } + + append_man_arguments_section(&mut man_page, command); + append_man_options_section(&mut man_page, command); + + Ok(man_page) +} + +fn append_man_section(buf: &mut String, title: &str) { + if !buf.is_empty() { + buf.push('\n'); + } + + buf.push_str(title); + buf.push('\n'); +} + +fn append_man_arguments_section(buf: &mut String, command: &clap::Command) { + let mut section_written = false; + for arg in command.get_positionals() { + if arg.is_hide_set() { + continue; + } + + if !section_written { + append_man_section(buf, "ARGUMENTS"); + section_written = true; + } + + write_man_arg_help(buf, &format_man_value_names(arg), arg); + } +} + +fn append_man_options_section(buf: &mut String, command: &clap::Command) { + let mut section_written = false; + for arg in command.get_opts() { + if arg.is_hide_set() { + continue; + } + + if !section_written { + append_man_section(buf, "OPTIONS"); + section_written = true; + } + + write_man_arg_help(buf, &format_man_option(arg), arg); + } +} + +fn write_man_arg_help(buf: &mut String, label: &str, arg: &clap::Arg) { + buf.push_str(" "); + buf.push_str(label); + buf.push('\n'); + if let Some(help) = arg.get_long_help().or_else(|| arg.get_help()) { + buf.push_str(" "); + buf.push_str(&help.to_string()); + buf.push('\n'); + } +} + +fn format_man_option(arg: &clap::Arg) -> String { + let mut option = String::new(); + if let Some(short) = arg.get_short() { + option.push('-'); + option.push(short); + } + + if let Some(long) = arg.get_long() { + if !option.is_empty() { + option.push_str(", "); + } + + option.push_str("--"); + option.push_str(long); + } + + let values = format_man_value_names(arg); + if !values.is_empty() { + if !option.is_empty() { + option.push(' '); + } + + option.push_str(&values); + } + + option +} + +fn format_man_value_names(arg: &clap::Arg) -> String { + let Some(value_names) = arg.get_value_names() else { + return String::new(); + }; + + let mut values = String::new(); + for name in value_names { + if !values.is_empty() { + values.push(' '); + } + + values.push('<'); + values.push_str(name); + values.push('>'); + } + + values } fn get_builtin_short_description(name: &str, command: &clap::Command) -> String { diff --git a/crates/brush-core-vendored/src/commands.rs b/crates/brush-core-vendored/src/commands.rs index 0cdf8d406..eef2c3244 100644 --- a/crates/brush-core-vendored/src/commands.rs +++ b/crates/brush-core-vendored/src/commands.rs @@ -814,8 +814,19 @@ pub(crate) async fn invoke_shell_function( // Handle control-flow. match result.next_control_flow { - ExecutionControlFlow::BreakLoop { .. } | ExecutionControlFlow::ContinueLoop { .. } => { - return error::unimp("break or continue returned from function invocation"); + ExecutionControlFlow::BreakLoop { .. } => { + writeln!( + context.params.stderr(context.shell), + "break: only meaningful in a `for', `while', or `until' loop" + )?; + result.next_control_flow = ExecutionControlFlow::Normal; + }, + ExecutionControlFlow::ContinueLoop { .. } => { + writeln!( + context.params.stderr(context.shell), + "continue: only meaningful in a `for', `while', or `until' loop" + )?; + result.next_control_flow = ExecutionControlFlow::Normal; }, ExecutionControlFlow::ReturnFromFunctionOrScript => { // It's now been handled. diff --git a/crates/brush-core-vendored/src/error.rs b/crates/brush-core-vendored/src/error.rs index 67b9da42b..9b673d0d9 100644 --- a/crates/brush-core-vendored/src/error.rs +++ b/crates/brush-core-vendored/src/error.rs @@ -29,6 +29,10 @@ pub enum ErrorKind { #[error("cannot assign list to array member")] AssigningListToArrayMember, + /// An attempt was made to assign an associative array value without using a subscript. + #[error("must use subscript when assigning associative array")] + AssociativeArrayMissingSubscript, + /// An attempt was made to convert an associative array to an indexed array. #[error("cannot convert associative array to indexed array")] ConvertingAssociativeArrayToIndexedArray, diff --git a/crates/brush-core-vendored/src/extendedtests.rs b/crates/brush-core-vendored/src/extendedtests.rs index 965397f2e..ec42d1c49 100644 --- a/crates/brush-core-vendored/src/extendedtests.rs +++ b/crates/brush-core-vendored/src/extendedtests.rs @@ -4,7 +4,7 @@ use brush_parser::ast; use crate::{ ExecutionParameters, Shell, ShellFd, arithmetic, env, error, escape, expansion, extensions, - namedoptions, patterns, + namedoptions, patterns, regex, sys::{ fs::{MetadataExt, PathExt}, users, @@ -160,7 +160,13 @@ pub(crate) fn apply_unary_predicate_to_str( Ok(md.gid() == users::get_effective_gid()?) }, ast::UnaryPredicate::FileExistsAndModifiedSinceLastRead => { - error::unimp("unary extended test predicate: FileExistsAndModifiedSinceLastRead") + let path = shell.absolute_path(Path::new(operand)); + if !path.exists() { + return Ok(false); + } + + let md = path.metadata()?; + Ok(md.modified()? > md.accessed()?) }, ast::UnaryPredicate::FileExistsAndOwnedByEffectiveUserId => { let path = shell.absolute_path(Path::new(operand)); @@ -527,7 +533,15 @@ pub(crate) fn apply_binary_predicate_to_strs( }, ast::BinaryPredicate::StringExactlyMatchesString => Ok(left == right), ast::BinaryPredicate::StringDoesNotExactlyMatchString => Ok(left != right), - _ => error::unimp("unsupported test binary predicate"), + ast::BinaryPredicate::StringContainsSubstring => Ok(left.contains(right)), + ast::BinaryPredicate::StringMatchesRegex => { + let re = regex::compile_regex( + right.to_owned(), + shell.options().case_insensitive_conditionals, + true, + )?; + Ok(re.is_match(left)?) + }, } } diff --git a/crates/brush-core-vendored/src/interp.rs b/crates/brush-core-vendored/src/interp.rs index 408182333..5741f789b 100644 --- a/crates/brush-core-vendored/src/interp.rs +++ b/crates/brush-core-vendored/src/interp.rs @@ -1766,7 +1766,7 @@ async fn apply_assignment( existing_value.assign_at_index(array_index, s, assignment.append)?; }, ShellValueLiteral::Array(_) => { - return error::unimp("replacing an array item with an array"); + return Err(error::ErrorKind::AssigningListToArrayMember.into()); }, } } else { @@ -1796,7 +1796,7 @@ async fn apply_assignment( ShellValue::indexed_array_from_literals(ArrayLiteral(vec![(Some(array_index), s)])) }, ShellValueLiteral::Array(_) => { - return error::unimp("cannot assign list to array member"); + return Err(error::ErrorKind::AssigningListToArrayMember.into()); }, } } else { @@ -1917,7 +1917,10 @@ pub(crate) async fn setup_redirect( ast::IoFileRedirectKind::DuplicateInput => 0, ast::IoFileRedirectKind::DuplicateOutput => 1, _ => { - return error::unimp("unexpected redirect kind"); + return Err(error::ErrorKind::InternalError(format!( + "unexpected redirect kind for file descriptor target: {kind:?}" + )) + .into()); }, }; @@ -1937,7 +1940,10 @@ pub(crate) async fn setup_redirect( ast::IoFileRedirectKind::DuplicateInput => 0, ast::IoFileRedirectKind::DuplicateOutput => 1, _ => { - return error::unimp("unexpected redirect kind"); + return Err(error::ErrorKind::InternalError(format!( + "unexpected redirect kind for duplicate target: {kind:?}" + )) + .into()); }, }; @@ -2008,7 +2014,12 @@ pub(crate) async fn setup_redirect( params.open_files.set_fd(fd_num, target_file); }, - _ => return error::unimp("invalid process substitution"), + _ => { + return Err(error::ErrorKind::InternalError(format!( + "process substitution used with invalid redirect kind: {kind:?}" + )) + .into()); + }, } }, } @@ -2113,6 +2124,16 @@ fn setup_process_substitution( let mut child_params = params.clone(); child_params.process_group_policy = ProcessGroupPolicy::SameProcessGroup; + // Starting at 63 (a.k.a. 64-1)--and decrementing--look for an + // available fd before starting the substitution command. + let mut candidate_fd_num = 63; + while params.open_files.contains_fd(candidate_fd_num) { + candidate_fd_num -= 1; + if candidate_fd_num == 0 { + return Err(error::ErrorKind::TooManyOpenFiles.into()); + } + } + // Set up pipe so we can connect to the command. let (reader, writer) = std::io::pipe()?; let (reader, writer) = (reader.into(), writer.into()); @@ -2139,15 +2160,6 @@ fn setup_process_substitution( .await; }); - // Starting at 63 (a.k.a. 64-1)--and decrementing--look for an - // available fd. - let mut candidate_fd_num = 63; - while params.open_files.contains_fd(candidate_fd_num) { - candidate_fd_num -= 1; - if candidate_fd_num == 0 { - return error::unimp("no available file descriptors"); - } - } Ok((candidate_fd_num, target_file)) } diff --git a/crates/brush-core-vendored/src/jobs.rs b/crates/brush-core-vendored/src/jobs.rs index 226650542..bc81dccab 100644 --- a/crates/brush-core-vendored/src/jobs.rs +++ b/crates/brush-core-vendored/src/jobs.rs @@ -226,7 +226,6 @@ impl JobManager { self.jobs.iter().any(|job| job.contains_process_id(pid)) } - /// Tries to resolve the given process ID to a managed job. /// /// # Arguments @@ -534,16 +533,17 @@ impl Job { /// Moves the job to execute in the background. pub fn move_to_background(&mut self) -> Result<(), error::Error> { - if matches!(self.state, JobState::Stopped) { - if let Some(pgid) = self.process_group_id() { + match &self.state { + JobState::Stopped => { + let pgid = self + .process_group_id() + .ok_or(error::ErrorKind::FailedToSendSignal)?; sys::signal::continue_process(pgid)?; self.state = JobState::Running; Ok(()) - } else { - Err(error::ErrorKind::FailedToSendSignal.into()) - } - } else { - error::unimp("move job to background") + }, + JobState::Running => Ok(()), + JobState::Unknown | JobState::Done => Err(error::ErrorKind::FailedToSendSignal.into()), } } @@ -605,7 +605,6 @@ impl Job { } } - fn contains_process_id(&self, pid: i32) -> bool { self.tasks.iter().any(|task| match task { JobTask::External(process) => process.pid().is_some_and(|process_pid| process_pid == pid), diff --git a/crates/brush-core-vendored/src/prompt.rs b/crates/brush-core-vendored/src/prompt.rs index 8dff6972d..06d4f32a2 100644 --- a/crates/brush-core-vendored/src/prompt.rs +++ b/crates/brush-core-vendored/src/prompt.rs @@ -71,7 +71,7 @@ fn format_prompt_piece( return error::unimp("prompt: current command number"); }, brush_parser::prompt::PromptPiece::CurrentHistoryNumber => { - return error::unimp("prompt: current history number"); + format_current_history_number(shell) }, brush_parser::prompt::PromptPiece::CurrentUser => users::get_current_username()?, brush_parser::prompt::PromptPiece::CurrentWorkingDirectory { tilde_replaced, basename } => { @@ -171,6 +171,13 @@ fn format_current_working_directory( working_dir_str } +fn format_current_history_number(shell: &Shell) -> String { + // Bash renders \! as the history number that will be assigned to the next + // interactive command. When command history is disabled, bash keeps this at + // 1 rather than rendering 0. + shell.history().map_or(1, |history| history.count() + 1).to_string() +} + fn format_time( datetime: &chrono::DateTime, format: &brush_parser::prompt::PromptTimeFormat, diff --git a/crates/brush-core-vendored/src/shell/funcs.rs b/crates/brush-core-vendored/src/shell/funcs.rs index 063199bd6..5f23b438b 100644 --- a/crates/brush-core-vendored/src/shell/funcs.rs +++ b/crates/brush-core-vendored/src/shell/funcs.rs @@ -1,7 +1,10 @@ //! Function support for shells. +use std::io::Write; + use crate::{ - ExecutionParameters, commands, error, extensions, functions, results::ExecutionWaitResult, + ExecutionParameters, commands, error, extensions, functions, jobs, + results::{ExecutionResult, ExecutionWaitResult}, }; impl crate::Shell { @@ -117,7 +120,20 @@ impl crate::Shell { match result.wait_with_cancel(params.cancel_token()).await? { ExecutionWaitResult::Completed(result) => Ok(result.exit_code.into()), - ExecutionWaitResult::Stopped(..) => error::unimp("stopped child from function invocation"), + ExecutionWaitResult::Stopped(child) => { + let result = ExecutionResult::stopped(); + let job = self.jobs_mut().add_as_current(jobs::Job::new( + [jobs::JobTask::External(child)], + name.to_owned(), + jobs::JobState::Stopped, + )); + let formatted = job.to_string(); + + // N.B. We use the '\r' to overwrite any ^Z output. + writeln!(params.stderr(self), "\r{formatted}")?; + + Ok(result.exit_code.into()) + }, } } } diff --git a/crates/brush-core-vendored/src/shell/initscripts.rs b/crates/brush-core-vendored/src/shell/initscripts.rs index a30e16d21..31f26fd5b 100644 --- a/crates/brush-core-vendored/src/shell/initscripts.rs +++ b/crates/brush-core-vendored/src/shell/initscripts.rs @@ -2,7 +2,7 @@ use std::path::PathBuf; -use crate::{Shell, error, extensions, interp}; +use crate::{Shell, error, expansion, extensions, interp}; /// Behavior for loading profile files. #[derive(Default)] @@ -130,14 +130,24 @@ impl Shell { "BASH_ENV" }; - if self.env.is_set(env_var_name) { - // - // TODO(well-known-vars): look at $ENV/BASH_ENV; source its expansion if that - // file exists - // - return error::unimp( - "load config from $ENV/BASH_ENV for non-interactive, non-login shell", - ); + if let Some(config_path) = self.env_str(env_var_name) { + let config_path = config_path.into_owned(); + let options = expansion::ExpanderOptions { + brace_expand: false, + pathname_expand: false, + ..Default::default() + }; + let expanded_path = expansion::basic_expand_word_with_options( + self, + ¶ms, + config_path.as_str(), + &options, + ) + .await?; + + if !expanded_path.is_empty() { + self.source_if_exists(PathBuf::from(expanded_path), ¶ms).await?; + } } } } diff --git a/crates/brush-core-vendored/src/variables.rs b/crates/brush-core-vendored/src/variables.rs index a89c2409b..ea839f89a 100644 --- a/crates/brush-core-vendored/src/variables.rs +++ b/crates/brush-core-vendored/src/variables.rs @@ -437,10 +437,8 @@ impl ShellVariable { } Ok(()) }, - _ => { - tracing::error!("assigning to index {array_index} of {:?}", self.value); - error::unimp("assigning to index of non-array variable") - }, + ShellValue::Dynamic { .. } => Ok(()), + ShellValue::Unset(_) | ShellValue::String(_) => Err(error::ErrorKind::NotArray.into()), } } @@ -800,23 +798,45 @@ impl ShellValue { existing_values: &mut BTreeMap, literal_values: ArrayLiteral, ) -> Result<(), error::Error> { - let mut current_key = None; - for (key, value) in literal_values.0 { - if let Some(current_key) = current_key.take() { - if key.is_some() { - return error::unimp("misaligned keys/values in associative array literal"); - } else { - existing_values.insert(current_key, value); - } - } else if let Some(key) = key { - existing_values.insert(key, value); - } else { - current_key = Some(value); - } - } + let mut literal_values = literal_values.0.into_iter(); + let Some((first_key, first_value)) = literal_values.next() else { + return Ok(()); + }; - if let Some(current_key) = current_key { - existing_values.insert(current_key, String::new()); + if let Some(first_key) = first_key { + existing_values.insert(first_key, first_value); + + for (key, value) in literal_values { + let Some(key) = key else { + return Err(error::ErrorKind::AssociativeArrayMissingSubscript.into()); + }; + + existing_values.insert(key, value); + } + } else { + let mut current_key = Some(first_value); + for (key, value) in literal_values { + let value = if let Some(key) = key { + let mut word = String::with_capacity(key.len() + value.len() + 3); + word.push('['); + word.push_str(key.as_str()); + word.push_str("]="); + word.push_str(value.as_str()); + word + } else { + value + }; + + if let Some(key) = current_key.take() { + existing_values.insert(key, value); + } else { + current_key = Some(value); + } + } + + if let Some(current_key) = current_key { + existing_values.insert(current_key, String::new()); + } } Ok(())