Support help argument in "{ -h"

Unlike other builtins, "{" is a separate token, not a keyword-string
token.

Allow the left brace token as command string; produce it when parsing
"{ -h"/"{ --help" (and nowhere else).  By using a decorated statement,
we reuse logic for redirections etc.

Other syntax elements like "and" are in the builtin list, which
- adds highlighting logic
- adds it to "builtin --names"
- makes it runnable as builtin
  (e.g. "builtin '{'" would hypothetically print the man page)

These don't seem very important (highlighting for '{' needs to match
'}' anyway).

Additionally, making it a real builtin would mean that we'd need to
deactivate a few places that unescape "{" to BRACE_BEGIN.

Let's not add it to the built in list. Instead, simply synthesize
builtin_generic in the right spot.

I'm assuming we want "{ -h" to print help, but '"{" -h' to run an
external command, since the latter is historical behavior.  This works
naturally with the above fake builtin approach which never tries to
unescape the left brace.

share/functions/__fish_print_help.fish

@@ -8,6 +8,8 @@ function __fish_print_help --description "Print help message for the specified f
             set item true
         case '['
             set item test
+        case '{'
+            set item begin
     end
 
     # Do nothing if the file does not exist

src/ast.rs

@@ -71,6 +71,7 @@ fn visit_block_statement_header(
         &mut self,
         _node: &mut BlockStatementHeaderVariant,
     ) -> VisitResult;
+    fn visit_command_token(&mut self, _node: &mut CommandTokenVariant) -> VisitResult;
     fn visit_statement(&mut self, _node: &mut StatementVariant) -> VisitResult;
 
     fn visit_decorated_statement_decorator(
@@ -179,6 +180,9 @@ fn as_argument_or_redirection(&self) -> Option<&ArgumentOrRedirection> {
     fn as_argument_or_redirection_list(&self) -> Option<&ArgumentOrRedirectionList> {
         None
     }
+    fn as_command_token(&self) -> Option<&CommandToken> {
+        None
+    }
     fn as_statement(&self) -> Option<&Statement> {
         None
     }
@@ -300,6 +304,9 @@ fn as_mut_argument_or_redirection(&mut self) -> Option<&mut ArgumentOrRedirectio
     fn as_mut_argument_or_redirection_list(&mut self) -> Option<&mut ArgumentOrRedirectionList> {
         None
     }
+    fn as_mut_command_token(&mut self) -> Option<&mut CommandToken> {
+        None
+    }
     fn as_mut_statement(&mut self) -> Option<&mut Statement> {
         None
     }
@@ -923,6 +930,9 @@ macro_rules! visit_variant_field_mut {
     (BlockStatementHeaderVariant, $visitor:ident, $field:expr) => {
         $visitor.visit_block_statement_header(&mut $field)
     };
+    (CommandTokenVariant, $visitor:ident, $field:expr) => {
+        $visitor.visit_command_token(&mut $field)
+    };
     (StatementVariant, $visitor:ident, $field:expr) => {
         $visitor.visit_statement(&mut $field)
     };
@@ -1023,6 +1033,19 @@ macro_rules! set_parent_of_union_field {
             $self.$field_name.as_mut_redirection().set_parents();
         }
     };
+    (
+        $self:ident,
+        $field_name:ident,
+        CommandTokenVariant
+    ) => {
+        if matches!($self.$field_name, CommandTokenVariant::LeftBrace(_)) {
+            $self.$field_name.as_mut_left_brace().parent = Some($self);
+            $self.$field_name.as_mut_left_brace().set_parents();
+        } else {
+            $self.$field_name.as_mut_string().parent = Some($self);
+            $self.$field_name.as_mut_string().set_parents();
+        }
+    };
     (
         $self:ident,
         $field_name:ident,
@@ -1164,6 +1187,38 @@ fn as_mut_argument_or_redirection_list(&mut self) -> Option<&mut ArgumentOrRedir
     }
 }
 
+#[derive(Default, Debug)]
+pub struct CommandToken {
+    parent: Option<*const dyn Node>,
+    pub contents: CommandTokenVariant,
+}
+implement_node!(CommandToken, branch, command_token);
+implement_acceptor_for_branch!(
+    CommandToken,
+    (contents: (variant<CommandTokenVariant>))
+);
+impl ConcreteNode for CommandToken {
+    fn as_command_token(&self) -> Option<&CommandToken> {
+        Some(self)
+    }
+}
+impl ConcreteNodeMut for CommandToken {
+    fn as_mut_command_token(&mut self) -> Option<&mut CommandToken> {
+        Some(self)
+    }
+}
+impl CheckParse for CommandToken {
+    fn can_be_parsed(pop: &mut Populator<'_>) -> bool {
+        let typ = pop.peek_type(0);
+        matches!(typ, ParseTokenType::string | ParseTokenType::left_brace)
+    }
+}
+impl CommandToken {
+    pub fn is_left_brace(&self) -> bool {
+        matches!(&self.contents, CommandTokenVariant::LeftBrace(_))
+    }
+}
+
 /// A statement is a normal command, or an if / while / etc
 #[derive(Default, Debug)]
 pub struct Statement {
@@ -1652,7 +1707,7 @@ pub struct DecoratedStatement {
     /// An optional decoration (command, builtin, exec, etc).
     pub opt_decoration: Option<DecoratedStatementDecorator>,
     /// Command to run.
-    pub command: String_,
+    pub command: CommandToken,
     /// Args and redirs
     pub args_or_redirs: ArgumentOrRedirectionList,
 }
@@ -1660,7 +1715,7 @@ pub struct DecoratedStatement {
 implement_acceptor_for_branch!(
     DecoratedStatement,
     (opt_decoration: (Option<DecoratedStatementDecorator>)),
-    (command: (String_)),
+    (command: (CommandToken)),
     (args_or_redirs: (ArgumentOrRedirectionList)),
 );
 impl ConcreteNode for DecoratedStatement {
@@ -2171,6 +2226,56 @@ pub fn redirection(&self) -> &Redirection {
     }
 }
 
+#[derive(Debug)]
+pub enum CommandTokenVariant {
+    String(String_),
+    LeftBrace(TokenLeftBrace),
+}
+
+impl Default for CommandTokenVariant {
+    fn default() -> Self {
+        CommandTokenVariant::String(String_::default())
+    }
+}
+
+impl Acceptor for CommandTokenVariant {
+    fn accept<'a>(&'a self, visitor: &mut dyn NodeVisitor<'a>, reversed: bool) {
+        match self {
+            CommandTokenVariant::String(child) => child.accept(visitor, reversed),
+            CommandTokenVariant::LeftBrace(child) => child.accept(visitor, reversed),
+        }
+    }
+}
+impl AcceptorMut for CommandTokenVariant {
+    fn accept_mut(&mut self, visitor: &mut dyn NodeVisitorMut, reversed: bool) {
+        match self {
+            CommandTokenVariant::String(child) => child.accept_mut(visitor, reversed),
+            CommandTokenVariant::LeftBrace(child) => child.accept_mut(visitor, reversed),
+        }
+    }
+}
+
+impl CommandTokenVariant {
+    pub fn embedded_node(&self) -> &dyn Node {
+        match self {
+            CommandTokenVariant::String(node) => node,
+            CommandTokenVariant::LeftBrace(node) => node,
+        }
+    }
+    fn as_mut_string(&mut self) -> &mut String_ {
+        match self {
+            CommandTokenVariant::String(node) => node,
+            _ => panic!(),
+        }
+    }
+    fn as_mut_left_brace(&mut self) -> &mut TokenLeftBrace {
+        match self {
+            CommandTokenVariant::LeftBrace(node) => node,
+            _ => panic!(),
+        }
+    }
+}
+
 #[derive(Debug)]
 pub enum BlockStatementHeaderVariant {
     None,
@@ -2425,6 +2530,7 @@ pub fn ast_type_to_string(t: Type) -> &'static wstr {
         Type::variable_assignment_list => L!("variable_assignment_list"),
         Type::argument_or_redirection => L!("argument_or_redirection"),
         Type::argument_or_redirection_list => L!("argument_or_redirection_list"),
+        Type::command_token => L!("command_token"),
         Type::statement => L!("statement"),
         Type::job_pipeline => L!("job_pipeline"),
         Type::job_conjunction => L!("job_conjunction"),
@@ -3105,6 +3211,17 @@ fn visit_block_statement_header(
         *node = self.allocate_populate_block_header();
         VisitResult::Continue(())
     }
+    fn visit_command_token(&mut self, node: &mut CommandTokenVariant) -> VisitResult {
+        if let Some(lbrace) = self.try_parse::<TokenLeftBrace>() {
+            *node = CommandTokenVariant::LeftBrace(*lbrace);
+        } else if let Some(s) = self.try_parse::<String_>() {
+            *node = CommandTokenVariant::String(*s);
+        } else {
+            // On error, we return an unsourced decorated statement.
+            *node = CommandTokenVariant::String(Default::default());
+        }
+        ControlFlow::Continue(())
+    }
     fn visit_statement(&mut self, node: &mut StatementVariant) -> VisitResult {
         *node = self.allocate_populate_statement_contents();
         VisitResult::Continue(())
@@ -3653,6 +3770,9 @@ fn new_decorated_statement(slf: &mut Populator<'_>) -> StatementVariant {
             // Construct a decorated statement, which will be unsourced.
             self.allocate_visit::<DecoratedStatement>();
         } else if self.peek_token(0).typ == ParseTokenType::left_brace {
+            if self.peek_token(1).is_help_argument {
+                return new_decorated_statement(self);
+            }
             let embedded = self.allocate_visit::<BraceStatement>();
             return StatementVariant::BraceStatement(embedded);
         } else if self.peek_token(0).typ != ParseTokenType::string {
@@ -4134,6 +4254,7 @@ pub enum Type {
     variable_assignment_list,
     argument_or_redirection,
     argument_or_redirection_list,
+    command_token,
     statement,
     job_pipeline,
     job_conjunction,

src/builtins/fish_indent.rs

@@ -342,6 +342,7 @@ fn gap_text_flags_before_node(&self, node: &dyn Node) -> GapFlags {
                         // Allow escaped newlines before commands that follow a variable assignment
                         // since both can be long (#7955).
                         let p = node.parent().unwrap();
+                        let p = p.parent().unwrap();
                         if p.typ() != Type::decorated_statement {
                             return result;
                         }

src/builtins/shared.rs

@@ -415,12 +415,17 @@ pub fn builtin_run(parser: &Parser, argv: &mut [&wstr], streams: &mut IoStreams)
         return ProcStatus::from_exit_code(STATUS_CMD_OK.unwrap());
     }
 
+    let builtin_func = if argv[0] == L!("{") {
+        builtin_generic
+    } else {
         let Some(builtin) = builtin_lookup(argv[0]) else {
             FLOGF!(error, "%s", wgettext_fmt!(UNKNOWN_BUILTIN_ERR_MSG, argv[0]));
             return ProcStatus::from_exit_code(STATUS_CMD_ERROR.unwrap());
         };
+        builtin.func
+    };
 
-    let builtin_ret = (builtin.func)(parser, streams, argv);
+    let builtin_ret = (builtin_func)(parser, streams, argv);
 
     // Flush our out and error streams, and check for their errors.
     let out_ret = streams.out.flush_and_check_error();

src/highlight/highlight.rs

@@ -753,7 +753,7 @@ fn io_still_ok(&self) -> bool {
     }
 
     // Color a command.
-    fn color_command(&mut self, node: &ast::String_) {
+    fn color_command(&mut self, node: &ast::CommandToken) {
         let source_range = node.source_range();
         let cmd_str = self.get_source(source_range);
 

src/parse_execution.rs

@@ -486,7 +486,7 @@ fn expand_command(
 
         // Get the unexpanded command string. We expect to always get it here.
         let unexp_cmd = self.node_source(&statement.command);
-        let pos_of_command_token = statement.command.range().unwrap().start();
+        let pos_of_command_token = statement.command.try_source_range().unwrap().start();
 
         // Expand the string to produce completions, and report errors.
         let expand_err = expand_to_command_and_args(
@@ -733,6 +733,10 @@ fn populate_plain_process(
         // Get the command and any arguments due to expanding the command.
         let mut cmd = WString::new();
         let mut args_from_cmd_expansion = vec![];
+        let mut process_type = if statement.command.is_left_brace() {
+            cmd = L!("{").to_owned();
+            ProcessType::builtin
+        } else {
             let ret = self.expand_command(ctx, statement, &mut cmd, &mut args_from_cmd_expansion);
             if ret != EndExecutionReason::ok {
                 return ret;
@@ -749,7 +753,9 @@ fn populate_plain_process(
             );
 
             // Determine the process type.
-        let mut process_type = self.process_type_for_command(ctx, statement, &cmd);
+            self.process_type_for_command(ctx, statement, &cmd)
+        };
+
         let external_cmd = if [ProcessType::external, ProcessType::exec].contains(&process_type) {
             let parser = ctx.parser();
             // Determine the actual command. This may be an implicit cd.

src/parse_util.rs

@@ -1679,7 +1679,7 @@ fn detect_errors_in_decorated_statement(
     }
 
     let unexp_command = com;
-    if !unexp_command.is_empty() {
+    if !unexp_command.is_empty() && !dst.command.is_left_brace() {
         // Check that we can expand the command.
         // Make a new error list so we can fix the offset for just those, then append later.
         let mut new_errors = ParseErrorList::new();

src/reader.rs

@@ -5315,7 +5315,7 @@ fn extract_tokens(s: &wstr) -> Vec<PositionedToken> {
         let mut cursor = Some(node);
         while let Some(cur) = cursor {
             if let Some(stmt) = cur.as_decorated_statement() {
-                if node.pointer_eq(&stmt.command) {
+                if node.pointer_eq(stmt.command.contents.embedded_node()) {
                     return true;
                 }
             }

src/tests/parser.rs

@@ -496,6 +496,7 @@ macro_rules! validate {
     validate!("command", "command", "", StatementDecoration::none);
     validate!("command -", "command", "-", StatementDecoration::none);
     validate!("command --", "command", "--", StatementDecoration::none);
+    validate!("{ -h", "{", "-h", StatementDecoration::none);
     validate!(
         "builtin --names",
         "builtin",

tests/checks/braces.fish

@@ -187,11 +187,27 @@ PATH= "{"
 # CHECKERR: PATH= "{"
 # CHECKERR:       ^~^
 
+$fish -c '{ -h'
+# CHECKERR: fish: '{': missing man page
+# CHECKERR: Documentation may not be installed.
+# CHECKERR: `help '{'` will show an online version
+
+PATH= "{" -h
+# CHECKERR: fish: Unknown command: '{'
+# CHECKERR: {{.*}}/braces.fish (line {{\d+}}):
+# CHECKERR: PATH= "{" -h
+# CHECKERR:       ^~^
+
 $fish -c 'builtin {'
 # CHECKERR: fish: Expected end of the statement, but found a '{'
 # CHECKERR: builtin {
 # CHECKERR:         ^
 
+$fish -c 'builtin "{"'
+# CHECKERR: fish: Unknown builtin '"{"'
+# CHECKERR: builtin "{"
+# CHECKERR: ^~~~~~~~~~^
+
 $fish -c 'command {'
 # CHECKERR: fish: Expected end of the statement, but found a '{'
 # CHECKERR: command {

tests/checks/indent.fish

@@ -482,6 +482,9 @@ level 2 } }
     # CHECK: {{^        level 2$}}
     # CHECK: {{^    \}$}}
     # CHECK: {{^\}$}}
+
+echo '{  -h'
+# CHECK: { -h
 } | $fish_indent
 
 echo 'multiline-\\