From 4a71ee12883a720408419a53482818177a834ec7 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Fri, 22 Nov 2024 00:00:42 +0100 Subject: [PATCH] Allow special variables $?,$$,$@,$# --- src/common.rs | 14 +++++++++--- src/expand.rs | 42 ++++++++++++++++++++++++++++++++--- src/parse_util.rs | 19 +++++++++++++++- src/tests/parse_util.rs | 7 +----- src/tests/parser.rs | 9 +------- tests/checks/commandline.fish | 1 - tests/checks/invocation.fish | 6 ++--- 7 files changed, 73 insertions(+), 25 deletions(-) diff --git a/src/common.rs b/src/common.rs index d4a8a32da..cb1764cc6 100644 --- a/src/common.rs +++ b/src/common.rs @@ -584,9 +584,17 @@ enum Mode { } '$' => { if unescape_special { - let is_cmdsub = input_position + 1 < input.len() - && input.char_at(input_position + 1) == '('; - if !is_cmdsub { + let next = input.char_at(input_position + 1); + let is_cmdsub = input_position + 1 < input.len() && next == '('; + let is_pid = !valid_var_name_char(next) + && result + .as_char_slice() + .last() + .map(|prev| { + [VARIABLE_EXPAND, VARIABLE_EXPAND_SINGLE].contains(prev) + }) + .unwrap_or_default(); + if !is_cmdsub && !is_pid { to_append_or_none = Some(VARIABLE_EXPAND); vars_or_seps.push(input_position); } diff --git a/src/expand.rs b/src/expand.rs index 72f0e381e..c7c7a0bda 100644 --- a/src/expand.rs +++ b/src/expand.rs @@ -13,7 +13,7 @@ UnescapeFlags, UnescapeStringStyle, EXPAND_RESERVED_BASE, EXPAND_RESERVED_END, }; use crate::complete::{CompleteFlags, Completion, CompletionList, CompletionReceiver}; -use crate::env::{EnvVar, Environment}; +use crate::env::{EnvVar, EnvVarFlags, Environment}; use crate::exec::exec_subshell_for_expand; use crate::future_feature_flags::{feature_test, FeatureFlag}; use crate::history::{history_session_id, History}; @@ -622,7 +622,33 @@ fn expand_variables( ); // Get the variable name as a string, then try to get the variable from env. - let var_name = &instr[var_name_start..var_name_stop]; + let mut var_name = None; + let mut may_slice = true; + if var_name_stop == var_name_start { + match instr.char_at(var_name_stop) { + '?' => { + var_name_stop += 1; + may_slice = false; + var_name = Some(L!("status")); + } + '$' | VARIABLE_EXPAND | VARIABLE_EXPAND_SINGLE => { + var_name_stop += 1; + may_slice = false; + var_name = Some(L!("fish_pid")); + } + '#' => { + var_name_stop += 1; + may_slice = false; + } + '@' => { + var_name_stop += 1; + may_slice = false; + var_name = Some(L!("argv")); + } + _ => (), + } + } + let var_name = var_name.unwrap_or(&instr[var_name_start..var_name_stop]); // It's an error if the name is empty. if var_name.is_empty() { @@ -644,6 +670,16 @@ fn expand_variables( let mut var = None; if var_name == "history" { history = Some(History::with_name(&history_session_id(vars))); + } else if var_name == "#" { + var = Some(EnvVar::new( + sprintf!( + "%lu", + vars.get(L!("argv")) + .map(|v| v.as_list().len()) + .unwrap_or_default() + ), + EnvVarFlags::default(), + )); } else if var_name.as_char_slice() != [VARIABLE_EXPAND_EMPTY] { var = vars.get(var_name); } @@ -655,7 +691,7 @@ fn expand_variables( let slice_start = var_name_stop; let mut var_idx_list = vec![]; - if instr.as_char_slice().get(slice_start) == Some(&'[') { + if may_slice && instr.as_char_slice().get(slice_start) == Some(&'[') { all_values = false; // If a variable is missing, behave as though we have one value, so that $var[1] always // works. diff --git a/src/parse_util.rs b/src/parse_util.rs index deb0b7ddd..2b7184d09 100644 --- a/src/parse_util.rs +++ b/src/parse_util.rs @@ -664,6 +664,7 @@ fn get_first_arg(list: &ast::ArgumentOrRedirectionList) -> Option<&ast::Argument /// Given a wide character immediately after a dollar sign, return the appropriate error message. /// For example, if wc is @, then the variable name was $@ and we suggest $argv. fn error_for_character(c: char) -> WString { + // TODO match c { '?' => wgettext!(ERROR_NOT_STATUS).to_owned(), '#' => wgettext!(ERROR_NOT_ARGV_COUNT).to_owned(), @@ -1401,7 +1402,21 @@ pub fn parse_util_detect_errors_in_argument( continue; } let next_char = unesc.get(idx + 1).copied().unwrap_or('\0'); - if ![VARIABLE_EXPAND, VARIABLE_EXPAND_SINGLE, '('].contains(&next_char) + let prev_char = idx + .checked_sub(1) + .and_then(|i| unesc.get(i).copied()) + .unwrap_or('\0'); + if (![ + VARIABLE_EXPAND, + VARIABLE_EXPAND_SINGLE, + '$', + '(', + '?', + '#', + '@', + ] + .contains(&next_char) + && ![VARIABLE_EXPAND, VARIABLE_EXPAND_SINGLE, '$'].contains(&prev_char)) && !valid_var_name_char(next_char) { err = ParserTestErrorBits::ERROR; @@ -1893,6 +1908,8 @@ pub fn parse_util_expand_variable_error( '\0' => { append_syntax_error!(errors, global_dollar_pos, 1, ERROR_NO_VAR_NAME); } + // TODO + '?' | VARIABLE_EXPAND | VARIABLE_EXPAND_SINGLE | '$' | '#' | '@' => return, _ => { let mut token_stop_char = char_after_dollar; // Unescape (see issue #50). diff --git a/src/tests/parse_util.rs b/src/tests/parse_util.rs index b41a94649..6dedd4038 100644 --- a/src/tests/parse_util.rs +++ b/src/tests/parse_util.rs @@ -3,8 +3,7 @@ use crate::common::EscapeFlags; use crate::parse_constants::{ ERROR_BAD_VAR_CHAR1, ERROR_BRACKETED_VARIABLE1, ERROR_BRACKETED_VARIABLE_QUOTED1, - ERROR_NOT_ARGV_AT, ERROR_NOT_ARGV_COUNT, ERROR_NOT_ARGV_STAR, ERROR_NOT_PID, ERROR_NOT_STATUS, - ERROR_NO_VAR_NAME, + ERROR_NOT_ARGV_STAR, ERROR_NO_VAR_NAME, }; use crate::parse_util::{ parse_util_cmdsubst_extent, parse_util_compute_indents, parse_util_detect_errors, @@ -75,10 +74,6 @@ macro_rules! validate { validate!("echo foo${a}bar", ERROR_BRACKETED_VARIABLE1); validate!("echo foo\"${a}\"bar", ERROR_BRACKETED_VARIABLE_QUOTED1); validate!("echo foo\"${\"bar", ERROR_BAD_VAR_CHAR1); - validate!("echo $?", ERROR_NOT_STATUS); - validate!("echo $$", ERROR_NOT_PID); - validate!("echo $#", ERROR_NOT_ARGV_COUNT); - validate!("echo $@", ERROR_NOT_ARGV_AT); validate!("echo $*", ERROR_NOT_ARGV_STAR); validate!("echo $", ERROR_NO_VAR_NAME); validate!("echo foo\"$\"bar", ERROR_NO_VAR_NAME); diff --git a/src/tests/parser.rs b/src/tests/parser.rs index 0b89806f6..5805c73bf 100644 --- a/src/tests/parser.rs +++ b/src/tests/parser.rs @@ -193,14 +193,7 @@ fn detect_argument_errors(src: &str) -> Result<(), ParserTestErrorBits> { ); assert!( - detect_argument_errors("foo$$") - .unwrap_err() - .contains(ParserTestErrorBits::ERROR), - "Bad variable expansion not reported as error" - ); - - assert!( - detect_argument_errors("foo$@") + detect_argument_errors("foo$%") .unwrap_err() .contains(ParserTestErrorBits::ERROR), "Bad variable expansion not reported as error" diff --git a/tests/checks/commandline.fish b/tests/checks/commandline.fish index 6562d5873..642ce9226 100644 --- a/tests/checks/commandline.fish +++ b/tests/checks/commandline.fish @@ -16,7 +16,6 @@ or echo Invalid $status commandline --input 'echo $$' --is-valid or echo Invalid $status -# CHECK: Invalid 1 commandline --help &>/dev/null echo Help $status diff --git a/tests/checks/invocation.fish b/tests/checks/invocation.fish index 623bdf6ac..57c1172d6 100644 --- a/tests/checks/invocation.fish +++ b/tests/checks/invocation.fish @@ -91,10 +91,10 @@ $fish --no-config -c 'echo notprinted; echo foo | exec true; echo banana' # CHECKERR: ^~~~~~~~^ # Running multiple command lists continues even if one has a syntax error. -$fish --no-config -c 'echo $$ oh no syntax error' -c 'echo this works' +$fish --no-config -c 'echo $% oh no syntax error' -c 'echo this works' # CHECK: this works -# CHECKERR: fish: $$ is not the pid. In fish, please use $fish_pid. -# CHECKERR: echo $$ oh no syntax error +# CHECKERR: fish: $% is not a valid variable in fish. +# CHECKERR: echo $% oh no syntax error # CHECKERR: ^ $fish --no-config .