Docs updates plus a few test fixes (#43)

.devcontainer/Dockerfile

@@ -5,10 +5,13 @@ ARG USER_UID=1000
 ARG USER_GID=$USER_UID
 
 # Install basic utilities and prerequisites.
+#   bash-completion - for completion testing
 #   bsdmainutils - provides hexdump, used by integration tests
 #   neovim - for convenience and modern editing
 RUN apt-get update -y && \
     apt-get install -y \
+        bash \
+        bash-completion \
         bsdmainutils \
         build-essential \
         curl \

README.md

@@ -21,9 +21,9 @@ Available for use and distribution under the [MIT license](LICENSE).
 
 In short, quite a lot. Standard and extended control flow, word expansion, most frequently used builtin commands, pipelines, redirection, variables, etc. The plumbing for completion is present, along with support for common cases (e.g. file/dir completion, basic support for programmable completion such as used with git and other tools). 
 
-### Known limitations
+### --Known limitations-- a.k.a. Where you can help!
 
-There's a lot that *is* working, but also non-trivial gaps in compatibility. Most notably:
+There's a lot that *is* working, but there are non-trivial gaps in compatibility. Most notably:
 
 * **Commands run asynchronously as jobs, job management.**
   You can run `some-command &` but it's proof-of-concept quality at best. Standard job management via `fg`, `bg`, and `jobs` is not fully implemented. This would be a great area for enthusiastic contributors to dive in :).
@@ -34,12 +34,25 @@ Shell built-ins are a mixed bag. Some are completely and fully implemented (e.g.
 
 There's certainly more gaps; with time we'll find a way to represent the gaps in some understandable way. Ideally, we'd like to evolve the test suites to add tests for all known missing pieces. That will let us focus on just "fixing the tests". 
 
+We'd absolutely love your help with 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.
+
 ## 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 [300+ test cases](cli/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).
 
+## Credits
+
+There's a long list of OSS crates whose shoulders this project rests on. Notably, the following crates are directly relied on for major portions of shell functionality:
+
+* [`rustyline`](https://github.com/kkawakam/rustyline) - for readline input and interactive usage
+* [`clap`]() - command-line parsing, used both by the top-level brush CLI as well as built-in commands
+* [`fancy-regex`]() - relied on for everything regex
+* [`tokio`]() - async, well, everything
+
+Huge kudos and thanks also to `pprof` and `criterion` projects for enabling awesome flamegraphs in smooth integration with `cargo bench`'s standard benchmarking facilities.
+
 ## Links: other shell implementations
 
 This is certainly not the first attempt to implement a feature-rich POSIX-ish shell in a non-C/C++ implementation language. Some examples include:

cli/src/main.rs

@@ -224,7 +224,7 @@ async fn run(
 
         // Execute the command string.
         let params = shell.shell().default_exec_params();
-        shell.shell_mut().run_string(&command, &params).await?;
+        shell.shell_mut().run_string(command, &params).await?;
     } else if args.read_commands_from_stdin {
         shell.run_interactively().await?;
     } else if let Some(script_path) = args.script_path {

cli/tests/cases/arithmetic.yaml

@@ -24,7 +24,6 @@ cases:
       echo "$(((10)))"
 
   - name: "Unquoted parentheses"
-    known_failure: true
     stdin: |
       echo $(( (10) ))
       echo $(((10)))
@@ -121,7 +120,7 @@ cases:
       echo "x is now $x"
 
   - name: "Assignments in logical boolean expressions"
-    known_failure: true
+    known_failure: false
     stdin: |
       x=0
       echo "0 && x+=1 => $(( 0 && (x+=1) ))"

cli/tests/cases/builtins/alias.yaml

@@ -11,7 +11,7 @@ cases:
       myalias 'hello'
 
   - name: "Alias referencing to alias"
-    known_failure: true # Issue #149
+    known_failure: true
     args: ["-i"]
     env:
       PS1: "$ "

cli/tests/cases/builtins/shopt.yaml

@@ -23,14 +23,14 @@ cases:
       shopt -o | sort | grep -v monitor
 
   - name: "extglob defaults"
-    known_failure: true # Issue #169
+    known_failure: true
     stdin: |
       shopt extglob
 
   - name: "extglob interactive defaults"
     args: ["-i"]
     ignore_stderr: true
-    known_failure: true # Issue #169
+    known_failure: true
     stdin: |
       shopt extglob
 

cli/tests/cases/builtins/unset.yaml

@@ -48,7 +48,7 @@ cases:
       declare -p myarray
 
   - name: "Unset array element with interesting expression"
-    known_failure: true # Issues #170
+    known_failure: true
     stdin: |
       declare -a myarray=(a b c d e)
 

cli/tests/cases/compound_cmds/arithmetic.yaml

@@ -4,3 +4,8 @@ cases:
     stdin: |
       ((0 == 0)) && echo "0 == 0"
       ((0 != 0)) && echo "0 != 0"
+
+  - name: "Arithmetic statements with parens"
+    stdin: |
+      (( (0) )) && echo "0"
+      (( (1) )) && echo "1"

cli/tests/cases/options.yaml

@@ -5,7 +5,7 @@ cases:
       echo "Default options: $-"
 
   - name: "set -a"
-    known_failure: true # Issue #154
+    known_failure: true
     stdin: |
       unexported=original
       set -a

cli/tests/cases/patterns.yaml

@@ -38,13 +38,6 @@ cases:
       - path: "file2.txt"
     stdin: "echo file[12].txt"
 
-  - name: "Expansion with curly braces"
-    known_failure: true # Issue #152
-    test_files:
-      - path: "file1.txt"
-      - path: "file2.txt"
-    stdin: "echo file{1,2}.txt"
-
   - name: "Expansion with range"
     test_files:
       - path: "file1.txt"
@@ -156,7 +149,7 @@ cases:
       test_pattern "ad" "+(ab|ac)"
 
   - name: "Pathname expansion: extglob disabled"
-    known_failure: true # Issue #169
+    known_failure: true
     ignore_stderr: true
     test_files:
       - path: "ab.txt"

cli/tests/cases/redirection.yaml

@@ -42,7 +42,7 @@ cases:
       ls -d . non-existent-dir &>/dev/null
 
   - name: "Process substitution: input + output"
-    known_failure: true # Issue #151
+    known_failure: true
     stdin: |
       shopt -u -o posix
       cp <(echo hi) >(cat)

cli/tests/cases/word_expansion.yaml

@@ -337,7 +337,7 @@ cases:
       echo "  -> result: $?"
 
   - name: "Parameter expression: error on condition (non-interactive)"
-    known_failure: true # Issue #155
+    known_failure: true
     ignore_stderr: true
     stdin: |
       echo "${non_existent_var?error message}"
@@ -703,13 +703,13 @@ cases:
       echo "\${arr2@k}: ${arr2@k}"
 
   - name: "Parameter transformations: expand escapes"
-    known_failure: true # Issue #155
+    known_failure: true
     stdin: |
       var="a\n\"b"
       echo "\${var@E}: ${var@E}"
 
   - name: "Parameter transformation: assignment"
-    known_failure: true # Issue #155
+    known_failure: true
     stdin: |
       var="hello"
       echo "\${var@A}: ${var@A}"
@@ -718,10 +718,37 @@ cases:
       echo "\${arr@A}: ${arr@A}"
 
   - name: "Parameter transformation: attributes"
-    known_failure: true # Issue #155
+    known_failure: true
     stdin: |
       var="hello"
       echo "\${var@a}: ${var@a}"
 
       declare -ia arr=(1 2 3)
       echo "\${arr@a}: ${arr@a}"
+
+  - name: "Expansion with curly braces"
+    known_failure: true # Issue #42
+    stdin: |
+      echo "{a,b}:"
+      echo {a,b}
+
+      echo "{}:"
+      echo {}
+
+      echo "1{a,b}2"
+      echo 1{a,b}2
+
+      echo "{a,b}{1,2}"
+      echo {a,b}{1,2}
+
+      echo "{a..f}"
+      echo {a..f}
+
+      echo "{a..f..2}"
+      echo {a..f..2}
+
+      echo "{2..9}"
+      echo {2..9}
+
+      echo "{2..9..2}"
+      echo {2..9..2}

interactive-shell/src/interactive_shell.rs

@@ -136,9 +136,7 @@ impl InteractiveShell {
 
             let params = self.shell().default_exec_params();
 
-            self.shell_mut()
-                .run_string(prompt_cmd.as_str(), &params)
-                .await?;
+            self.shell_mut().run_string(prompt_cmd, &params).await?;
 
             self.shell_mut().last_exit_status = prev_last_result;
         }
@@ -149,7 +147,7 @@ impl InteractiveShell {
         match self.editor.readline(&prompt) {
             Ok(read_result) => {
                 let params = self.shell().default_exec_params();
-                match self.shell_mut().run_string(&read_result, &params).await {
+                match self.shell_mut().run_string(read_result, &params).await {
                     Ok(result) => Ok(InteractiveExecutionResult::Executed(result)),
                     Err(e) => Ok(InteractiveExecutionResult::Failed(e)),
                 }
@@ -319,7 +317,7 @@ impl rustyline::validate::Validator for EditorHelper {
     ) -> rustyline::Result<rustyline::validate::ValidationResult> {
         let line = ctx.input();
 
-        let parse_result = self.shell.parse_string(line);
+        let parse_result = self.shell.parse_string(line.to_owned());
 
         let validation_result = match parse_result {
             Err(parser::ParseError::Tokenizing { inner, position: _ }) if inner.is_incomplete() => {

parser/src/parser.rs

@@ -2,7 +2,7 @@ use crate::ast::{self, SeparatorOperator};
 use crate::error;
 use crate::tokenizer::{Token, TokenEndReason, Tokenizer, TokenizerOptions, Tokens};
 
-#[derive(Clone)]
+#[derive(Clone, Eq, Hash, PartialEq)]
 pub struct ParserOptions {
     pub enable_extended_globbing: bool,
     pub posix_mode: bool,
@@ -57,6 +57,8 @@ impl<R: std::io::BufRead> Parser<R> {
             },
         );
 
+        tracing::debug!(target: "tokenize", "Tokenizing...");
+
         let mut tokens = vec![];
         loop {
             let result = match tokenizer.next_token() {
@@ -85,6 +87,8 @@ impl<R: std::io::BufRead> Parser<R> {
             }
         }
 
+        tracing::debug!(target: "tokenize", "  => {} token(s)", tokens.len());
+
         parse_tokens(&tokens, &self.options, &self.source_info)
     }
 }
@@ -251,14 +255,17 @@ peg::parser! {
             non_posix_extensions_enabled() c:arithmetic_for_clause() { ast::CompoundCommand::ArithmeticForClause(c) } /
             expected!("compound command")
 
-        // N.B. This is not supported in sh.
         rule arithmetic_command() -> ast::ArithmeticCommand =
             specific_operator("(") specific_operator("(") expr:arithmetic_expression() arithmetic_end() {
                 ast::ArithmeticCommand { expr }
             }
 
         rule arithmetic_expression() -> ast::UnexpandedArithmeticExpr =
-            raw_expr:$((!arithmetic_end() [_])*) { ast::UnexpandedArithmeticExpr { value: raw_expr } }
+            raw_expr:$(arithmetic_expression_piece()*) { ast::UnexpandedArithmeticExpr { value: raw_expr } }
+
+        rule arithmetic_expression_piece() =
+            specific_operator("(") arithmetic_expression_piece()* specific_operator(")") {} /
+            !arithmetic_end() [_] {}
 
         // TODO: evaluate arithmetic end; the semicolon is used in arithmetic for loops.
         rule arithmetic_end() -> () =