return to psub --file being the default

The recent change to switch `psub` to use `argparse` caused it to use
a fifo by default because it inadvertently fixed a long standing bug in
the fish script. This changes the behavior back to `psub --file` being
the default behavior and introduces a `--fifo` flag. It also updates the
documentation to make it clearer when and why `--fifo` mode should not
be used.

Fixes #4222

doc_src/psub.txt

@@ -2,17 +2,20 @@
 
 \subsection psub-synopsis Synopsis
 \fish{synopsis}
-COMMAND1 ( COMMAND2 | psub [-f] [-s SUFFIX])
+COMMAND1 ( COMMAND2 | psub [-F | --fifo] [-f | --file] [-s SUFFIX])
 \endfish
 
 \subsection psub-description Description
 
-Posix shells feature a syntax that is a mix between command substitution and piping, called process substitution. It is used to send the output of a command into the calling command, much like command substitution, but with the difference that the output is not sent through commandline arguments but through a named pipe, with the filename of the named pipe sent as an argument to the calling program. `psub` combined with a regular command substitution provides the same functionality.
+Some shells (e.g., ksh, bash) feature a syntax that is a mix between command substitution and piping, called process substitution. It is used to send the output of a command into the calling command, much like command substitution, but with the difference that the output is not sent through commandline arguments but through a named pipe, with the filename of the named pipe sent as an argument to the calling program. `psub` combined with a regular command substitution provides the same functionality.
 
-If the `-f` or `--file` switch is given to `psub`, `psub` will use a regular file instead of a named pipe to communicate with the calling process. This will cause `psub` to be significantly slower when large amounts of data are involved, but has the advantage that the reading process can seek in the stream.
+The following options are available:
 
-If the `-s` or `---suffix` switch is given, `psub` will append SUFFIX to the filename.
+- `-f` or `--file` will cause psub to use a regular file instead of a named pipe to communicate with the calling process. This will cause `psub` to be significantly slower when large amounts of data are involved, but has the advantage that the reading process can seek in the stream. This is the default.
 
+- `-F` or `--fifo` will cause psub to use a named pipe rather than a file. You should only use this if the command produces no more than 8 KiB of output. The limit on the amount of data a FIFO can buffer varies with the OS but is typically 8 KiB, 16 KiB or 64 KiB. If you use this option and the command on the left of the psub pipeline produces more output a deadlock is likely to occur.
+
+- `-s` or `--suffix` will append SUFFIX to the filename.
 
 \subsection psub-example Example
 
@@ -20,6 +23,6 @@ If the `-s` or `---suffix` switch is given, `psub` will append SUFFIX to the fil
 diff (sort a.txt | psub) (sort b.txt | psub)
 # shows the difference between the sorted versions of files `a.txt` and `b.txt`.
 
-source-highlight -f esc (cpp main.c | psub -s .c)
+source-highlight -f esc (cpp main.c | psub -f -s .c)
 # highlights `main.c` after preprocessing as a C source.
 \endfish

share/functions/psub.fish

@@ -1,5 +1,5 @@
 function psub --description "Read from stdin into a file and output the filename. Remove the file when the command that called psub exits."
-    set -l options 'h/help' 'f/file' 's/suffix='
+    set -l options -x 'f,F' -x 'F,s' 'h/help' 'f/file' 'F/fifo' 's/suffix=' 'T-testing'
     argparse -n psub --max-args=0 $options -- $argv
     or return
 
@@ -21,10 +21,10 @@ function psub --description "Read from stdin into a file and output the filename
     set -q TMPDIR
     and set tmpdir $TMPDIR
 
-    if not set -q _flag_file
+    if set -q _flag_fifo
         # Write output to pipe. This needs to be done in the background so
         # that the command substitution exits without needing to wait for
-        # all the commands to exit
+        # all the commands to exit.
         set dirname (mktemp -d $tmpdir/.psub.XXXXXXXXXX)
         or return
         set filename $dirname/psub.fifo"$_flag_suffix"
@@ -35,13 +35,18 @@ function psub --description "Read from stdin into a file and output the filename
         cat >$filename
     else
         set dirname (mktemp -d $tmpdir/.psub.XXXXXXXXXX)
-        set filename $dirname/psub"$_flag_suffix"
+        set filename "$dirname/psub$_flag_suffix"
         cat >$filename
     end
 
     # Write filename to stdout
     echo $filename
 
+    # This flag isn't documented. It's strictly for our unit tests.
+    if set -q _flag_testing
+        return
+    end
+
     # Find unique function name
     while true
         set funcname __fish_psub_(random)
@@ -53,7 +58,7 @@ function psub --description "Read from stdin into a file and output the filename
     # Make sure we erase file when caller exits
     function $funcname --on-job-exit caller --inherit-variable filename --inherit-variable dirname --inherit-variable funcname
         command rm $filename
-        if count $dirname >/dev/null
+        if test -n "$dirname"
             command rmdir $dirname
         end
         functions -e $funcname

tests/psub.in

@@ -0,0 +1,49 @@
+# Test psub behavior
+set -l filename (echo foo | psub --testing)
+test -f $filename
+or echo 'psub is not a regular file' >&2
+rm $filename
+
+set -l filename (echo foo | psub --testing --file)
+test -f $filename
+or echo 'psub is not a regular file' >&2
+rm $filename
+
+set -l filename (echo foo | psub --testing --fifo)
+test -p $filename
+or echo 'psub is not a fifo' >&2
+rm $filename
+
+cat (echo foo | psub)
+cat (echo bar | psub --fifo)
+cat (echo baz | psub)
+
+set -l filename (echo foo | psub)
+if test -e $filename
+    echo 'psub file was not deleted'
+else
+    echo 'psub file was deleted'
+end
+
+# The --file flag is the default behavior.
+if count (echo foo | psub -s .cc | grep -o '\.cc$') >/dev/null
+    echo 'psub filename ends with .cc'
+else
+    echo 'psub filename does not end with .cc'
+end
+
+# Make sure we get the same result if we explicitly ask for a temp file.
+if count (echo foo | psub -f -s .cc | grep -o '\.cc$') >/dev/null
+    echo 'psub filename ends with .cc'
+else
+    echo 'psub filename does not end with .cc'
+end
+
+set -l filename (echo foo | psub -s .fish)
+if test -e (dirname $filename)
+    echo 'psub directory was not deleted'
+else
+    echo 'psub directory was deleted'
+end
+
+diff -q (__fish_print_help psub | psub) (psub -hs banana | psub)

tests/psub.out

@@ -0,0 +1,7 @@
+foo
+bar
+baz
+psub file was deleted
+psub filename ends with .cc
+psub filename ends with .cc
+psub directory was deleted

tests/test9.in

@@ -85,39 +85,6 @@ eval 'status -n
 status -n
 status -n'
 
-# Test psub
-cat (echo foo | psub)
-cat (echo bar | psub)
-cat (echo baz | psub)
-
-set -l filename (echo foo | psub)
-if test -e $filename
-    echo 'psub file was not deleted'
-else
-    echo 'psub file was deleted'
-end
-
-if count (echo foo | psub -s .cc | grep -o '\.cc$') >/dev/null
-    echo 'psub filename ends with .cc'
-else
-    echo 'psub filename does not end with .cc'
-end
-
-if count (echo foo | psub -f -s .cc | grep -o '\.cc$') >/dev/null
-    echo 'psub filename ends with .cc'
-else
-    echo 'psub filename does not end with .cc'
-end
-
-set -l filename (echo foo | psub -s .fish)
-if test -e (dirname $filename)
-    echo 'psub directory was not deleted'
-else
-    echo 'psub directory was deleted'
-end
-
-diff -q (__fish_print_help psub | psub) (psub -hs banana | psub)
-
 # Test support for unbalanced blocks
 function try_unbalanced_block
     ../test/root/bin/fish -c "echo $argv | source " 2>&1 | grep "Missing end" 1>&2

tests/test9.out

@@ -13,13 +13,6 @@ Testing for loop
 1
 2
 3
-foo
-bar
-baz
-psub file was deleted
-psub filename ends with .cc
-psub filename ends with .cc
-psub directory was deleted
 bom_test
 not#a#comment
 is