diff --git a/README.md b/README.md index 1c22b1c2..0ffa9053 100644 --- a/README.md +++ b/README.md @@ -54,17 +54,17 @@ installed on your system, then you can also author a `~/.brushrc` file. There are some known gaps in compatibility. Most notably: -* **Honoring the full semantics of all `set` and `shopt` options.** - The `set` builtin is implemented, as is `set -x` and many frequently used options, but a number of options aren't fully implemented. `set -e`, for example, will execute but its semantics aren't applied across execution. +* **Some `set` and `shopt` options.** + The `set` builtin is implemented, as is `set -x` and many frequently used `set`/`shopt` options, but a number aren't fully implemented. For example, `set -e` will execute but its semantics aren't applied across execution. * **Anything tagged with a `TODO` comment or where `error::unimp()` is used to return a "not implemented" error**. These aren't all tracked with GitHub issues right now, but there's a number of these scattered throughout the code base. Some are indicative of missing functionality that may be straightforward to implement; others may be more complicated. -If you feel so inclined, we'd love contributions toward any of the above, with broadening test coverage, deeper compatibility evaluation, or really any other opportunities you can find to help make this project better. +If you feel so inclined, we'd love contributions toward any of the above, with broadening test coverage, deeper compatibility evaluation, or really any other opportunities you can find to help us make this project better. ## Testing strategy -This project is primarily tested by comparing its behavior with other existing shells, leveraging the latter as test oracles. The integration tests implemented in this repo include [515+ test cases](brush-shell/tests/cases) run on both this shell and an oracle, comparing standard output and exit codes. +This project is primarily tested by comparing its behavior with other existing shells, leveraging the latter as test oracles. The integration tests implemented in this repo include [525+ test cases](brush-shell/tests/cases) run on both this shell and an oracle, comparing standard output and exit codes. For more details, please consult the [reference documentation on integration testing](docs/reference/integration-testing.md). diff --git a/brush-core/src/arithmetic.rs b/brush-core/src/arithmetic.rs index c13257a4..6d041ccb 100644 --- a/brush-core/src/arithmetic.rs +++ b/brush-core/src/arithmetic.rs @@ -19,8 +19,8 @@ pub enum EvalError { FailedToTokenizeExpression, /// Failed to expand an arithmetic expression. - #[error("failed to expand expression")] - FailedToExpandExpression, + #[error("failed to expand expression: '{0}'")] + FailedToExpandExpression(String), /// Failed to access an element of an array. #[error("failed to access array")] @@ -71,7 +71,7 @@ pub(crate) async fn expand_and_eval( // Per documentation, first shell-expand it. let expanded_self = expansion::basic_expand_str_without_tilde(shell, expr) .await - .map_err(|_e| EvalError::FailedToExpandExpression)?; + .map_err(|_e| EvalError::FailedToExpandExpression(expr.to_owned()))?; // Now parse. let expr = brush_parser::arithmetic::parse(&expanded_self) diff --git a/brush-core/src/builtins.rs b/brush-core/src/builtins.rs index a5c44337..fcec8472 100644 --- a/brush-core/src/builtins.rs +++ b/brush-core/src/builtins.rs @@ -191,15 +191,18 @@ pub trait Command: Parser { } else { // N.B. clap doesn't support named options like '+x'. To work around this, we // establish a pattern of renaming them. - let args = args.into_iter().map(|arg| { - if arg.starts_with('+') { - format!("--{arg}") + let mut updated_args = vec![]; + for arg in args { + if let Some(plus_options) = arg.strip_prefix("+") { + for c in plus_options.chars() { + updated_args.push(format!("--+{c}")); + } } else { - arg + updated_args.push(arg); } - }); + } - Self::try_parse_from(args) + Self::try_parse_from(updated_args) } } diff --git a/brush-core/src/builtins/set.rs b/brush-core/src/builtins/set.rs index 5aa76cee..f487e278 100644 --- a/brush-core/src/builtins/set.rs +++ b/brush-core/src/builtins/set.rs @@ -150,17 +150,35 @@ impl builtins::Command for SetCommand { // leaking into too many commands' custom parsing. // - // Apply the same workaround from the default implementation of Command::new to handle + + // Apply the same workaround from the default implementation of Command::new to handle '+' // args. - let args = args.into_iter().map(|arg| { - if arg.starts_with('+') { - format!("--{arg}") + let mut updated_args = vec![]; + let mut now_parsing_positional_args = false; + let mut next_arg_is_option_value = false; + for (i, arg) in args.into_iter().enumerate() { + if now_parsing_positional_args || next_arg_is_option_value { + updated_args.push(arg); + + next_arg_is_option_value = false; + continue; + } + + if arg == "-" || arg == "--" || (i > 0 && !arg.starts_with(['-', '+'])) { + now_parsing_positional_args = true; + } + + if let Some(plus_options) = arg.strip_prefix("+") { + next_arg_is_option_value = plus_options.ends_with('o'); + for c in plus_options.chars() { + updated_args.push(format!("--+{c}")); + } } else { - arg + next_arg_is_option_value = arg.starts_with('-') && arg.ends_with('o'); + updated_args.push(arg); } - }); + } - let (mut this, rest_args) = crate::builtins::try_parse_known::(args)?; + let (mut this, rest_args) = crate::builtins::try_parse_known::(updated_args)?; if let Some(args) = rest_args { this.positional_args.extend(args); } diff --git a/brush-core/src/builtins/unset.rs b/brush-core/src/builtins/unset.rs index 4866404a..52ee85aa 100644 --- a/brush-core/src/builtins/unset.rs +++ b/brush-core/src/builtins/unset.rs @@ -48,6 +48,7 @@ impl builtins::Command for UnsetCommand { let unspecified = self.name_interpretation.unspecified(); + #[allow(clippy::needless_continue)] for name in &self.names { if unspecified || self.name_interpretation.shell_variables { let parameter = diff --git a/brush-core/src/jobs.rs b/brush-core/src/jobs.rs index b997bcd5..da91b469 100644 --- a/brush-core/src/jobs.rs +++ b/brush-core/src/jobs.rs @@ -427,7 +427,7 @@ impl Job { return Some(pid); } } - JobTask::Internal(_) => continue, + JobTask::Internal(_) => (), } } None diff --git a/brush-shell/tests/cases/builtins/set.yaml b/brush-shell/tests/cases/builtins/set.yaml index 9668b6f2..2a6d38bd 100644 --- a/brush-shell/tests/cases/builtins/set.yaml +++ b/brush-shell/tests/cases/builtins/set.yaml @@ -14,6 +14,40 @@ cases: set a b c d echo ${*} + - name: "set with options" + stdin: | + function dumpopts { + # Dump the options + echo "[Options: $1]" + echo "set options: " $- + shopt -p -o pipefail + } + + set -e -u -o pipefail + dumpopts enabled + echo '*: ' $* + + set +e +u +o pipefail + dumpopts disabled + echo '*: ' $* + + - name: "set with multiple combined options" + stdin: | + function dumpopts { + # Dump the options + echo "[Options: $1]" + echo "set options: " $- + shopt -p -o pipefail + } + + set -euo pipefail + dumpopts enabled + echo '$*: ' $* + + set +euo pipefail + dumpopts disabled + echo '$*: ' $* + - name: "set clearing args" stdin: | set a b c @@ -35,7 +69,22 @@ cases: set -- echo "args: " ${*} - - name: "set with option-looking tokens" + - name: "set with option-looking args" stdin: | + set -- a -v + echo ${*} + + set - a -v + echo ${*} + set a -v echo ${*} + + set -- a +x + echo ${*} + + set - a +x + echo ${*} + + set a +x + echo ${*}