diff --git a/.config/nextest.toml b/.config/nextest.toml index ac4f8ee4..34500a2a 100644 --- a/.config/nextest.toml +++ b/.config/nextest.toml @@ -2,6 +2,7 @@ retries = 3 status-level = "slow" slow-timeout = { period = "5s", terminate-after = 3, grace-period = "2s" } +fail-fast = false [profile.default.junit] path = "test-results.xml" diff --git a/brush-core/src/builtins/complete.rs b/brush-core/src/builtins/complete.rs index 3634c945..0f70bfdf 100644 --- a/brush-core/src/builtins/complete.rs +++ b/brush-core/src/builtins/complete.rs @@ -442,7 +442,6 @@ impl builtins::Command for CompGenCommand { context: commands::ExecutionContext<'_>, ) -> Result { let spec = self.common_args.create_spec(); - let token_to_complete = self.word.as_deref().unwrap_or_default(); // N.B. We basic-expand the token-to-be-completed first. @@ -471,6 +470,12 @@ impl builtins::Command for CompGenCommand { match result { completion::Answer::Candidates(candidates, _options) => { + // We are expected to return 1 if there are no candidates, even if no errors + // occurred along the way. + if candidates.is_empty() { + return Ok(builtins::ExitCode::Custom(1)); + } + for candidate in candidates { writeln!(context.stdout(), "{candidate}")?; } diff --git a/brush-core/src/builtins/echo.rs b/brush-core/src/builtins/echo.rs index 3b7aaa1d..ffa07dea 100644 --- a/brush-core/src/builtins/echo.rs +++ b/brush-core/src/builtins/echo.rs @@ -55,7 +55,7 @@ impl builtins::Command for EchoCommand { let (expanded_arg, keep_going) = escape::expand_backslash_escapes( arg.as_str(), - escape::EscapeMode::EchoBuiltin, + escape::EscapeExpansionMode::EchoBuiltin, )?; s.push_str(&String::from_utf8_lossy(expanded_arg.as_slice())); diff --git a/brush-core/src/builtins/printf.rs b/brush-core/src/builtins/printf.rs index 4bf87003..fcdc712b 100644 --- a/brush-core/src/builtins/printf.rs +++ b/brush-core/src/builtins/printf.rs @@ -1,7 +1,7 @@ use clap::Parser; use std::io::Write; -use crate::{builtins, commands, expansion}; +use crate::{builtins, commands, escape, expansion}; /// Format a string. #[derive(Parser)] @@ -40,13 +40,30 @@ impl PrintfCommand { &self, context: &commands::ExecutionContext<'_>, ) -> Result { - // Special-case common format string: "%s". match self.format_and_args.as_slice() { + // Special-case common format string: "%s". [fmt, arg] if fmt == "%s" => Ok(arg.clone()), + // Special-case invocation of printf with %q-based format string from bash-completion. + // It has hard-coded expectation of backslash-style escaping instead of quoting. + [fmt, arg] if fmt == "%q" || fmt == "~%q" => { + Ok(Self::evaluate_format_with_percent_q(None, arg)) + } + [fmt, arg] if fmt == "~%q" => Ok(Self::evaluate_format_with_percent_q(Some("~"), arg)), + // Fallback to external command. _ => self.evaluate_via_external_command(context), } } + fn evaluate_format_with_percent_q(prefix: Option<&str>, arg: &str) -> String { + let mut result = escape::quote_if_needed(arg, escape::QuoteMode::BackslashEscape); + + if let Some(prefix) = prefix { + result.insert_str(0, prefix); + } + + result + } + #[allow(clippy::unwrap_in_result)] fn evaluate_via_external_command( &self, diff --git a/brush-core/src/commands.rs b/brush-core/src/commands.rs index 88aa5b6c..dc39a072 100644 --- a/brush-core/src/commands.rs +++ b/brush-core/src/commands.rs @@ -8,7 +8,7 @@ use command_fds::{CommandFdExt, FdMapping}; use itertools::Itertools; use crate::{ - builtins, error, + builtins, error, escape, interp::{self, Execute, ProcessGroupPolicy}, openfiles::{self, OpenFile, OpenFiles}, processes, sys, trace_categories, ExecutionParameters, ExecutionResult, Shell, @@ -177,6 +177,24 @@ impl From<&String> for CommandArg { } } +impl CommandArg { + pub fn quote_for_tracing(&self) -> String { + match self { + CommandArg::String(s) => escape::quote_if_needed(s, escape::QuoteMode::Quote), + CommandArg::Assignment(a) => { + let mut s = a.name.to_string(); + let op = if a.append { "+=" } else { "=" }; + s.push_str(op); + s.push_str(&escape::quote_if_needed( + a.value.to_string().as_str(), + escape::QuoteMode::Quote, + )); + s + } + } + } +} + #[allow(unused_variables)] pub(crate) fn compose_std_command>( shell: &mut Shell, diff --git a/brush-core/src/completion.rs b/brush-core/src/completion.rs index 79dcd44d..d90cca1d 100644 --- a/brush-core/src/completion.rs +++ b/brush-core/src/completion.rs @@ -235,13 +235,126 @@ impl Spec { shell: &mut Shell, context: &Context<'_>, ) -> Result { - let mut candidates: IndexSet = IndexSet::new(); - // Store the current options in the shell; this is needed since the compopt // built-in has the ability of modifying the options for an in-flight // completion process. shell.completion_config.current_completion_options = Some(self.options.clone()); + // Generate completions based on any provided actions (and on words). + let mut candidates = self.generate_action_completions(shell, context)?; + if let Some(word_list) = &self.word_list { + let words = crate::expansion::full_expand_and_split_str(shell, word_list).await?; + for word in words { + candidates.insert(word); + } + } + + // Only keep generated completions that match the token being completed. Further + // generations below don't get filtered. + if !context.token_to_complete.is_empty() { + candidates.retain(|candidate| candidate.starts_with(context.token_to_complete)); + } + + if let Some(glob_pattern) = &self.glob_pattern { + let pattern = patterns::Pattern::from(glob_pattern.as_str()); + let expansions = pattern.expand( + shell.working_dir.as_path(), + shell.parser_options().enable_extended_globbing, + Some(&patterns::Pattern::accept_all_expand_filter), + )?; + + for expansion in expansions { + candidates.insert(expansion); + } + } + if let Some(function_name) = &self.function_name { + let call_result = self + .call_completion_function(shell, function_name.as_str(), context) + .await?; + + match call_result { + Answer::RestartCompletionProcess => return Ok(call_result), + Answer::Candidates(mut new_candidates, _options) => { + candidates.append(&mut new_candidates); + } + } + } + if let Some(command) = &self.command { + tracing::debug!(target: trace_categories::COMPLETION, "UNIMPLEMENTED: complete -C({command})"); + } + + // Apply filter pattern, if present. + if let Some(filter_pattern) = &self.filter_pattern { + if !filter_pattern.is_empty() { + tracing::debug!(target: trace_categories::COMPLETION, "UNIMPLEMENTED: complete -X (filter pattern): '{filter_pattern}'"); + } + } + + // Add prefix and/or suffix, if present. + if self.prefix.is_some() || self.suffix.is_some() { + let empty = String::new(); + let prefix = self.prefix.as_ref().unwrap_or(&empty); + let suffix = self.suffix.as_ref().unwrap_or(&empty); + + let mut updated = IndexSet::new(); + for candidate in candidates { + updated.insert(std::format!("{prefix}{candidate}{suffix}")); + } + + candidates = updated; + } + + // + // Now apply options + // + + let options = if let Some(options) = &shell.completion_config.current_completion_options { + options + } else { + &self.options + }; + + let processing_options = ProcessingOptions { + treat_as_filenames: options.file_names, + no_autoquote_filenames: options.no_quote, + no_trailing_space_at_end_of_line: options.no_space, + }; + + if candidates.is_empty() { + if options.bash_default { + // TODO: if we have no completions, then fall back to default "bash" completion + tracing::debug!(target: trace_categories::COMPLETION, "UNIMPLEMENTED: complete -o bashdefault"); + } + if options.default { + // TODO: if we have no completions, then fall back to default file name completion + tracing::debug!(target: trace_categories::COMPLETION, "UNIMPLEMENTED: complete -o default"); + } + if options.dir_names { + // TODO: if we have no completions, then fall back to performing dir name completion + tracing::debug!(target: trace_categories::COMPLETION, "UNIMPLEMENTED: complete -o dirnames"); + } + } + if options.plus_dirs { + // Also add dir name completion. + tracing::debug!(target: trace_categories::COMPLETION, "UNIMPLEMENTED: complete -o plusdirs"); + } + + // Sort, unless blocked by options. + if !self.options.no_sort { + candidates.sort(); + } + + Ok(Answer::Candidates(candidates, processing_options)) + } + + #[allow(clippy::too_many_lines)] + fn generate_action_completions( + &self, + shell: &mut Shell, + context: &Context<'_>, + ) -> Result, error::Error> { + let mut candidates = IndexSet::new(); + for action in &self.actions { match action { CompleteAction::Alias => { @@ -256,7 +369,9 @@ impl Spec { } } } - CompleteAction::Binding => tracing::debug!("UNIMPLEMENTED: complete -A binding"), + CompleteAction::Binding => { + tracing::debug!(target: trace_categories::COMPLETION, "UNIMPLEMENTED: complete -A binding"); + } CompleteAction::Builtin => { for name in shell.builtins.keys() { candidates.insert(name.to_owned()); @@ -334,7 +449,9 @@ impl Spec { } } } - CompleteAction::Service => tracing::debug!("UNIMPLEMENTED: complete -A service"), + CompleteAction::Service => { + tracing::debug!(target: trace_categories::COMPLETION, "UNIMPLEMENTED: complete -A service"); + } CompleteAction::SetOpt => { for (name, _) in namedoptions::SET_O_OPTIONS.iter() { candidates.insert((*name).to_owned()); @@ -370,107 +487,7 @@ impl Spec { } } - if let Some(glob_pattern) = &self.glob_pattern { - let pattern = patterns::Pattern::from(glob_pattern.as_str()); - let expansions = pattern.expand( - shell.working_dir.as_path(), - shell.parser_options().enable_extended_globbing, - Some(&patterns::Pattern::accept_all_expand_filter), - )?; - - for expansion in expansions { - candidates.insert(expansion); - } - } - if let Some(word_list) = &self.word_list { - let words = crate::expansion::full_expand_and_split_str(shell, word_list).await?; - for word in words { - candidates.insert(word); - } - } - if let Some(function_name) = &self.function_name { - let call_result = self - .call_completion_function(shell, function_name.as_str(), context) - .await?; - - match call_result { - Answer::RestartCompletionProcess => return Ok(call_result), - Answer::Candidates(mut new_candidates, _options) => { - candidates.append(&mut new_candidates); - } - } - } - if let Some(command) = &self.command { - tracing::debug!("UNIMPLEMENTED: complete -C({command})"); - } - - // Make sure the token we have (if non-empty) is a prefix. - if !context.token_to_complete.is_empty() { - candidates.retain(|candidate| candidate.starts_with(context.token_to_complete)); - } - - // Apply filter pattern, if present. - if let Some(filter_pattern) = &self.filter_pattern { - if !filter_pattern.is_empty() { - tracing::debug!("UNIMPLEMENTED: complete -X (filter pattern): '{filter_pattern}'"); - } - } - - // Add prefix and/or suffix, if present. - if self.prefix.is_some() || self.suffix.is_some() { - let empty = String::new(); - let prefix = self.prefix.as_ref().unwrap_or(&empty); - let suffix = self.suffix.as_ref().unwrap_or(&empty); - - let mut updated = IndexSet::new(); - for candidate in candidates { - updated.insert(std::format!("{prefix}{candidate}{suffix}")); - } - - candidates = updated; - } - - // - // Now apply options - // - - let options = if let Some(options) = &shell.completion_config.current_completion_options { - options - } else { - &self.options - }; - - let processing_options = ProcessingOptions { - treat_as_filenames: options.file_names, - no_autoquote_filenames: options.no_quote, - no_trailing_space_at_end_of_line: options.no_space, - }; - - if candidates.is_empty() { - if options.bash_default { - // TODO: if we have no completions, then fall back to default "bash" completion - tracing::debug!("UNIMPLEMENTED: complete -o bashdefault"); - } - if options.default { - // TODO: if we have no completions, then fall back to default file name completion - tracing::debug!("UNIMPLEMENTED: complete -o default"); - } - if options.dir_names { - // TODO: if we have no completions, then fall back to performing dir name completion - tracing::debug!("UNIMPLEMENTED: complete -o dirnames"); - } - } - if options.plus_dirs { - // Also add dir name completion. - tracing::debug!("UNIMPLEMENTED: complete -o plusdirs"); - } - - // Sort, unless blocked by options. - if !self.options.no_sort { - candidates.sort(); - } - - Ok(Answer::Candidates(candidates, processing_options)) + Ok(candidates) } async fn call_completion_function( @@ -713,114 +730,102 @@ impl Config { // Make a best-effort attempt to tokenize. let tokens = Self::tokenize_input_for_completion(shell, input); - { - let cursor: i32 = i32::try_from(position)?; - let mut preceding_token = None; - let mut completion_prefix = ""; - let mut insertion_index = cursor; - let mut completion_token_index = tokens.len(); - - // Copy a set of references to the tokens; we will adjust this list as - // we find we need to insert an empty token. - let mut adjusted_tokens: Vec<&brush_parser::Token> = tokens.iter().collect(); - - // Try to find which token we are in. - for (i, token) in tokens.iter().enumerate() { - // If the cursor is before the start of the token, then it's between - // this token and the one that preceded it (or it's before the first - // token if this is the first token). - if cursor < token.location().start.index { - // TODO: Should insert an empty token here; the position looks to have - // been between this token and the preceding one. - completion_token_index = i; - break; - } - // If the cursor is anywhere from the first char of the token up to - // (and including) the first char after the token, then this we need - // to generate completions to replace/update this token. We'll pay - // attention to the position to figure out the prefix that we should - // be completing. - else if cursor >= token.location().start.index - && cursor <= token.location().end.index - { - // Update insertion index. - insertion_index = token.location().start.index; - - // Update prefix. - let offset_into_token = (cursor - insertion_index) as usize; - let token_str = token.to_str(); - completion_prefix = &token_str[..offset_into_token]; - - // Update token index. - completion_token_index = i; - - break; - } - // Otherwise, we need to keep looking. Update what we think the - // preceding token may be. - preceding_token = Some(token); + let cursor: i32 = i32::try_from(position)?; + let mut preceding_token = None; + let mut completion_prefix = ""; + let mut insertion_index = cursor; + let mut completion_token_index = tokens.len(); + + // Copy a set of references to the tokens; we will adjust this list as + // we find we need to insert an empty token. + let mut adjusted_tokens: Vec<&brush_parser::Token> = tokens.iter().collect(); + + // Try to find which token we are in. + for (i, token) in tokens.iter().enumerate() { + // If the cursor is before the start of the token, then it's between + // this token and the one that preceded it (or it's before the first + // token if this is the first token). + if cursor < token.location().start.index { + // TODO: Should insert an empty token here; the position looks to have + // been between this token and the preceding one. + completion_token_index = i; + break; } - - // If the position is after the last token, then we need to insert an empty - // token for the new token to be generated. - let empty_token = - brush_parser::Token::Word(String::new(), brush_parser::TokenLocation::default()); - if completion_token_index == tokens.len() { - adjusted_tokens.push(&empty_token); + // If the cursor is anywhere from the first char of the token up to + // (and including) the first char after the token, then this we need + // to generate completions to replace/update this token. We'll pay + // attention to the position to figure out the prefix that we should + // be completing. + else if cursor >= token.location().start.index && cursor <= token.location().end.index + { + // Update insertion index. + insertion_index = token.location().start.index; + + // Update prefix. + let offset_into_token = (cursor - insertion_index) as usize; + let token_str = token.to_str(); + completion_prefix = &token_str[..offset_into_token]; + + // Update token index. + completion_token_index = i; + + break; } - // Get the completions. - let mut result = Answer::RestartCompletionProcess; - let mut restart_count = 0; - while matches!(result, Answer::RestartCompletionProcess) { - if restart_count > MAX_RESTARTS { - tracing::error!("possible infinite loop detected in completion process"); - break; - } + // Otherwise, we need to keep looking. Update what we think the + // preceding token may be. + preceding_token = Some(token); + } - let completion_context = Context { - token_to_complete: completion_prefix, - preceding_token: preceding_token.map(|t| t.to_str()), - command_name: adjusted_tokens.first().map(|token| token.to_str()), - input_line: input, - token_index: completion_token_index, - tokens: adjusted_tokens.as_slice(), - cursor_index: position, - }; - - result = self - .get_completions_for_token(shell, completion_context) - .await; - - restart_count += 1; - } + // If the position is after the last token, then we need to insert an empty + // token for the new token to be generated. + let empty_token = + brush_parser::Token::Word(String::new(), brush_parser::TokenLocation::default()); + if completion_token_index == tokens.len() { + adjusted_tokens.push(&empty_token); + } - match result { - Answer::Candidates(candidates, options) => Ok(Completions { - insertion_index: insertion_index as usize, - delete_count: completion_prefix.len(), - candidates, - options, - }), - Answer::RestartCompletionProcess => Ok(Completions { - insertion_index: insertion_index as usize, - delete_count: 0, - candidates: IndexSet::new(), - options: ProcessingOptions::default(), - }), + // Get the completions. + let mut result = Answer::RestartCompletionProcess; + let mut restart_count = 0; + while matches!(result, Answer::RestartCompletionProcess) { + if restart_count > MAX_RESTARTS { + tracing::error!("possible infinite loop detected in completion process"); + break; } + + let completion_context = Context { + token_to_complete: completion_prefix, + preceding_token: preceding_token.map(|t| t.to_str()), + command_name: adjusted_tokens.first().map(|token| token.to_str()), + input_line: input, + token_index: completion_token_index, + tokens: adjusted_tokens.as_slice(), + cursor_index: position, + }; + + result = self + .get_completions_for_token(shell, completion_context) + .await; + + restart_count += 1; + } + + match result { + Answer::Candidates(candidates, options) => Ok(Completions { + insertion_index: insertion_index as usize, + delete_count: completion_prefix.len(), + candidates, + options, + }), + Answer::RestartCompletionProcess => Ok(Completions { + insertion_index: insertion_index as usize, + delete_count: 0, + candidates: IndexSet::new(), + options: ProcessingOptions::default(), + }), } - // else { - // tracing::debug!(target: trace_categories::COMPLETION, "failed to tokenize input line"); - - // Ok(Completions { - // insertion_index: position, - // delete_count: 0, - // candidates: IndexSet::new(), - // options: ProcessingOptions::default(), - // }) - // } } fn tokenize_input_for_completion(_shell: &mut Shell, input: &str) -> Vec { @@ -840,16 +845,8 @@ impl Config { async fn get_completions_for_token<'a>( &self, shell: &mut Shell, - mut context: Context<'a>, + context: Context<'a>, ) -> Answer { - // N.B. We basic-expand the token-to-be-completed first. - let mut throwaway_shell = shell.clone(); - let expanded_token_to_complete = throwaway_shell - .basic_expand_string(context.token_to_complete) - .await - .unwrap_or_else(|_| context.token_to_complete.to_owned()); - context.token_to_complete = expanded_token_to_complete.as_str(); - // See if we can find a completion spec matching the current command. let mut found_spec: Option<&Spec> = None; diff --git a/brush-core/src/escape.rs b/brush-core/src/escape.rs index 7989ce0c..bfd68a14 100644 --- a/brush-core/src/escape.rs +++ b/brush-core/src/escape.rs @@ -3,7 +3,7 @@ use itertools::Itertools; use crate::error; #[derive(Clone, Copy)] -pub(crate) enum EscapeMode { +pub(crate) enum EscapeExpansionMode { EchoBuiltin, AnsiCQuotes, } @@ -11,7 +11,7 @@ pub(crate) enum EscapeMode { #[allow(clippy::too_many_lines)] pub(crate) fn expand_backslash_escapes( s: &str, - mode: EscapeMode, + mode: EscapeExpansionMode, ) -> Result<(Vec, bool), crate::error::Error> { let mut result: Vec = vec![]; let mut it = s.chars(); @@ -27,11 +27,11 @@ pub(crate) fn expand_backslash_escapes( Some('b') => result.push(b'\x08'), Some('c') => { match mode { - EscapeMode::EchoBuiltin => { + EscapeExpansionMode::EchoBuiltin => { // Stop all additional output! return Ok((result, false)); } - EscapeMode::AnsiCQuotes => { + EscapeExpansionMode::AnsiCQuotes => { if let Some(_next_next) = it.next() { return error::unimp("control character in ANSI C quotes"); } else { @@ -48,9 +48,9 @@ pub(crate) fn expand_backslash_escapes( Some('t') => result.push(b'\t'), Some('v') => result.push(b'\x0b'), Some('\\') => result.push(b'\\'), - Some('\'') if matches!(mode, EscapeMode::AnsiCQuotes) => result.push(b'\''), - Some('\"') if matches!(mode, EscapeMode::AnsiCQuotes) => result.push(b'\"'), - Some('?') if matches!(mode, EscapeMode::AnsiCQuotes) => result.push(b'?'), + Some('\'') if matches!(mode, EscapeExpansionMode::AnsiCQuotes) => result.push(b'\''), + Some('\"') if matches!(mode, EscapeExpansionMode::AnsiCQuotes) => result.push(b'\"'), + Some('?') if matches!(mode, EscapeExpansionMode::AnsiCQuotes) => result.push(b'?'), Some('0') => { // Consume 0-3 valid octal chars let mut taken_so_far = 0; @@ -165,14 +165,85 @@ pub(crate) fn expand_backslash_escapes( Ok((result, true)) } +#[derive(Clone, Copy)] +pub(crate) enum QuoteMode { + BackslashEscape, + Quote, +} + +pub(crate) fn quote_if_needed(s: &str, mode: QuoteMode) -> String { + match mode { + QuoteMode::BackslashEscape => escape_with_backslash(s), + QuoteMode::Quote => escape_with_quoting(s), + } +} + +fn escape_with_backslash(s: &str) -> String { + let mut output = String::new(); + + // TODO: Handle other interesting sequences. + for c in s.chars() { + match c { + c if needs_escaping(c) => { + output.push('\\'); + output.push(c); + } + c => output.push(c), + } + } + + output +} + +fn escape_with_quoting(s: &str) -> String { + // TODO: Handle single-quote! + if s.chars().any(needs_escaping) { + std::format!("'{s}'") + } else { + s.to_owned() + } +} + +fn needs_escaping(c: char) -> bool { + matches!( + c, + '(' | ')' + | '[' + | ']' + | '{' + | '}' + | '$' + | '*' + | '?' + | '|' + | '&' + | ';' + | '<' + | '>' + | '`' + | '\\' + | '"' + | '!' + | '^' + | ',' + | ' ' + ) +} + #[cfg(test)] mod tests { use super::*; + #[test] + fn test_escape() { + assert_eq!(quote_if_needed("a", QuoteMode::BackslashEscape), "a"); + assert_eq!(quote_if_needed("a b", QuoteMode::BackslashEscape), r"a\ b"); + } + fn assert_echo_expands_to(unexpanded: &str, expected: &str) { assert_eq!( String::from_utf8( - expand_backslash_escapes(unexpanded, EscapeMode::EchoBuiltin) + expand_backslash_escapes(unexpanded, EscapeExpansionMode::EchoBuiltin) .unwrap() .0 ) diff --git a/brush-core/src/expansion.rs b/brush-core/src/expansion.rs index 8aa42b1c..aba68ba2 100644 --- a/brush-core/src/expansion.rs +++ b/brush-core/src/expansion.rs @@ -15,6 +15,7 @@ use crate::patterns; use crate::prompt; use crate::shell::Shell; use crate::sys; +use crate::trace_categories; use crate::variables::ShellValueUnsetType; use crate::variables::ShellVariable; use crate::variables::{self, ShellValue}; @@ -406,7 +407,7 @@ impl<'a> WordExpander<'a> { /// Apply tilde-expansion, parameter expansion, command substitution, and arithmetic expansion; /// yield pieces that could be further processed. async fn basic_expand(&mut self, word: &str) -> Result { - tracing::debug!("Basic expanding: '{word}'"); + tracing::debug!(target: trace_categories::EXPANSION, "Basic expanding: '{word}'"); // // TODO: Brace expansion in unquoted pieces @@ -512,6 +513,7 @@ impl<'a> WordExpander<'a> { } #[async_recursion::async_recursion] + #[allow(clippy::too_many_lines)] async fn expand_word_piece( &mut self, word_piece: brush_parser::word::WordPiece, @@ -524,8 +526,10 @@ impl<'a> WordExpander<'a> { Expansion::from(ExpansionPiece::Unsplittable(s)) } brush_parser::word::WordPiece::AnsiCQuotedText(s) => { - let (expanded, _) = - escape::expand_backslash_escapes(s.as_str(), escape::EscapeMode::AnsiCQuotes)?; + let (expanded, _) = escape::expand_backslash_escapes( + s.as_str(), + escape::EscapeExpansionMode::AnsiCQuotes, + )?; Expansion::from(ExpansionPiece::Unsplittable( String::from_utf8_lossy(expanded.as_slice()).into_owned(), )) @@ -619,14 +623,16 @@ impl<'a> WordExpander<'a> { params.process_group_policy = ProcessGroupPolicy::SameProcessGroup; // Run the command. - // TODO: inspect result? - let _cmd_result = subshell.run_string(s, ¶ms).await?; + let result = subshell.run_string(s, ¶ms).await?; // Make sure the subshell and params are closed; among other things, this // ensures they're not holding onto the write end of the pipe. drop(subshell); drop(params); + // Store the status. + self.shell.last_exit_status = result.exit_code; + // Extract output. let output_str = std::io::read_to_string(reader)?; @@ -1478,7 +1484,7 @@ impl<'a> WordExpander<'a> { } brush_parser::word::ParameterTransformOp::ExpandEscapeSequences => { let (result, _) = - escape::expand_backslash_escapes(s, escape::EscapeMode::AnsiCQuotes)?; + escape::expand_backslash_escapes(s, escape::EscapeExpansionMode::AnsiCQuotes)?; Ok(String::from_utf8_lossy(result.as_slice()).into_owned()) } brush_parser::word::ParameterTransformOp::PossiblyQuoteWithArraysExpanded { diff --git a/brush-core/src/interp.rs b/brush-core/src/interp.rs index b790b435..1b986b0b 100644 --- a/brush-core/src/interp.rs +++ b/brush-core/src/interp.rs @@ -914,7 +914,7 @@ impl ExecuteInPipeline for ast::SimpleCommand { if context.shell.options.print_commands_and_arguments { context .shell - .trace_command(args.iter().map(|arg| arg.to_string()).join(" "))?; + .trace_command(args.iter().map(|arg| arg.quote_for_tracing()).join(" "))?; } // TODO: This is adding more complexity here; should be factored out into an appropriate @@ -978,6 +978,9 @@ impl ExecuteInPipeline for ast::SimpleCommand { execution_result } else { + // Reset last status. + context.shell.last_exit_status = 0; + // No command to run; assignments must be applied to this shell. for assignment in assignments { apply_assignment( @@ -990,7 +993,11 @@ impl ExecuteInPipeline for ast::SimpleCommand { .await?; } - Ok(CommandSpawnResult::ImmediateExit(0)) + // Return the last exit status we have; in some cases, an expansion + // might result in a non-zero exit status stored in the shell. + Ok(CommandSpawnResult::ImmediateExit( + context.shell.last_exit_status, + )) } } } @@ -1121,7 +1128,7 @@ async fn apply_assignment( if shell.options.print_commands_and_arguments { let op = if assignment.append { "+=" } else { "=" }; - shell.trace_command(std::format!("{}{}{}", assignment.name, op, new_value))?; + shell.trace_command(std::format!("{}{op}{new_value}", assignment.name))?; } // See if we need to eval an array index. diff --git a/brush-core/src/patterns.rs b/brush-core/src/patterns.rs index c1c6340f..a4e82398 100644 --- a/brush-core/src/patterns.rs +++ b/brush-core/src/patterns.rs @@ -1,4 +1,4 @@ -use crate::{error, regex}; +use crate::{error, regex, trace_categories}; use std::{ collections::VecDeque, path::{Path, PathBuf}, @@ -110,7 +110,7 @@ impl Pattern { return Ok(vec![concatenated]); } - tracing::debug!("expanding pattern: {self:?}"); + tracing::debug!(target: trace_categories::PATTERN, "expanding pattern: {self:?}"); let mut components: Vec = vec![]; for piece in &self.pieces { @@ -221,7 +221,7 @@ impl Pattern { }) .collect(); - tracing::debug!(" => results: {results:?}"); + tracing::debug!(target: trace_categories::PATTERN, " => results: {results:?}"); Ok(results) } @@ -308,7 +308,7 @@ impl Pattern { enable_extended_globbing, )?; - tracing::debug!("pattern: '{self:?}' => regex: '{regex_str}'"); + tracing::debug!(target: trace_categories::PATTERN, "pattern: '{self:?}' => regex: '{regex_str}'"); let re = regex::compile_regex(regex_str)?; Ok(re) diff --git a/brush-core/src/trace_categories.rs b/brush-core/src/trace_categories.rs index a609391f..9b579e28 100644 --- a/brush-core/src/trace_categories.rs +++ b/brush-core/src/trace_categories.rs @@ -1,4 +1,6 @@ pub(crate) const COMMANDS: &str = "commands"; pub(crate) const COMPLETION: &str = "completion"; +pub(crate) const EXPANSION: &str = "expansion"; pub(crate) const JOBS: &str = "jobs"; pub(crate) const PARSE: &str = "parse"; +pub(crate) const PATTERN: &str = "pattern"; diff --git a/brush-parser/src/arithmetic.rs b/brush-parser/src/arithmetic.rs index 852e6ece..7cec87dc 100644 --- a/brush-parser/src/arithmetic.rs +++ b/brush-parser/src/arithmetic.rs @@ -9,7 +9,7 @@ use crate::error; /// /// * `input` - The arithmetic expression to parse, in string form. pub fn parse(input: &str) -> Result { - tracing::debug!("parsing arithmetic expression: '{input}'"); + tracing::debug!(target: "arithmetic", "parsing arithmetic expression: '{input}'"); // Special-case the empty string. diff --git a/brush-parser/src/parser.rs b/brush-parser/src/parser.rs index 3d4a3a70..59c854e7 100644 --- a/brush-parser/src/parser.rs +++ b/brush-parser/src/parser.rs @@ -132,7 +132,7 @@ pub fn parse_tokens( Ok(program) } Err(parse_error) => { - tracing::debug!("Parse error: {:?}", parse_error); + tracing::debug!(target: "parse", "Parse error: {:?}", parse_error); Err(error::convert_peg_parse_error( parse_error, tokens.as_slice(), diff --git a/brush-parser/src/word.rs b/brush-parser/src/word.rs index 929482d8..bb965db1 100644 --- a/brush-parser/src/word.rs +++ b/brush-parser/src/word.rs @@ -381,12 +381,12 @@ fn cacheable_parse( word: String, options: ParserOptions, ) -> Result, error::WordParseError> { - tracing::debug!("Parsing word '{}'", word); + tracing::debug!(target: "expansion", "Parsing word '{}'", word); let pieces = expansion_parser::unexpanded_word(word.as_str(), &options) .map_err(|err| error::WordParseError::Word(word.to_owned(), err))?; - tracing::debug!(target: "parse", "Parsed word '{}' => {{{:?}}}", word, pieces); + tracing::debug!(target: "expansion", "Parsed word '{}' => {{{:?}}}", word, pieces); Ok(pieces) } diff --git a/brush-shell/src/events.rs b/brush-shell/src/events.rs index bb1f2821..e4e6ae2a 100644 --- a/brush-shell/src/events.rs +++ b/brush-shell/src/events.rs @@ -96,17 +96,13 @@ impl TraceEventConfig { for event in &self.enabled_trace_events { let targets = match event { - TraceEvent::Arithmetic => vec!["parser::arithmetic"], + TraceEvent::Arithmetic => vec!["arithmetic"], TraceEvent::Commands => vec!["commands"], - TraceEvent::Complete => vec![ - "completion", - "shell::completion", - "shell::builtins::complete", - ], - TraceEvent::Expand => vec!["parser::word", "shell::expansion"], + TraceEvent::Complete => vec!["completion"], + TraceEvent::Expand => vec!["expansion"], TraceEvent::Jobs => vec!["jobs"], TraceEvent::Parse => vec!["parse"], - TraceEvent::Pattern => vec!["shell::pattern"], + TraceEvent::Pattern => vec!["pattern"], TraceEvent::Tokenize => vec!["tokenize"], }; diff --git a/brush-shell/tests/cases/builtins/compgen.yaml b/brush-shell/tests/cases/builtins/compgen.yaml index 83eba774..b224f056 100644 --- a/brush-shell/tests/cases/builtins/compgen.yaml +++ b/brush-shell/tests/cases/builtins/compgen.yaml @@ -60,3 +60,44 @@ cases: - name: "compgen -W with options" stdin: | compgen -W '--abc --def' -- '--ab' + + - name: "compgen with no matches" + stdin: | + compgen -W yes no && echo "1. Result" + + - name: "compgen -f with tilde" + stdin: | + touch item1 + HOME=$(pwd) + + echo "[0]" + for p in $(compgen -f ~); do + echo ${p//$HOME/HOME} + done + + echo "[1]" + for p in $(compgen -f ~/); do + echo ${p//$HOME/HOME} + done + + - name: "compgen -f with quoted tilde" + known_failure: true + stdin: | + touch item1 + HOME=$(pwd) + + echo "[0]" + for p in $(compgen -f '~/'); do + echo ${p//$HOME/HOME} + done + + - name: "compgen -f with quoted var" + known_failure: true + stdin: | + touch item1 + HOME=$(pwd) + + echo "[0]" + for p in $(compgen -f '$HOME/'); do + echo ${p//$HOME/HOME} + done diff --git a/brush-shell/tests/cases/builtins/printf.yaml b/brush-shell/tests/cases/builtins/printf.yaml index eab820d1..5bf69df4 100644 --- a/brush-shell/tests/cases/builtins/printf.yaml +++ b/brush-shell/tests/cases/builtins/printf.yaml @@ -22,3 +22,14 @@ cases: - name: "printf with -v as a format arg" stdin: | printf "%s\n" "-v" + + - name: "printf %q" + stdin: | + echo "[1]" + printf "%q" 'TEXT'; echo + + echo "[2]" + printf "%q" '$VAR'; echo + + echo "[3]" + printf "%q" '"'; echo diff --git a/brush-shell/tests/cases/extended_tests.yaml b/brush-shell/tests/cases/extended_tests.yaml index 00c46964..c8e84edf 100644 --- a/brush-shell/tests/cases/extended_tests.yaml +++ b/brush-shell/tests/cases/extended_tests.yaml @@ -135,6 +135,12 @@ cases: [[ "abc" == "a*" ]] && echo "1. Matches" [[ "abc" != "a*" ]] && echo "2. Matches" + - name: "Tilde binary string matching" + known_failure: true + stdin: | + x='~/' + [[ $x == ~* ]] && echo "1. Matches" + - name: "Arithmetic extended tests" stdin: | [[ 0 -eq 0 ]] && echo "1. Pass" diff --git a/brush-shell/tests/cases/word_expansion.yaml b/brush-shell/tests/cases/word_expansion.yaml index 2eadf6b6..6236cca8 100644 --- a/brush-shell/tests/cases/word_expansion.yaml +++ b/brush-shell/tests/cases/word_expansion.yaml @@ -61,6 +61,11 @@ cases: x=$(echo foo | (wc -l; echo hi)) echo "\$x: $x" + - name: "Command substitution exit code" + stdin: | + x=$(false) && echo "1. Made it past false" + y=$(true) && echo "2. Made it past true" + - name: "Backtick command substitution" stdin: | echo `echo hi` diff --git a/brush-shell/tests/compat_tests.rs b/brush-shell/tests/compat_tests.rs index f0e21682..d1d4c040 100644 --- a/brush-shell/tests/compat_tests.rs +++ b/brush-shell/tests/compat_tests.rs @@ -419,7 +419,7 @@ impl TestCaseSet { TestCaseResult { success: true, comparison: RunComparison::ignored(), - name: self.name.clone(), + name: test_case.name.clone(), skip: true, known_failure: test_case.known_failure, } @@ -495,6 +495,10 @@ impl TestCaseResult { mut writer: W, options: &TestOptions, ) -> Result<()> { + if self.skip { + return Ok(()); + } + if !options.verbose { if (!self.comparison.is_failure() && !self.known_failure) || (self.comparison.is_failure() && self.known_failure) @@ -596,7 +600,19 @@ impl TestCaseResult { indent::indent_all_by(8, make_expectrl_output_readable(test_string)) )?; } - StringComparison::Same(_) => writeln!(writer, " stdout matches {}", "✔️".green())?, + StringComparison::Same(s) => { + writeln!(writer, " stdout matches {}", "✔️".green())?; + + if options.verbose { + writeln!( + writer, + " {}", + "------ Oracle <> Test: stdout ---------------------------------".cyan() + )?; + + writeln!(writer, "{}", indent::indent_all_by(8, s))?; + } + } StringComparison::TestDiffers { test_string: t, oracle_string: o, @@ -624,7 +640,19 @@ impl TestCaseResult { test_string: _, oracle_string: _, } => writeln!(writer, " stderr {}", "ignored".cyan())?, - StringComparison::Same(_) => writeln!(writer, " stderr matches {}", "✔️".green())?, + StringComparison::Same(s) => { + writeln!(writer, " stderr matches {}", "✔️".green())?; + + if options.verbose { + writeln!( + writer, + " {}", + "------ Oracle <> Test: stderr ---------------------------------".cyan() + )?; + + writeln!(writer, "{}", indent::indent_all_by(8, s))?; + } + } StringComparison::TestDiffers { test_string: t, oracle_string: o, diff --git a/brush-shell/tests/completion_tests.rs b/brush-shell/tests/completion_tests.rs index 4b06801c..a9b16a01 100644 --- a/brush-shell/tests/completion_tests.rs +++ b/brush-shell/tests/completion_tests.rs @@ -86,6 +86,147 @@ async fn complete_relative_dir_path() -> Result<()> { Ok(()) } +#[tokio::test] +async fn complete_under_empty_dir() -> Result<()> { + let mut test_shell = TestShellWithBashCompletion::new().await?; + + // Create file and dir. + test_shell.temp_dir.child("empty").create_dir_all()?; + + // Complete; expect to see nothing. + let input = "ls empty/"; + let results = test_shell.complete(input, input.len()).await?; + + assert_eq!(results, Vec::::new()); + + Ok(()) +} + +#[tokio::test] +async fn complete_nonexistent_relative_path() -> Result<()> { + let mut test_shell = TestShellWithBashCompletion::new().await?; + + // Complete; expect to see nothing. + let input = "ls item"; + let results = test_shell.complete(input, input.len()).await?; + + assert_eq!(results, Vec::::new()); + + Ok(()) +} + +#[tokio::test] +async fn complete_absolute_paths() -> Result<()> { + let mut test_shell = TestShellWithBashCompletion::new().await?; + + // Create file and dir. + test_shell.temp_dir.child("item1").touch()?; + test_shell.temp_dir.child("item2").create_dir_all()?; + + // Complete; expect to see just the dir. + let input = std::format!("ls {}", test_shell.temp_dir.path().join("item").display()); + let results = test_shell.complete(input.as_str(), input.len()).await?; + + assert_eq!( + results, + [ + test_shell + .temp_dir + .child("item1") + .path() + .display() + .to_string(), + test_shell + .temp_dir + .child("item2") + .path() + .display() + .to_string(), + ] + ); + + Ok(()) +} + +#[ignore] // TODO: Fix this for newer versions of bash-completion +#[tokio::test] +async fn complete_path_with_var() -> Result<()> { + let mut test_shell = TestShellWithBashCompletion::new().await?; + + // Create file and dir. + test_shell.temp_dir.child("item1").touch()?; + test_shell.temp_dir.child("item2").create_dir_all()?; + + // Complete; expect to see the two files. + let input = "ls $PWD/item"; + let results = test_shell.complete(input, input.len()).await?; + + assert_eq!( + results, + [ + test_shell + .temp_dir + .child("item1") + .path() + .display() + .to_string(), + test_shell + .temp_dir + .child("item2") + .path() + .display() + .to_string(), + ] + ); + + Ok(()) +} + +#[ignore] // TODO: Fix this for newer versions of bash-completion +#[tokio::test] +async fn complete_path_with_tilde() -> Result<()> { + let mut test_shell = TestShellWithBashCompletion::new().await?; + + // Set HOME to the temp dir so we can use ~ to reference it. + test_shell.set_var( + "HOME", + test_shell + .temp_dir + .path() + .to_string_lossy() + .to_string() + .as_str(), + )?; + + // Create file and dir. + test_shell.temp_dir.child("item1").touch()?; + test_shell.temp_dir.child("item2").create_dir_all()?; + + // Complete; expect to see the two files. + let input = "ls ~/item"; + let results = test_shell.complete(input, input.len()).await?; + + assert_eq!( + results, + [ + test_shell + .temp_dir + .child("item1") + .path() + .display() + .to_string(), + test_shell + .temp_dir + .child("item2") + .path() + .display() + .to_string(), + ] + ); + + Ok(()) +} + #[tokio::test] async fn complete_variable_names() -> Result<()> { let mut test_shell = TestShellWithBashCompletion::new().await?;