From f4d666f56cec4fe767a64bf14c9d78d71484f397 Mon Sep 17 00:00:00 2001
From: ridiculousfish <corydoras@ridiculousfish.com>
Date: Sat, 6 Oct 2018 20:22:35 -0700
Subject: [PATCH] Allow user to set and unset path property of variables

This adds flags --path and --unpath to builtin set, analogous to
--export and --unexport. These flags change whether a variable is
marked as a path variable.

Universal variables cannot yet be path variables.
---
 src/builtin_set.cpp | 28 +++++++++++++++++++++++++++-
 tests/expansion.err |  2 +-
 tests/expansion.in  | 14 ++++++++++++--
 tests/expansion.out |  9 +++++++--
 4 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/src/builtin_set.cpp b/src/builtin_set.cpp
index aea3801dc..9ef8594aa 100644
--- a/src/builtin_set.cpp
+++ b/src/builtin_set.cpp
@@ -37,6 +37,8 @@ struct set_cmd_opts_t {
     bool erase = false;
     bool list = false;
     bool unexport = false;
+    bool pathvar = false;
+    bool unpathvar = false;
     bool universal = false;
     bool query = false;
     bool shorten_ok = true;
@@ -45,6 +47,12 @@ struct set_cmd_opts_t {
     bool preserve_failure_exit_status = true;
 };
 
+/// Values used for long-only options.
+enum {
+    opt_path = 1,
+    opt_unpath = 2,
+};
+
 // Variables used for parsing the argument list. This command is atypical in using the "+"
 // (REQUIRE_ORDER) option for flag parsing. This is not typical of most fish commands. It means
 // we stop scanning for flags when the first non-flag argument is seen.
@@ -56,6 +64,7 @@ static const struct woption long_options[] = {
     {L"universal", no_argument, NULL, 'U'}, {L"long", no_argument, NULL, 'L'},
     {L"query", no_argument, NULL, 'q'},     {L"show", no_argument, NULL, 'S'},
     {L"append", no_argument, NULL, 'a'},    {L"prepend", no_argument, NULL, 'p'},
+    {L"path", no_argument, NULL, opt_path}, {L"unpath", no_argument, NULL, opt_unpath},
     {L"help", no_argument, NULL, 'h'},      {NULL, 0, NULL, 0}};
 
 // Hint for invalid path operation with a colon.
@@ -122,6 +131,14 @@ static int parse_cmd_opts(set_cmd_opts_t &opts, int *optind,  //!OCLINT(high ncs
                 opts.unexport = true;
                 break;
             }
+            case opt_path: {
+                opts.pathvar = true;
+                break;
+            }
+            case opt_unpath: {
+                opts.unpathvar = true;
+                break;
+            }
             case 'U': {
                 opts.universal = true;
                 break;
@@ -184,6 +201,13 @@ static int validate_cmd_opts(const wchar_t *cmd, set_cmd_opts_t &opts,  //!OCLIN
         return STATUS_INVALID_ARGS;
     }
 
+    // Variables can only have one path status.
+    if (opts.pathvar && opts.unpathvar) {
+        streams.err.append_format(BUILTIN_ERR_EXPUNEXP, cmd);
+        builtin_print_help(parser, streams, cmd, streams.err);
+        return STATUS_INVALID_ARGS;
+    }
+
     // Trying to erase and (un)export at the same time doesn't make sense.
     if (opts.erase && (opts.exportv || opts.unexport)) {
         streams.err.append_format(BUILTIN_ERR_COMBO, cmd);
@@ -429,13 +453,15 @@ static void erase_values(wcstring_list_t &list, const std::vector<long> &indexes
     }
 }
 
-static int compute_scope(set_cmd_opts_t &opts) {
+static env_mode_flags_t compute_scope(set_cmd_opts_t &opts) {
     int scope = ENV_USER;
     if (opts.local) scope |= ENV_LOCAL;
     if (opts.global) scope |= ENV_GLOBAL;
     if (opts.exportv) scope |= ENV_EXPORT;
     if (opts.unexport) scope |= ENV_UNEXPORT;
     if (opts.universal) scope |= ENV_UNIVERSAL;
+    if (opts.pathvar) scope |= ENV_PATHVAR;
+    if (opts.unpathvar) scope |= ENV_UNPATHVAR;
     return scope;
 }
 
diff --git a/tests/expansion.err b/tests/expansion.err
index 35a49d308..8469cfc76 100644
--- a/tests/expansion.err
+++ b/tests/expansion.err
@@ -21,4 +21,4 @@ echo $$paren
 # Test tilde expansion
 
 ####################
-# Test delimiters
+# Test path variables
diff --git a/tests/expansion.in b/tests/expansion.in
index afb76321d..e31dc9723 100644
--- a/tests/expansion.in
+++ b/tests/expansion.in
@@ -127,7 +127,17 @@ unlink $tmpdir/linkhome
 rmdir $tmpdir/realhome
 rmdir $tmpdir
 
-logmsg Test delimiters
+logmsg Test path variables
 set TEST_DELIMITER one two three
 set TEST_DELIMITER_PATH one two three
-echo "$TEST_DELIMITER" "$TEST_DELIMITER_PATH"
+echo TEST_DELIMITER: $TEST_DELIMITER "$TEST_DELIMITER"
+echo TEST_DELIMITER_PATH: $TEST_DELIMITER_PATH "$TEST_DELIMITER_PATH"
+
+set testvar ONE:TWO:THREE
+echo "Not a path: $testvar" (count $testvar)
+set --path testvar $testvar
+echo "As a path: $testvar" (count $testvar)
+set testvar "$testvar:FOUR"
+echo "Appended path: $testvar" (count $testvar)
+set --unpath testvar $testvar
+echo "Back to normal variable: $testvar" (count $testvar)
diff --git a/tests/expansion.out b/tests/expansion.out
index ab86d0c92..194906487 100644
--- a/tests/expansion.out
+++ b/tests/expansion.out
@@ -75,5 +75,10 @@ All digits: 0
 # Test tilde expansion
 
 ####################
-# Test delimiters
-one two three one:two:three
+# Test path variables
+TEST_DELIMITER: one two three one two three
+TEST_DELIMITER_PATH: one two three one:two:three
+Not a path: ONE:TWO:THREE 1
+As a path: ONE:TWO:THREE 3
+Appended path: ONE:TWO:THREE:FOUR 4
+Back to normal variable: ONE TWO THREE FOUR 4