Reinstate failglob behaviour for most commands

Expand globs to zero arguments (nullglob) only for set, for and count.

The warning about failing globs, and setting the accompanying $status,
now happens regardless of mode, interactive or not.

It is assumed that the above commands are the common cases where
nullglob behaviour is desirable.
More importantly, doing this with `set` is a real feature enabler,
since the resulting empty array can be passed on to any command.

The previous behaviour was actually all nullglob (since commit
cab115c8b9), but this was undocumented;
the failglob warning was still printed in interactive mode,
and the documentation was bragging about failglob behaviour.

doc_src/index.hdr.in

@@ -413,8 +413,19 @@ Examples:
 
 - `**` matches any files and directories in the current directory and all of its subdirectories.
 
-Note that if no matches are found for a specific wildcard, it will expand into zero arguments, i.e. to nothing. If none of the wildcarded arguments sent to a command result in any matches, the command will not be executed. If this happens when using the shell interactively, a warning will also be printed.
+Note that for most commands, if any wildcard fails to expand, the command is not executed, <a href='#variables-status'>`$status`</a> is set to nonzero, and a warning is printed. This behavior is consistent with setting `shopt -s failglob` in bash. There are exactly 3 exceptions, namely <a href="commands.html#set">`set`</a>, <a href="commands.html#count">`count`</a> and <a href="commands.html#for">`for`</a>. Their globs are permitted to expand to zero arguments, as with `shopt -s nullglob` in bash.
 
+Examples:
+\fish
+ls *.foo
+# Lists the .foo files, or warns if there aren't any.
+
+set foos *.foo
+if test (count $foos) -ge 1
+    ls $foos
+end
+# Lists the .foo files, if any.
+\endfish
 
 \subsection expand-command-substitution Command substitution
 

src/parse_execution.cpp

@@ -391,7 +391,7 @@ parse_execution_result_t parse_execution_context_t::run_function_statement(const
     
     /* Get arguments */
     wcstring_list_t argument_list;
-    parse_execution_result_t result = this->determine_arguments(header, &argument_list);
+    parse_execution_result_t result = this->determine_arguments(header, &argument_list, failglob);
 
     if (result == parse_execution_success)
     {
@@ -484,7 +484,7 @@ parse_execution_result_t parse_execution_context_t::run_for_statement(const pars
 
     /* Get the contents to iterate over. */
     wcstring_list_t argument_sequence;
-    parse_execution_result_t ret = this->determine_arguments(header, &argument_sequence);
+    parse_execution_result_t ret = this->determine_arguments(header, &argument_sequence, nullglob);
     if (ret != parse_execution_success)
     {
         return ret;
@@ -611,7 +611,7 @@ parse_execution_result_t parse_execution_context_t::run_switch_statement(const p
 
             /* Expand arguments. A case item list may have a wildcard that fails to expand to anything. We also report case errors, but don't stop execution; i.e. a case item that contains an unexpandable process will report and then fail to match. */
             wcstring_list_t case_args;
-            parse_execution_result_t case_result = this->determine_arguments(arg_list, &case_args);
+            parse_execution_result_t case_result = this->determine_arguments(arg_list, &case_args, failglob);
             if (case_result == parse_execution_success)
             {
                 for (size_t i=0; i < case_args.size(); i++)
@@ -762,34 +762,7 @@ parse_execution_result_t parse_execution_context_t::report_errors(const parse_er
 parse_execution_result_t parse_execution_context_t::report_unmatched_wildcard_error(const parse_node_t &unmatched_wildcard)
 {
     proc_set_last_status(STATUS_UNMATCHED_WILDCARD);
-    // unmatched wildcards are only reported in interactive use because scripts have legitimate reasons
-    // to want to use wildcards without knowing whether they expand to anything.
-    if (get_is_interactive())
-    {
-        // Check if we're running code that was typed at the commandline.
-        // We can't just use `is_block` or the eval level, because `begin; echo *.unmatched; end` would not report
-        // the error even though it's run interactively.
-        // But any non-interactive use must have at least one function / event handler / source on the stack.
-        bool interactive = true;
-        for (size_t i = 0, count = parser->block_count(); i < count; ++i)
-        {
-            switch (parser->block_at_index(i)->type())
-            {
-                case FUNCTION_CALL:
-                case FUNCTION_CALL_NO_SHADOW:
-                case EVENT:
-                case SOURCE:
-                    interactive = false;
-                    break;
-                default:
-                    break;
-            }
-        }
-        if (interactive)
-        {
     report_error(unmatched_wildcard, WILDCARD_ERR_MSG, get_source(unmatched_wildcard).c_str());
-        }
-    }
     return parse_execution_errored;
 }
 
@@ -865,7 +838,7 @@ parse_execution_result_t parse_execution_context_t::handle_command_not_found(con
 
         wcstring_list_t event_args;
         {
-            parse_execution_result_t arg_result = this->determine_arguments(statement_node, &event_args);
+            parse_execution_result_t arg_result = this->determine_arguments(statement_node, &event_args, failglob);
 
             if (arg_result != parse_execution_success)
             {
@@ -964,8 +937,11 @@ parse_execution_result_t parse_execution_context_t::populate_plain_process(job_t
     }
     else
     {
+        const globspec_t glob_behavior = (cmd == L"set" || cmd == L"count")
+            ? nullglob
+            : failglob;
         /* Form the list of arguments. The command is the first argument. TODO: count hack, where we treat 'count --help' as different from 'count $foo' that expands to 'count --help'. fish 1.x never successfully did this, but it tried to! */
-        parse_execution_result_t arg_result = this->determine_arguments(statement, &argument_list);
+        parse_execution_result_t arg_result = this->determine_arguments(statement, &argument_list, glob_behavior);
         if (arg_result != parse_execution_success)
         {
             return arg_result;
@@ -992,11 +968,8 @@ parse_execution_result_t parse_execution_context_t::populate_plain_process(job_t
 }
 
 /* Determine the list of arguments, expanding stuff. Reports any errors caused by expansion. If we have a wildcard that could not be expanded, report the error and continue. */
-parse_execution_result_t parse_execution_context_t::determine_arguments(const parse_node_t &parent, wcstring_list_t *out_arguments)
+parse_execution_result_t parse_execution_context_t::determine_arguments(const parse_node_t &parent, wcstring_list_t *out_arguments, globspec_t glob_behavior)
 {
-    /* The ultimate result */
-    enum parse_execution_result_t result = parse_execution_success;
-
     /* Get all argument nodes underneath the statement. We guess we'll have that many arguments (but may have more or fewer, if there are wildcards involved) */
     const parse_node_tree_t::parse_node_list_t argument_nodes = tree.find_nodes(parent, symbol_argument);
     out_arguments->reserve(out_arguments->size() + argument_nodes.size());
@@ -1018,16 +991,18 @@ parse_execution_result_t parse_execution_context_t::determine_arguments(const pa
             case EXPAND_ERROR:
             {
                 this->report_errors(errors);
-                result = parse_execution_errored;
+                return parse_execution_errored;
                 break;
             }
 
             case EXPAND_WILDCARD_NO_MATCH:
             {
-                // report the unmatched wildcard error but don't stop processing.
-                // this will only print an error in interactive mode, though it does set the
-                // process status (similar to a command substitution failing)
+                if (glob_behavior == failglob)
+                {
+                    // report the unmatched wildcard error and stop processing
                     report_unmatched_wildcard_error(arg_node);
+                    return parse_execution_errored;
+                }
                 break;
             }
 
@@ -1045,7 +1020,7 @@ parse_execution_result_t parse_execution_context_t::determine_arguments(const pa
         }
     }
 
-    return result;
+    return parse_execution_success;
 }
 
 bool parse_execution_context_t::determine_io_chain(const parse_node_t &statement_node, io_chain_t *out_chain)

src/parse_execution.h

@@ -102,7 +102,12 @@ private:
     parse_execution_result_t run_function_statement(const parse_node_t &header, const parse_node_t &block_end_command);
     parse_execution_result_t run_begin_statement(const parse_node_t &header, const parse_node_t &contents);
 
-    parse_execution_result_t determine_arguments(const parse_node_t &parent, wcstring_list_t *out_arguments);
+    enum globspec_t
+    {
+        failglob,
+        nullglob
+    };
+    parse_execution_result_t determine_arguments(const parse_node_t &parent, wcstring_list_t *out_arguments, globspec_t glob_behavior);
 
     /* Determines the IO chain. Returns true on success, false on error */
     bool determine_io_chain(const parse_node_t &statement, io_chain_t *out_chain);

tests/test5.err

@@ -0,0 +1,3 @@
+No matches for wildcard '*ee*'.
+fish: 	case *ee*
+      	     ^