From 2e24de7f47feeff829472e141659472f12ab9f7f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= <kubouch@gmail.com>
Date: Sat, 24 Jul 2021 19:44:36 +0300
Subject: [PATCH]  Support other variables than PATH in pathvar (2nd attempt)
 (#3828)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Fix swapped PATH env var separators

* Support pathvar to manipulate other vars than PATH

* Add tests for pathvar and its subcommands

* Adjust pathvar tests to comply with env quirks

* Make pathvar tests work on non-Windows as well

* Compact the comments in pathvar tests

* Fix last failing test

Co-authored-by: Jakub Žádník <jakub.zadnik@tuni.fi>
---
 crates/nu-command/src/commands/pathvar/add.rs |  22 +-
 .../nu-command/src/commands/pathvar/append.rs |  23 +-
 .../src/commands/pathvar/command.rs           |  29 +-
 crates/nu-command/src/commands/pathvar/mod.rs |  12 +
 .../nu-command/src/commands/pathvar/remove.rs |  31 +-
 .../nu-command/src/commands/pathvar/reset.rs  |  27 +-
 .../nu-command/src/commands/pathvar/save.rs   |  24 +-
 crates/nu-command/tests/commands/mod.rs       |   1 +
 .../nu-command/tests/commands/pathvar/mod.rs  | 380 ++++++++++++++++++
 9 files changed, 510 insertions(+), 39 deletions(-)
 create mode 100644 crates/nu-command/tests/commands/pathvar/mod.rs

diff --git a/crates/nu-command/src/commands/pathvar/add.rs b/crates/nu-command/src/commands/pathvar/add.rs
index a3d1d09bcf..4605a626f3 100644
--- a/crates/nu-command/src/commands/pathvar/add.rs
+++ b/crates/nu-command/src/commands/pathvar/add.rs
@@ -1,9 +1,10 @@
+use super::get_var;
 use crate::prelude::*;
 use nu_engine::WholeStreamCommand;
 use nu_errors::ShellError;
 use nu_protocol::{Signature, SyntaxShape};
 use nu_source::Tagged;
-use nu_test_support::{NATIVE_PATH_ENV_SEPARATOR, NATIVE_PATH_ENV_VAR};
+use nu_test_support::NATIVE_PATH_ENV_SEPARATOR;
 use std::path::PathBuf;
 
 pub struct SubCommand;
@@ -14,7 +15,14 @@ impl WholeStreamCommand for SubCommand {
     }
 
     fn signature(&self) -> Signature {
-        Signature::build("pathvar add").required("path", SyntaxShape::FilePath, "path to add")
+        Signature::build("pathvar add")
+            .required("path", SyntaxShape::FilePath, "path to add")
+            .named(
+                "var",
+                SyntaxShape::String,
+                "Use a different variable than PATH",
+                Some('v'),
+            )
     }
 
     fn usage(&self) -> &str {
@@ -37,17 +45,21 @@ impl WholeStreamCommand for SubCommand {
 pub fn add(args: CommandArgs) -> Result<OutputStream, ShellError> {
     let ctx = &args.context;
 
+    let var = get_var(&args)?;
     let path_to_add: Tagged<PathBuf> = args.req(0)?;
     let path = path_to_add.item.into_os_string().into_string();
 
     if let Ok(mut path) = path {
         path.push(NATIVE_PATH_ENV_SEPARATOR);
-        if let Some(old_pathvar) = ctx.scope.get_env(NATIVE_PATH_ENV_VAR) {
+        if let Some(old_pathvar) = ctx.scope.get_env(&var) {
             path.push_str(&old_pathvar);
-            ctx.scope.add_env_var(NATIVE_PATH_ENV_VAR, path);
+            ctx.scope.add_env_var(&var.item, path);
             Ok(OutputStream::empty())
         } else {
-            Err(ShellError::unexpected("PATH not set"))
+            Err(ShellError::unexpected(&format!(
+                "Variable {} not set",
+                &var.item
+            )))
         }
     } else {
         Err(ShellError::labeled_error(
diff --git a/crates/nu-command/src/commands/pathvar/append.rs b/crates/nu-command/src/commands/pathvar/append.rs
index 11f199510a..855cb179ca 100644
--- a/crates/nu-command/src/commands/pathvar/append.rs
+++ b/crates/nu-command/src/commands/pathvar/append.rs
@@ -1,9 +1,10 @@
+use super::get_var;
 use crate::prelude::*;
 use nu_engine::WholeStreamCommand;
 use nu_errors::ShellError;
 use nu_protocol::{Signature, SyntaxShape};
 use nu_source::Tagged;
-use nu_test_support::{NATIVE_PATH_ENV_SEPARATOR, NATIVE_PATH_ENV_VAR};
+use nu_test_support::NATIVE_PATH_ENV_SEPARATOR;
 use std::path::PathBuf;
 
 pub struct SubCommand;
@@ -14,7 +15,14 @@ impl WholeStreamCommand for SubCommand {
     }
 
     fn signature(&self) -> Signature {
-        Signature::build("pathvar append").required("path", SyntaxShape::FilePath, "path to append")
+        Signature::build("pathvar append")
+            .required("path", SyntaxShape::FilePath, "path to append")
+            .named(
+                "var",
+                SyntaxShape::String,
+                "Use a different variable than PATH",
+                Some('v'),
+            )
     }
 
     fn usage(&self) -> &str {
@@ -36,17 +44,22 @@ impl WholeStreamCommand for SubCommand {
 
 pub fn add(args: CommandArgs) -> Result<OutputStream, ShellError> {
     let ctx = &args.context;
+
+    let var = get_var(&args)?;
     let path_to_append_arg: Tagged<PathBuf> = args.req(0)?;
     let path_to_append = path_to_append_arg.item.into_os_string().into_string();
 
     if let Ok(path) = path_to_append {
-        if let Some(mut pathvar) = ctx.scope.get_env(NATIVE_PATH_ENV_VAR) {
+        if let Some(mut pathvar) = ctx.scope.get_env(&var) {
             pathvar.push(NATIVE_PATH_ENV_SEPARATOR);
             pathvar.push_str(&path);
-            ctx.scope.add_env_var(NATIVE_PATH_ENV_VAR, pathvar);
+            ctx.scope.add_env_var(&var.item, pathvar);
             Ok(OutputStream::empty())
         } else {
-            Err(ShellError::unexpected("PATH not set"))
+            Err(ShellError::unexpected(&format!(
+                "Variable {} not set",
+                &var.item
+            )))
         }
     } else {
         Err(ShellError::labeled_error(
diff --git a/crates/nu-command/src/commands/pathvar/command.rs b/crates/nu-command/src/commands/pathvar/command.rs
index 7d3474f918..74d84eff48 100644
--- a/crates/nu-command/src/commands/pathvar/command.rs
+++ b/crates/nu-command/src/commands/pathvar/command.rs
@@ -1,8 +1,9 @@
+use super::get_var;
 use crate::prelude::*;
 use nu_engine::WholeStreamCommand;
 use nu_errors::ShellError;
-use nu_protocol::{Signature, Value};
-use nu_test_support::{NATIVE_PATH_ENV_SEPARATOR, NATIVE_PATH_ENV_VAR};
+use nu_protocol::{Signature, SyntaxShape, Value};
+use nu_test_support::NATIVE_PATH_ENV_SEPARATOR;
 
 pub struct Command;
 
@@ -12,11 +13,17 @@ impl WholeStreamCommand for Command {
     }
 
     fn signature(&self) -> Signature {
-        Signature::build("pathvar")
+        Signature::build("pathvar").named(
+            "var",
+            SyntaxShape::String,
+            "Use a different variable than PATH",
+            Some('v'),
+        )
     }
 
     fn usage(&self) -> &str {
-        "Manipulate the PATH variable (or pathvar)."
+        r#"Manipulate the PATH variable (pathvar) or a different variable following the
+same rules."#
     }
 
     fn run(&self, args: CommandArgs) -> Result<OutputStream, ShellError> {
@@ -30,6 +37,11 @@ impl WholeStreamCommand for Command {
                 example: "pathvar",
                 result: None,
             },
+            Example {
+                description: "Display the current session's LD_LIBRARY_PATH",
+                example: "pathvar -v LD_LIBRARY_PATH",
+                result: None,
+            },
             Example {
                 description: "Add /usr/bin to the pathvar",
                 example: "pathvar add /usr/bin",
@@ -45,7 +57,9 @@ impl WholeStreamCommand for Command {
 }
 
 pub fn get_pathvar(args: CommandArgs) -> Result<OutputStream, ShellError> {
-    if let Some(pathvar) = args.context.scope.get_env(NATIVE_PATH_ENV_VAR) {
+    let var = get_var(&args)?;
+
+    if let Some(pathvar) = args.context.scope.get_env(&var) {
         let pathvar: Vec<Value> = pathvar
             .split(NATIVE_PATH_ENV_SEPARATOR)
             .map(Value::from)
@@ -53,6 +67,9 @@ pub fn get_pathvar(args: CommandArgs) -> Result<OutputStream, ShellError> {
 
         Ok(OutputStream::from(pathvar))
     } else {
-        Err(ShellError::unexpected("PATH not set"))
+        Err(ShellError::unexpected(&format!(
+            "Variable {} not set",
+            &var.item
+        )))
     }
 }
diff --git a/crates/nu-command/src/commands/pathvar/mod.rs b/crates/nu-command/src/commands/pathvar/mod.rs
index 6191bf1a83..9825ccefdb 100644
--- a/crates/nu-command/src/commands/pathvar/mod.rs
+++ b/crates/nu-command/src/commands/pathvar/mod.rs
@@ -11,3 +11,15 @@ pub use command::Command as Pathvar;
 pub use remove::SubCommand as PathvarRemove;
 pub use reset::SubCommand as PathvarReset;
 pub use save::SubCommand as PathvarSave;
+
+use nu_engine::CommandArgs;
+use nu_errors::ShellError;
+use nu_source::{Tagged, TaggedItem};
+use nu_test_support::NATIVE_PATH_ENV_VAR;
+
+fn get_var(args: &CommandArgs) -> Result<Tagged<String>, ShellError> {
+    Ok(args
+        .get_flag("var")?
+        .unwrap_or_else(|| String::from(NATIVE_PATH_ENV_VAR))
+        .tagged_unknown())
+}
diff --git a/crates/nu-command/src/commands/pathvar/remove.rs b/crates/nu-command/src/commands/pathvar/remove.rs
index c091b45587..035004843b 100644
--- a/crates/nu-command/src/commands/pathvar/remove.rs
+++ b/crates/nu-command/src/commands/pathvar/remove.rs
@@ -1,9 +1,10 @@
+use super::get_var;
 use crate::prelude::*;
 use nu_engine::WholeStreamCommand;
 use nu_errors::ShellError;
 use nu_protocol::{Signature, SyntaxShape};
 use nu_source::Tagged;
-use nu_test_support::{NATIVE_PATH_ENV_SEPARATOR, NATIVE_PATH_ENV_VAR};
+use nu_test_support::NATIVE_PATH_ENV_SEPARATOR;
 
 pub struct SubCommand;
 
@@ -13,11 +14,18 @@ impl WholeStreamCommand for SubCommand {
     }
 
     fn signature(&self) -> Signature {
-        Signature::build("pathvar remove").required(
-            "index",
-            SyntaxShape::Int,
-            "index of the path to remove (starting at 0)",
-        )
+        Signature::build("pathvar remove")
+            .required(
+                "index",
+                SyntaxShape::Int,
+                "index of the path to remove (starting at 0)",
+            )
+            .named(
+                "var",
+                SyntaxShape::String,
+                "Use a different variable than PATH",
+                Some('v'),
+            )
     }
 
     fn usage(&self) -> &str {
@@ -39,10 +47,12 @@ impl WholeStreamCommand for SubCommand {
 
 pub fn remove(args: CommandArgs) -> Result<OutputStream, ShellError> {
     let ctx = &args.context;
+
+    let var = get_var(&args)?;
     let index_to_remove_arg: Tagged<u64> = args.req(0)?;
     let index_to_remove = index_to_remove_arg.item as usize;
 
-    if let Some(old_pathvar) = ctx.scope.get_env(NATIVE_PATH_ENV_VAR) {
+    if let Some(old_pathvar) = ctx.scope.get_env(&var) {
         let mut paths: Vec<&str> = old_pathvar.split(NATIVE_PATH_ENV_SEPARATOR).collect();
 
         if index_to_remove >= paths.len() {
@@ -55,12 +65,15 @@ pub fn remove(args: CommandArgs) -> Result<OutputStream, ShellError> {
 
         paths.remove(index_to_remove);
         ctx.scope.add_env_var(
-            NATIVE_PATH_ENV_VAR,
+            &var.item,
             paths.join(&NATIVE_PATH_ENV_SEPARATOR.to_string()),
         );
 
         Ok(OutputStream::empty())
     } else {
-        Err(ShellError::unexpected("PATH not set"))
+        Err(ShellError::unexpected(&format!(
+            "Variable {} not set",
+            &var.item
+        )))
     }
 }
diff --git a/crates/nu-command/src/commands/pathvar/reset.rs b/crates/nu-command/src/commands/pathvar/reset.rs
index 34781e8e62..e8038e13ff 100644
--- a/crates/nu-command/src/commands/pathvar/reset.rs
+++ b/crates/nu-command/src/commands/pathvar/reset.rs
@@ -1,8 +1,9 @@
+use super::get_var;
 use crate::prelude::*;
 use nu_engine::WholeStreamCommand;
 use nu_errors::ShellError;
-use nu_protocol::{Signature, UntaggedValue};
-use nu_test_support::{NATIVE_PATH_ENV_SEPARATOR, NATIVE_PATH_ENV_VAR};
+use nu_protocol::{Signature, SyntaxShape, UntaggedValue};
+use nu_test_support::NATIVE_PATH_ENV_SEPARATOR;
 
 pub struct SubCommand;
 
@@ -12,7 +13,12 @@ impl WholeStreamCommand for SubCommand {
     }
 
     fn signature(&self) -> Signature {
-        Signature::build("pathvar reset")
+        Signature::build("pathvar reset").named(
+            "var",
+            SyntaxShape::String,
+            "Use a different variable than PATH",
+            Some('v'),
+        )
     }
 
     fn usage(&self) -> &str {
@@ -23,24 +29,29 @@ impl WholeStreamCommand for SubCommand {
         reset(args)
     }
 }
+
 pub fn reset(args: CommandArgs) -> Result<OutputStream, ShellError> {
     let name = args.call_info.name_tag.clone();
     let ctx = &args.context;
 
+    let var = get_var(&args)?;
+    let var_lower = var.clone().map(|s| s.to_lowercase());
+
     if let Some(global_cfg) = &mut ctx.configs().lock().global_config {
-        let default_pathvar = global_cfg.vars.get("path");
+        let default_pathvar = global_cfg.vars.get(&var_lower.item);
         if let Some(pathvar) = default_pathvar {
             if let UntaggedValue::Table(paths) = &pathvar.value {
                 let pathvar_str = paths
                     .iter()
                     .map(|x| x.as_string().expect("Error converting path to string"))
                     .join(&NATIVE_PATH_ENV_SEPARATOR.to_string());
-                ctx.scope.add_env_var(NATIVE_PATH_ENV_VAR, pathvar_str);
+                ctx.scope.add_env_var(&var.item, pathvar_str);
             }
         } else {
-            return Err(ShellError::untagged_runtime_error(
-                "Default path is not set in config file.",
-            ));
+            return Err(ShellError::untagged_runtime_error(&format!(
+                "Default {} is not set in config file.",
+                &var_lower.item
+            )));
         }
         Ok(OutputStream::empty())
     } else {
diff --git a/crates/nu-command/src/commands/pathvar/save.rs b/crates/nu-command/src/commands/pathvar/save.rs
index 7b25823de0..5684019a2e 100644
--- a/crates/nu-command/src/commands/pathvar/save.rs
+++ b/crates/nu-command/src/commands/pathvar/save.rs
@@ -1,8 +1,9 @@
+use super::get_var;
 use crate::prelude::*;
 use nu_engine::WholeStreamCommand;
 use nu_errors::ShellError;
-use nu_protocol::{Signature, UntaggedValue, Value};
-use nu_test_support::{NATIVE_PATH_ENV_SEPARATOR, NATIVE_PATH_ENV_VAR};
+use nu_protocol::{Signature, SyntaxShape, UntaggedValue, Value};
+use nu_test_support::NATIVE_PATH_ENV_SEPARATOR;
 
 pub struct SubCommand;
 
@@ -12,7 +13,12 @@ impl WholeStreamCommand for SubCommand {
     }
 
     fn signature(&self) -> Signature {
-        Signature::build("pathvar save")
+        Signature::build("pathvar save").named(
+            "var",
+            SyntaxShape::String,
+            "Use a different variable than PATH",
+            Some('v'),
+        )
     }
 
     fn usage(&self) -> &str {
@@ -27,8 +33,11 @@ pub fn save(args: CommandArgs) -> Result<OutputStream, ShellError> {
     let name = args.call_info.name_tag.clone();
     let ctx = &args.context;
 
+    let var = get_var(&args)?;
+    let var_lower = var.clone().map(|s| s.to_lowercase());
+
     if let Some(global_cfg) = &mut ctx.configs().lock().global_config {
-        if let Some(pathvar) = ctx.scope.get_env(NATIVE_PATH_ENV_VAR) {
+        if let Some(pathvar) = ctx.scope.get_env(&var) {
             let paths: Vec<Value> = pathvar
                 .split(NATIVE_PATH_ENV_SEPARATOR)
                 .map(Value::from)
@@ -40,13 +49,16 @@ pub fn save(args: CommandArgs) -> Result<OutputStream, ShellError> {
                 Tag::from(Span::from(&span_range)),
             );
 
-            global_cfg.vars.insert("path".to_string(), row);
+            global_cfg.vars.insert(var_lower.item, row);
             global_cfg.write()?;
             ctx.reload_config(global_cfg)?;
 
             Ok(OutputStream::empty())
         } else {
-            Err(ShellError::unexpected("PATH not set"))
+            Err(ShellError::unexpected(&format!(
+                "Variable {} not set",
+                &var.item
+            )))
         }
     } else {
         let value = UntaggedValue::Error(crate::commands::config::err_no_global_cfg_present())
diff --git a/crates/nu-command/tests/commands/mod.rs b/crates/nu-command/tests/commands/mod.rs
index ebf4295c73..c1c7c77188 100644
--- a/crates/nu-command/tests/commands/mod.rs
+++ b/crates/nu-command/tests/commands/mod.rs
@@ -37,6 +37,7 @@ mod move_;
 mod open;
 mod parse;
 mod path;
+mod pathvar;
 mod prepend;
 mod random;
 mod range;
diff --git a/crates/nu-command/tests/commands/pathvar/mod.rs b/crates/nu-command/tests/commands/pathvar/mod.rs
new file mode 100644
index 0000000000..653be916a6
--- /dev/null
+++ b/crates/nu-command/tests/commands/pathvar/mod.rs
@@ -0,0 +1,380 @@
+use nu_test_support::fs::Stub::FileWithContent;
+use nu_test_support::fs::{AbsolutePath, DisplayPath};
+use nu_test_support::playground::{says, Playground};
+use nu_test_support::{nu, NATIVE_PATH_ENV_SEPARATOR};
+
+use std::path::PathBuf;
+
+use hamcrest2::assert_that;
+use hamcrest2::prelude::*;
+
+/// Helper function that joins string literals with ':' or ';', based on host OS
+fn join_env_sep(pieces: &[&str]) -> String {
+    let sep_string = String::from(NATIVE_PATH_ENV_SEPARATOR);
+    pieces.join(&sep_string)
+}
+
+// Helpers
+
+#[cfg(windows)]
+#[test]
+fn joins_env_on_windows() {
+    let pieces = ["sausage", "bacon", "spam"];
+    let actual = join_env_sep(&pieces);
+
+    assert_eq!(&actual, "sausage;bacon;spam");
+}
+
+#[cfg(not(windows))]
+#[test]
+fn joins_env_on_non_windows() {
+    let pieces = ["sausage", "bacon", "spam"];
+    let actual = join_env_sep(&pieces);
+
+    assert_eq!(&actual, "sausage:bacon:spam");
+}
+
+// The following tests don't work on Windows likely because of this issue:
+//   https://github.com/nushell/nushell/issues/3831
+//
+// fn pathvar_correctly_reads_path_from_config_and_env()
+// fn pathvar_correctly_reads_path_from_env()
+// fn pathvar_saves_new_path_to_config()
+//
+// Requires also auto-reading other-than-path env vars from config:
+// fn pathvar_correctly_reads_env_var_from_config_and_env()
+// fn pathvar_correctly_reads_env_var_from_config()
+
+// pathvar
+
+#[test]
+fn pathvar_correctly_reads_path_from_config() {
+    Playground::setup("hi_there", |dirs, sandbox| {
+        let file = AbsolutePath::new(dirs.test().join("config.toml"));
+
+        sandbox
+            .with_files(vec![FileWithContent(
+                "config.toml",
+                r#"
+                    skip_welcome_message = true
+
+                    path = ["/Users/andresrobalino/.volta/bin", "/Users/mosqueteros/bin"]
+                "#,
+            )])
+            .with_config(&file);
+
+        let expected = "/Users/andresrobalino/.volta/bin-/Users/mosqueteros/bin";
+        let actual = sandbox.pipeline(r#" pathvar | first 2 | str collect '-' "#);
+
+        assert_that!(actual, says().stdout(&expected));
+    })
+}
+
+#[test]
+fn pathvar_correctly_reads_env_var_from_env() {
+    Playground::setup("hi_there", |_, sandbox| {
+        sandbox.with_env("BREAKFAST", &join_env_sep(&["bacon", "spam"]));
+
+        let expected = "bacon-spam";
+        let actual = sandbox.pipeline(r#" pathvar -v BREAKFAST | str collect '-' "#);
+
+        assert_that!(actual, says().stdout(&expected));
+    })
+}
+
+// pathvar add
+
+#[test]
+fn pathvar_adds_to_path() {
+    Playground::setup("hi_there", |dirs, sandbox| {
+        let file = AbsolutePath::new(dirs.test().join("config.toml"));
+
+        sandbox
+            .with_files(vec![FileWithContent(
+                "config.toml",
+                r#"
+                    skip_welcome_message = true
+
+                    path = ["/Users/mosquito/proboscis"]
+                "#,
+            )])
+            .with_config(&file);
+
+        let expected = "spam";
+        let actual = sandbox.pipeline(r#" pathvar add spam; pathvar | first "#);
+
+        assert_that!(actual, says().stdout(&expected));
+    })
+}
+
+#[test]
+fn pathvar_adds_to_env_var() {
+    Playground::setup("hi_there", |_, sandbox| {
+        sandbox.with_env("BREAKFAST", &join_env_sep(&["egg", "sausage", "bacon"]));
+
+        let expected = join_env_sep(&["spam", "egg", "sausage", "bacon"]);
+        let actual = sandbox.pipeline(
+            r#" 
+                pathvar add -v BREAKFAST spam
+                $nu.env.BREAKFAST
+            "#,
+        );
+
+        assert_that!(actual, says().stdout(&expected));
+    })
+}
+
+// pathvar append
+
+#[test]
+fn pathvar_appends_to_path() {
+    Playground::setup("hi_there", |dirs, sandbox| {
+        let file = AbsolutePath::new(dirs.test().join("config.toml"));
+
+        sandbox
+            .with_files(vec![FileWithContent(
+                "config.toml",
+                r#"
+                    skip_welcome_message = true
+
+                    path = ["/Users/mosquito/proboscis"]
+                "#,
+            )])
+            .with_config(&file);
+
+        let expected = "spam";
+        let actual = sandbox.pipeline(r#" pathvar append spam; pathvar | last "#);
+
+        assert_that!(actual, says().stdout(&expected));
+    })
+}
+
+#[test]
+fn pathvar_appends_to_env_var() {
+    Playground::setup("hi_there", |_, sandbox| {
+        sandbox.with_env("BREAKFAST", &join_env_sep(&["egg", "sausage", "bacon"]));
+
+        let expected = join_env_sep(&["egg", "sausage", "bacon", "spam"]);
+        let actual = sandbox.pipeline(
+            r#" 
+                pathvar append -v BREAKFAST spam
+                $nu.env.BREAKFAST
+            "#,
+        );
+
+        assert_that!(actual, says().stdout(&expected));
+    })
+}
+
+// pathvar remove
+
+#[test]
+fn pathvar_removes_from_path() {
+    Playground::setup("hi_there", |dirs, sandbox| {
+        let file = AbsolutePath::new(dirs.test().join("config.toml"));
+
+        sandbox
+            .with_files(vec![FileWithContent(
+                "config.toml",
+                r#"
+                    skip_welcome_message = true
+
+                    path = ["/Users/mosquito/proboscis", "spam"]
+                "#,
+            )])
+            .with_config(&file);
+
+        let expected = "/Users/mosquito/proboscis";
+        let actual = sandbox.pipeline(r#" pathvar remove 1; pathvar | first "#);
+
+        assert_that!(actual, says().stdout(&expected));
+    })
+}
+
+#[test]
+fn pathvar_removes_from_env_var() {
+    Playground::setup("hi_there", |_, sandbox| {
+        sandbox.with_env(
+            "BREAKFAST",
+            &join_env_sep(&["egg", "sausage", "bacon", "spam"]),
+        );
+
+        let expected = join_env_sep(&["egg", "sausage", "bacon"]);
+        let actual = sandbox.pipeline(
+            r#" 
+                pathvar remove -v BREAKFAST 3
+                $nu.env.BREAKFAST
+            "#,
+        );
+
+        assert_that!(actual, says().stdout(&expected));
+    })
+}
+
+// pathvar reset
+
+#[test]
+fn pathvar_resets_path_from_config() {
+    Playground::setup("hi_there", |dirs, sandbox| {
+        let file = AbsolutePath::new(dirs.test().join("config.toml"));
+
+        sandbox
+            .with_files(vec![FileWithContent(
+                "config.toml",
+                r#"
+                    skip_welcome_message = true
+
+                    path = ["/Users/andresrobalino/.volta/bin", "/Users/mosqueteros/bin"]
+                "#,
+            )])
+            .with_config(&file)
+            .with_env(
+                nu_test_support::NATIVE_PATH_ENV_VAR,
+                &PathBuf::from("/Users/mosquito/proboscis").display_path(),
+            );
+
+        let expected = "/Users/andresrobalino/.volta/bin-/Users/mosqueteros/bin";
+        let actual = sandbox.pipeline(
+            r#"
+                pathvar reset
+                pathvar | str collect '-'
+            "#,
+        );
+
+        assert_that!(actual, says().stdout(&expected));
+    })
+}
+
+#[test]
+fn pathvar_resets_env_var_from_config() {
+    Playground::setup("hi_there", |dirs, sandbox| {
+        let file = AbsolutePath::new(dirs.test().join("config.toml"));
+
+        sandbox
+            .with_files(vec![FileWithContent(
+                "config.toml",
+                r#"
+                    skip_welcome_message = true
+
+                    breakfast = ["egg", "sausage", "bacon"]
+                "#,
+            )])
+            .with_config(&file)
+            .with_env(
+                "BREAKFAST",
+                &join_env_sep(&["egg", "sausage", "bacon", "spam"]),
+            );
+
+        let expected = "egg-sausage-bacon";
+        let actual = sandbox.pipeline(
+            r#"
+                pathvar reset -v BREAKFAST
+                pathvar -v BREAKFAST | str collect '-'
+            "#,
+        );
+
+        assert_that!(actual, says().stdout(&expected));
+    })
+}
+
+// pathvar save
+
+#[test]
+fn pathvar_saves_path_to_config() {
+    Playground::setup("hi_there", |dirs, sandbox| {
+        let file = AbsolutePath::new(dirs.test().join("config.toml"));
+
+        sandbox
+            .with_files(vec![FileWithContent(
+                "config.toml",
+                r#"
+                    skip_welcome_message = true
+
+                    path = ["/Users/andresrobalino/.volta/bin", "/Users/mosqueteros/bin"]
+                "#,
+            )])
+            .with_config(&file);
+
+        let expected = "/Users/mosquito/proboscis";
+        let actual = sandbox.pipeline(
+            r#"
+                pathvar append "/Users/mosquito/proboscis"
+                pathvar save
+                (config).path | last
+            "#,
+        );
+
+        assert_that!(actual, says().stdout(&expected));
+    })
+}
+
+#[test]
+fn pathvar_saves_env_var_to_config() {
+    Playground::setup("hi_there", |dirs, sandbox| {
+        let file = AbsolutePath::new(dirs.test().join("config.toml"));
+
+        sandbox
+            .with_files(vec![FileWithContent(
+                "config.toml",
+                r#"
+                    skip_welcome_message = true
+
+                    breakfast = ["egg", "sausage", "bacon"]
+                "#,
+            )])
+            .with_config(&file)
+            .with_env("BREAKFAST", "spam");
+
+        let expected = "spam";
+        let actual = sandbox.pipeline(
+            r#"
+                pathvar save -v BREAKFAST
+                (config).breakfast | str collect '-'
+            "#,
+        );
+
+        assert_that!(actual, says().stdout(&expected));
+    })
+}
+
+#[test]
+fn pathvar_saves_new_env_var_to_config() {
+    Playground::setup("hi_there", |dirs, sandbox| {
+        let file = AbsolutePath::new(dirs.test().join("config.toml"));
+
+        sandbox
+            .with_files(vec![FileWithContent(
+                "config.toml",
+                r#"
+                    skip_welcome_message = true
+                "#,
+            )])
+            .with_config(&file)
+            .with_env("BREAKFAST", "spam");
+
+        let expected = "spam";
+        let actual = sandbox.pipeline(
+            r#"
+                pathvar save -v BREAKFAST
+                (config).breakfast | str collect '-'
+            "#,
+        );
+
+        assert_that!(actual, says().stdout(&expected));
+    })
+}
+
+// test some errors
+
+#[test]
+fn pathvar_error_non_existent_env_var() {
+    Playground::setup("hi_there", |dirs, _| {
+        let actual = nu!(
+            cwd: dirs.test(),
+            "pathvar -v EGGS_BACON_SPAM_SAUSAGE_SPAM_AND_SPAM_WITH_EXTRA_SPAM"
+        );
+
+        assert!(actual.err.contains("Error"));
+        assert!(actual.err.contains("not set"));
+    })
+}