Better handling for jobs, backquotes, and builtins (#14)

README.md

@@ -25,8 +25,6 @@ There's a lot that *is* working, but also non-trivial gaps in compatibility. Mos
   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 :).
 * **Honoring `set` options (e.g., `set -e`).**
   The `set` builtin is implemented, as is `set -x` and a few other options, but most of the behaviors aren't there. `set -e`, for example, will execute but its semantics aren't applied across execution.
-* **Backtick (`` ` ``) expansions**
-  Modern command expansions (e.g. `$(command)`) work fine. It's just the tokenizing and parsing of backtick syntax that isn't there. 
 
 Shell built-ins are a mixed bag. Some are completely and fully implemented (e.g. echo), while some only support their most commonly used options. Some aren't implemented at all.
 

cli/tests/cases/builtins/enable.yaml

@@ -0,0 +1,30 @@
+name: "Builtins: enable"
+cases:
+  - name: "List special builtins"
+    stdin: enable -s
+
+  - name: "List default-disabled builtins"
+    stdin: enable -n
+
+  - name: "List all builtins"
+    stdin: enable
+
+  - name: "Disable builtins"
+    ignore_stderr: true
+    stdin: |
+      type printf
+
+      # Disable the builtin
+      PATH=
+      enable -n printf
+
+      # Check
+      type printf
+      print "Gone\n"
+
+      # Re-enable
+      enable printf
+
+      # Re-check
+      type printf
+      printf "Back\n"

cli/tests/cases/builtins/jobs.yaml

@@ -0,0 +1,7 @@
+name: "Builtins: job-related builtins"
+cases:
+  - name: "Basic async job"
+    stdin: |
+      echo hi &
+      wait
+      jobs

cli/tests/cases/word_expansion.yaml

@@ -56,6 +56,14 @@ cases:
       echo "$(var="updated"; echo ${var})"
       echo "var=${var}"
 
+  - name: "Backtick command substitution"
+    stdin: |
+      echo `echo hi`
+
+  - name: "Backtick command substitution with escaping"
+    stdin: |
+      echo `echo \`echo hi\``
+
   - name: "String length"
     stdin: |
       x="abc"

interactive-shell/src/interactive_shell.rs

@@ -80,6 +80,9 @@ impl InteractiveShell {
 
     pub async fn run_interactively(&mut self) -> Result<(), InteractiveShellError> {
         loop {
+            // Check for any completed jobs.
+            self.shell_mut().check_for_completed_jobs()?;
+
             let result = self.run_interactively_once().await?;
             match result {
                 InteractiveExecutionResult::Executed(shell::ExecutionResult {

parser/src/word.rs

@@ -412,7 +412,7 @@ peg::parser! {
 
         pub(crate) rule command_substitution() -> WordPiece =
             "$(" c:command() ")" { WordPiece::CommandSubstitution(c.to_owned()) } /
-            "`" backquoted_command() "`" { todo!("UNIMPLEMENTED: backquoted command substitution") }
+            "`" c:backquoted_command() "`" { WordPiece::CommandSubstitution(c) }
 
         pub(crate) rule command() -> &'input str =
             $(command_piece()*)
@@ -421,8 +421,12 @@ peg::parser! {
             word_piece(<![')']>) {} /
             ([' ' | '\t'])+ {}
 
-        rule backquoted_command() -> () =
-            "<BACKQUOTES UNIMPLEMENTED>" {}
+        rule backquoted_command() -> String =
+            chars:(backquoted_char()*) { chars.into_iter().collect() }
+
+        rule backquoted_char() -> char =
+            "\\`" { '`' } /
+            [^'`']
 
         rule arithmetic_expansion() -> WordPiece =
             "$((" e:$(arithmetic_word(<!"))">)) "))" { WordPiece::ArithmeticExpression(ast::UnexpandedArithmeticExpr { value: e.to_owned() } ) }

shell/src/builtin.rs

@@ -115,10 +115,34 @@ pub trait BuiltinCommand: Parser {
         &self,
         context: context::CommandExecutionContext<'_>,
     ) -> Result<BuiltinExitCode, error::Error>;
+
+    #[allow(dead_code)]
+    fn get_long_help() -> String {
+        Self::command().render_long_help().to_string()
+    }
 }
 
 #[allow(clippy::module_name_repetitions)]
 #[async_trait::async_trait]
 pub trait BuiltinDeclarationCommand: BuiltinCommand {
     fn set_declarations(&mut self, declarations: Vec<CommandArg>);
 }
+
+#[allow(clippy::module_name_repetitions)]
+#[derive(Clone)]
+pub struct BuiltinRegistration {
+    /// Function to execute the builtin.
+    pub execute_func: BuiltinCommandExecuteFunc,
+
+    /// Function to retrieve the builtin's detailed help text.
+    pub help_func: fn() -> String,
+
+    /// Has this registration been disabled?
+    pub disabled: bool,
+
+    /// Is the builtin classified as "special" by specification?
+    pub special_builtin: bool,
+
+    /// Is this builtin one that takes specially handled declarations?
+    pub declaration_builtin: bool,
+}

shell/src/builtins/enable.rs

@@ -0,0 +1,86 @@
+use clap::Parser;
+use itertools::Itertools;
+use std::io::Write;
+
+use crate::builtin::{BuiltinCommand, BuiltinExitCode};
+use crate::error;
+
+#[derive(Parser)]
+pub(crate) struct EnableCommand {
+    #[arg(short = 'a')]
+    print_list: bool,
+
+    #[arg(short = 'n')]
+    disable: bool,
+
+    #[arg(short = 'p')]
+    print_reusably: bool,
+
+    #[arg(short = 's')]
+    special_only: bool,
+
+    #[arg(short = 'f')]
+    shared_object_path: Option<String>,
+
+    #[arg(short = 'd')]
+    remove_loaded_builtin: bool,
+
+    names: Vec<String>,
+}
+
+#[async_trait::async_trait]
+impl BuiltinCommand for EnableCommand {
+    async fn execute(
+        &self,
+        context: crate::context::CommandExecutionContext<'_>,
+    ) -> Result<crate::builtin::BuiltinExitCode, crate::error::Error> {
+        let mut result = BuiltinExitCode::Success;
+
+        if self.shared_object_path.is_some() {
+            return error::unimp("enable -f");
+        }
+        if self.remove_loaded_builtin {
+            return error::unimp("enable -d");
+        }
+
+        if !self.names.is_empty() {
+            for name in &self.names {
+                if let Some(builtin) = context.shell.builtins.get_mut(name) {
+                    builtin.disabled = self.disable;
+                } else {
+                    writeln!(context.stderr(), "{name}: not a shell builtin")?;
+                    result = BuiltinExitCode::Custom(1);
+                }
+            }
+        } else {
+            let builtins: Vec<_> = context
+                .shell
+                .builtins
+                .iter()
+                .sorted_by_key(|(name, _reg)| *name)
+                .collect();
+
+            for (builtin_name, builtin) in builtins {
+                if self.disable {
+                    if !builtin.disabled {
+                        continue;
+                    }
+                } else if self.print_list {
+                    if builtin.disabled {
+                        continue;
+                    }
+                }
+
+                if self.special_only && !builtin.special_builtin {
+                    continue;
+                }
+
+                let prefix = if builtin.disabled { "-n " } else { "" };
+
+                writeln!(context.stdout(), "enable {prefix}{builtin_name}")?;
+            }
+        }
+
+        Ok(result)
+    }
+}

shell/src/builtins/help.rs

@@ -1,9 +1,21 @@
-use crate::builtin::{BuiltinCommand, BuiltinExitCode};
+use crate::builtin::{BuiltinCommand, BuiltinExitCode, BuiltinRegistration};
 use clap::Parser;
+use itertools::Itertools;
 use std::io::Write;
 
 #[derive(Parser)]
-pub(crate) struct HelpCommand {}
+pub(crate) struct HelpCommand {
+    #[arg(short = 'd')]
+    short_description: bool,
+
+    #[arg(short = 'm')]
+    man_page_style: bool,
+
+    #[arg(short = 's')]
+    short_usage: bool,
+
+    topic_patterns: Vec<String>,
+}
 
 const VERSION: &str = env!("CARGO_PKG_VERSION");
 
@@ -13,28 +25,81 @@ impl BuiltinCommand for HelpCommand {
         &self,
         context: crate::context::CommandExecutionContext<'_>,
     ) -> Result<crate::builtin::BuiltinExitCode, crate::error::Error> {
+        if self.topic_patterns.is_empty() {
+            Self::display_general_help(&context)?;
+        } else {
+            for topic_pattern in &self.topic_patterns {
+                self.display_help_for_topic_pattern(&context, topic_pattern)?;
+            }
+        }
+
+        Ok(BuiltinExitCode::Success)
+    }
+}
+
+impl HelpCommand {
+    fn display_general_help(
+        context: &crate::context::CommandExecutionContext<'_>,
+    ) -> Result<(), crate::error::Error> {
+        const COLUMN_COUNT: usize = 3;
+
         writeln!(context.stdout(), "brush version {VERSION}\n")?;
 
         writeln!(
             context.stdout(),
             "The following commands are implemented as shell built-ins:"
         )?;
 
-        let builtin_names = context.shell.get_builtin_names();
-
-        const COLUMN_COUNT: usize = 3;
-
-        let items_per_column = (builtin_names.len() + COLUMN_COUNT - 1) / COLUMN_COUNT;
+        let builtins: Vec<(&String, &BuiltinRegistration)> = context
+            .shell
+            .builtins
+            .iter()
+            .sorted_by_key(|(name, _)| *name)
+            .collect();
+        let items_per_column = (builtins.len() + COLUMN_COUNT - 1) / COLUMN_COUNT;
 
         for i in 0..items_per_column {
             for j in 0..COLUMN_COUNT {
-                if let Some(name) = builtin_names.get(i + j * items_per_column) {
-                    write!(context.stdout(), "  {name:<20}")?; // adjust 20 to the desired column width
+                if let Some((name, builtin)) = builtins.get(i + j * items_per_column) {
+                    let prefix = if builtin.disabled { "*" } else { " " };
+                    write!(context.stdout(), "  {prefix}{name:<20}")?; // adjust 20 to the desired column width
                 }
             }
             writeln!(context.stdout())?;
         }
 
-        Ok(BuiltinExitCode::Success)
+        Ok(())
+    }
+
+    fn display_help_for_topic_pattern(
+        &self,
+        context: &crate::context::CommandExecutionContext<'_>,
+        topic_pattern: &str,
+    ) -> Result<(), crate::error::Error> {
+        let pattern = crate::patterns::Pattern::from(topic_pattern);
+
+        let mut found_count = 0;
+        for (builtin_name, builtin_registration) in &context.shell.builtins {
+            if pattern.exactly_matches(builtin_name.as_str(), false)? {
+                self.display_help_for_builtin(context, builtin_registration)?;
+                found_count += 1;
+            }
+        }
+
+        if found_count == 0 {
+            writeln!(context.stderr(), "No help topics match '{topic_pattern}'")?;
+        }
+
+        Ok(())
+    }
+
+    #[allow(clippy::unused_self)]
+    fn display_help_for_builtin(
+        &self,
+        context: &crate::context::CommandExecutionContext<'_>,
+        registration: &BuiltinRegistration,
+    ) -> Result<(), crate::error::Error> {
+        writeln!(context.stdout(), "{}", (registration.help_func)())?;
+        Ok(())
     }
 }

shell/src/builtins/jobs.rs

@@ -51,22 +51,7 @@ impl BuiltinCommand for JobsCommand {
         }
 
         for job in &context.shell.jobs.background_jobs {
-            let annotation = if job.is_current() {
-                "+"
-            } else if job.is_prev() {
-                "-"
-            } else {
-                ""
-            };
-
-            writeln!(
-                context.stdout(),
-                "[{}]{:3}{:24}{}",
-                job.id,
-                annotation,
-                job.state.to_string(),
-                job.command_line
-            )?;
+            writeln!(context.stdout(), "{job}")?;
         }
 
         Ok(BuiltinExitCode::Success)