fix setenv and add unit tests

Fixes #3937

share/functions/setenv.fish

@@ -1,20 +1,41 @@
-function setenv --description 'Set env variable. Alias for `set -gx` for csh compatibility.'
+function setenv --description 'Set an env var for csh compatibility.'
+    # No arguments should cause the current env vars to be displayed.
     if not set -q argv[1]
-        set -x
+        env
-        return 0
+        return
     end
-    for arg in $argv
+
-        set -l v (string split -m 1 "=" -- $arg)
+    # A single argument should set the named var to nothing.
-        switch (count $v)
+    if not set -q argv[2]
-            case 1
+        set -gx $argv[1] ''
-                set -gx $v $$v
+        return
-            case 2
+    end
-                if contains -- $v[1] PATH CDPATH MANPATH
+
-                    set -l colonized_path (string replace -- "$$v[1]" (string join ":" -- $$v[1]) $v[2])
+    # `setenv` accepts only two arguments: the var name and the value. If there are more than two
-                    set -gx $v[1] (string split ":" -- $colonized_path)
+    # args it is an error. The error message is verbatim from csh.
+    if set -q argv[3]
+        printf (_ '%s: Too many arguments\n') setenv >&2
+        return 1
+    end
+
+    # We have exactly two arguments as required by the csh `setenv` command.
+    set -l var $argv[1]
+    set -l val $argv[2]
+
+    # Validate the variable name.
+    if not string match -qr '^\w+$' -- $var
+        # This message is verbatim from csh. We don't really need to do this but if we don't fish
+        # will display a different error message which might confuse someone expecting the csh
+        # message.
+        printf (_ '%s: Variable name must contain alphanumeric characters\n') setenv >&2
+        return 1
+    end
+
+    # We need to special case some vars to be compatible with fish. In particular how they are
+    # treated as arrays split on colon characters. All other var values are treated literally.
+    if contains -- $var PATH CDPATH MANPATH
+        set -gx $var (string split -- ':' $val)
     else
-                    set -gx $v[1] $v[2]
+        set -gx $var $val
-                end
-        end
     end
 end

tests/setenv.err

@@ -0,0 +1,2 @@
+too many arguments test
+setenv: Too many arguments

tests/setenv.in

@@ -0,0 +1,20 @@
+# Verify the correct behavior of the `setenv` compatibility shim.
+
+# No args to `setenv` should emit the current set of env vars. The first two
+# commands verify that `setenv` does not report non-env vars.
+set -g setenv1 abc
+setenv | grep '^setenv1=$'
+set -gx setenv1 xyz
+setenv | grep '^setenv1=xyz$'
+
+# A single arg should set and export the named var to nothing.
+setenv setenv2
+env | grep '^setenv2=$'
+
+# Three or more args should be an error.
+echo too many arguments test >&2
+setenv var hello you
+
+# Two args should set the named var to the second arg
+setenv setenv3 'hello you'
+setenv | grep '^setenv3=hello you'

tests/setenv.out

@@ -0,0 +1,3 @@
+setenv1=xyz
+setenv2=
+setenv3=hello you