Return no status from successful variable assignments

CHANGELOG.rst

@@ -21,8 +21,9 @@ Notable improvements and fixes
 -  A new ``fish_add_path`` helper function to add paths to $PATH without producing duplicates, to be used interactively or in ``config.fish`` (#6960).
 - ``fish_preexec`` and ``fish_postexec`` events are no longer triggered for empty commands.
 - The ``test`` builtin now better shows where an error occured (#6030).
-- builtins may now output before all data is read. For example, `string replace` no longer has to read all of stdin before it can begin to output.
+- builtins may now output before all data is read. For example, ``string replace`` no longer has to read all of stdin before it can begin to output.
 - A number of new debugging categories have been added, including ``config``, ``path``, ``reader`` and ``screen`` (#6511). See the output of ``fish --print-debug-categories`` for the full list.
+- ``set`` and backgrounded jobs no longer overwrite ``$pipestatus``.
 
 Syntax changes and new commands
 -------------------------------

src/builtin_set.cpp

@@ -805,7 +805,6 @@ static int builtin_set_set(const wchar_t *cmd, set_cmd_opts_t &opts, int argc, w
 
 /// The set builtin creates, updates, and erases (removes, deletes) variables.
 maybe_t<int> builtin_set(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
-    const int incoming_exit_status = parser.get_last_status();
     wchar_t *cmd = argv[0];
     int argc = builtin_count_args(argv);
     set_cmd_opts_t opts;
@@ -838,6 +837,6 @@ maybe_t<int> builtin_set(parser_t &parser, io_streams_t &streams, wchar_t **argv
         retval = builtin_set_set(cmd, opts, argc, argv, parser, streams);
     }
 
-    if (retval == STATUS_CMD_OK && opts.preserve_failure_exit_status) retval = incoming_exit_status;
+    if (retval == STATUS_CMD_OK && opts.preserve_failure_exit_status) return none();
     return retval;
 }

src/exec.cpp

@@ -293,6 +293,13 @@ static void run_internal_process_or_short_circuit(parser_t &parser, const std::s
             if (statuses) {
                 parser.set_last_statuses(statuses.value());
                 parser.libdata().status_count++;
+            } else if (j->flags().negate) {
+                // Special handling for `not set var (substitution)`.
+                // If there is no status, but negation was requested,
+                // take the last status and negate it.
+                auto last_statuses = parser.get_last_statuses();
+                last_statuses.status = !last_statuses.status;
+                parser.set_last_statuses(last_statuses);
             }
         }
     } else {

tests/checks/set.fish

@@ -270,6 +270,30 @@ __fish_test_shadow
 env | string match '__fish_test_env17=*'
 # CHECK: __fish_test_env17=UNSHADOWED
 
+# Test that set var (command substitution) works with if/while.
+
+if set fish_test_18 (false)
+    echo Test 18 fail
+else
+    echo Test 18 pass
+end
+# CHECK: Test 18 pass
+
+if not set fish_test_18 (true)
+    echo Test 18 fail
+else
+    echo Test 18 pass
+end
+# CHECK: Test 18 pass
+
+set __fish_test_18_status pass
+while set fish_test_18 (false); or not set fish_test_18 (true)
+    set __fish_test_18_status fail
+    break
+end
+echo Test 18 $__fish_test_18_status
+# CHECK: Test 18 pass
+
 # Test that local exported variables are copied to functions (#1091)
 function __fish_test_local_export
     echo $var

tests/pexpects/postexec.py

@@ -41,6 +41,16 @@ sendline("sleep 1000 &; sleep 2000 &")
 expect_str("pipestatus:0|1, generation:%d, command:sleep 1000 &; sleep 2000 &" % generation)
 expect_prompt()
 
+# valid variable assignment
+sendline("set foo bar")
+expect_str("pipestatus:0|1, generation:%d, command:set foo bar" % generation)
+expect_prompt()
+
+# valid variable assignment with background job
+sendline("set foo bar; sleep 1000 &")
+expect_str("pipestatus:0|1, generation:%d, command:set foo bar; sleep 1000 &" % generation)
+expect_prompt()
+
 # Increments $status_generation if any job was foreground.
 sendline("false|true; sleep 1000 &")
 generation += 1
@@ -76,6 +86,18 @@ generation += 1
 expect_str("pipestatus:0, generation:%d, command:function fail; false; end" % generation)
 expect_prompt()
 
+# or an invalid variable assignment
+sendline("set '!@#$' value")
+generation += 1
+expect_str("pipestatus:2, generation:%d, command:set '!@#$' value" % generation)
+expect_prompt()
+
+# or a variable query
+sendline("set -q fish_pid")
+generation += 1
+expect_str("pipestatus:0, generation:%d, command:set -q fish_pid" % generation)
+expect_prompt()
+
 # This is just to set a memorable pipestatus.
 sendline("true|false|true")
 generation += 1
@@ -96,3 +118,8 @@ expect_prompt()
 sendline("begin; sleep 200 &; end; sleep 400 &")
 expect_str("pipestatus:0|1|0, generation:%d, command:begin; sleep 200 &; end; sleep 400 &" % generation)
 expect_prompt()
+
+# Or a combination with variable assignments
+sendline("begin; set foo bar; sleep 1000 &; end; set bar baz; sleep 2000 &")
+expect_str("pipestatus:0|1|0, generation:%d, command:begin; set foo bar; sleep 1000 &; end; set bar baz; sleep 2000 &" % generation)
+expect_prompt()