Fix: eval should preserve previous $status if the evaluated block does not change it

Empty functions may return 1 when eval is used due to the $status not being correctly preserved inside the function definition.

1 files changed, 22 insertions, 21 deletions

diff --git a/share/functions/eval.fish b/share/functions/eval.fish
index 382e6fc8..a982a00f 100644
--- a/share/functions/eval.fish
+++ b/share/functions/eval.fish
@@ -1,4 +1,12 @@
 function eval -S -d "Evaluate parameters as a command"
+	# keep a copy of the previous $status and use restore_status
+	# to preserve the status in case the block that is evaluated
+	# does not modify the status itself.
+	set -l status_copy $status
+	function -S restore_status
+		return $status_copy
+	end
+
 	if not set -q argv[2]
 		# like most builtins, we only check for -h/--help
 		# if we only have a single argument
@@ -28,28 +36,21 @@ function eval -S -d "Evaluate parameters as a command"
 		status --job-control full
 	end
 
-        # rfish: To eval 'foo', we construct a block "begin ; foo; end <&3 3<&-"
-        # Note the redirections are also within the quotes.
-        #
-        # We then pipe this to 'source 3<&0’.
-        #
-        # You might expect that the dup2(3, stdin) should overwrite stdin,
-        # and therefore prevent 'source' from reading the piped-in block. This doesn't happen
-        # because when you pipe to a builtin, we don't overwrite stdin with the read end
-        # of the block; instead we set a separate fd in a variable 'builtin_stdin', which is
-        # what it reads from. So builtins are magic in that, in pipes, their stdin
-        # is not fd 0.
-        #
-        # ‘source’ does not apply the redirections to itself. Instead it saves them and passes
-        # them as block-level redirections to parser.eval(). Ultimately the eval’d code sees
-        # the following redirections (in the following order):
-        #    dup2 0 -> 3
-        #    dup2 pipe -> 0
-        #    dup2 3 -> 0
-        # where the pipe is the pipe we get from piping ‘echo’ to ‘source’. Thus the redirection
-        # effectively makes stdin fd0, instead of the thing that was piped to ‘source’
+  # rfish: To eval 'foo', we construct a block "begin ; foo; end <&3 3<&-"
+  # The 'eval2_inner' is a param to 'begin' itself; I believe it does nothing.
+  # Note the redirections are also within the quotes.
+  #
+  # We then pipe this to 'source 3<&0' which dup2's 3 to stdin.
+  #
+  # You might expect that the dup2(3, stdin) should overwrite stdin,
+  # and therefore prevent 'source' from reading the piped-in block. This doesn't happen
+  # because when you pipe to a builtin, we don't overwrite stdin with the read end
+  # of the block; instead we set a separate fd in a variable 'builtin_stdin', which is
+  # what it reads from. So builtins are magic in that, in pipes, their stdin
+  # is not fd 0.
 
-	echo "begin; $argv "\n" ;end <&3 3<&-" | source 3<&0
+	restore_status
+	echo "begin; $argv "\n" ;end eval2_inner <&3 3<&-" | source 3<&0
 	set -l res $status
 
 	status --job-control $mode