Treating environment variables as Values (#497)

* Proof of concept treating env vars as Values

* Refactor env var collection and method name

* Remove unnecessary pub

* Move env translations into a new file

* Fix LS_COLORS to support any Value

* Fix spans during env var translation

* Add span to env var in cd

* Improve error diagnostics

* Fix non-string env vars failing string conversion

* Make PROMPT_COMMAND a Block instead of String

* Record host env vars to a fake file

This will give spans to env vars that would otherwise be without one.
Makes errors less confusing.

* Add 'env' command to list env vars

It will list also their values translated to strings

* Sort env command by name; Add env var type

* Remove obsolete test

crates/nu-command/src/core_commands/use_.rs

@@ -88,15 +88,8 @@ impl Command for Use {
 
                 // TODO: Add string conversions (e.g. int to string)
                 // TODO: Later expand env to take all Values
-                let val = if let Ok(s) =
-                    eval_block(engine_state, stack, block, PipelineData::new(call.head))?
-                        .into_value(Span::unknown())
-                        .as_string()
-                {
-                    s
-                } else {
-                    return Err(ShellError::EnvVarNotAString(import_pattern.span()));
-                };
+                let val = eval_block(engine_state, stack, block, PipelineData::new(call.head))?
+                    .into_value(Span::unknown());
 
                 stack.add_env_var(name, val);
             }

crates/nu-command/src/default_context.rs

@@ -226,6 +226,7 @@ pub fn create_default_context() -> EngineState {
         bind_command! {
             LetEnv,
             WithEnv,
+            Env,
         };
 
         // Math

crates/nu-command/src/env/env_command.rs

@@ -0,0 +1,61 @@
+use nu_engine::env_to_string;
+use nu_protocol::ast::Call;
+use nu_protocol::engine::{Command, EngineState, Stack};
+use nu_protocol::{Category, IntoPipelineData, PipelineData, Signature, Value};
+
+#[derive(Clone)]
+pub struct Env;
+
+impl Command for Env {
+    fn name(&self) -> &str {
+        "env"
+    }
+
+    fn usage(&self) -> &str {
+        "Display current environment"
+    }
+
+    fn signature(&self) -> nu_protocol::Signature {
+        Signature::build("env").category(Category::Env)
+    }
+
+    fn run(
+        &self,
+        engine_state: &EngineState,
+        stack: &mut Stack,
+        call: &Call,
+        _input: PipelineData,
+    ) -> Result<nu_protocol::PipelineData, nu_protocol::ShellError> {
+        let span = call.head;
+        let config = stack.get_config().unwrap_or_default();
+
+        let mut env_vars: Vec<(String, Value)> = stack.get_env_vars().into_iter().collect();
+        env_vars.sort_by(|(name1, _), (name2, _)| name1.cmp(name2));
+
+        let mut values = vec![];
+
+        for (name, val) in env_vars {
+            let mut cols = vec![];
+            let mut vals = vec![];
+
+            let raw = env_to_string(&name, val.clone(), engine_state, stack, &config)?;
+            let val_type = val.get_type();
+
+            cols.push("name".into());
+            vals.push(Value::string(name, span));
+
+            cols.push("type".into());
+            vals.push(Value::string(format!("{}", val_type), span));
+
+            cols.push("value".into());
+            vals.push(val);
+
+            cols.push("raw".into());
+            vals.push(Value::string(raw, span));
+
+            values.push(Value::Record { cols, vals, span });
+        }
+
+        Ok(Value::List { vals: values, span }.into_pipeline_data())
+    }
+}

crates/nu-command/src/env/let_env.rs

@@ -20,7 +20,7 @@ impl Command for LetEnv {
             .required("var_name", SyntaxShape::String, "variable name")
             .required(
                 "initial_value",
-                SyntaxShape::Keyword(b"=".to_vec(), Box::new(SyntaxShape::String)),
+                SyntaxShape::Keyword(b"=".to_vec(), Box::new(SyntaxShape::Any)),
                 "equals sign followed by value",
             )
             .category(Category::Env)
@@ -42,9 +42,6 @@ impl Command for LetEnv {
             .expect("internal error: missing keyword");
 
         let rhs = eval_expression(engine_state, stack, keyword_expr)?;
-        let rhs = rhs.as_string()?;
-
-        //println!("Adding: {:?} to {}", rhs, var_id);
 
         stack.add_env_var(env_var, rhs);
         Ok(PipelineData::new(call.head))

crates/nu-command/src/env/mod.rs

@@ -1,5 +1,7 @@
+mod env_command;
 mod let_env;
 mod with_env;
 
+pub use env_command::Env;
 pub use let_env::LetEnv;
 pub use with_env::WithEnv;

crates/nu-command/src/env/with_env.rs

@@ -1,7 +1,4 @@
-use std::{
-    collections::HashMap,
-    convert::{TryFrom, TryInto},
-};
+use std::collections::HashMap;
 
 use nu_engine::{eval_block, CallExt};
 use nu_protocol::{
@@ -73,34 +70,6 @@ impl Command for WithEnv {
     }
 }
 
-#[derive(Debug, Clone)]
-pub enum EnvVar {
-    Proper(String),
-    Nothing,
-}
-
-impl TryFrom<&Value> for EnvVar {
-    type Error = ShellError;
-
-    fn try_from(value: &Value) -> Result<Self, Self::Error> {
-        if matches!(value, Value::Nothing { .. }) {
-            Ok(EnvVar::Nothing)
-        } else if let Ok(s) = value.as_string() {
-            if s.is_empty() {
-                Ok(EnvVar::Nothing)
-            } else {
-                Ok(EnvVar::Proper(s))
-            }
-        } else {
-            Err(ShellError::CantConvert(
-                "string".into(),
-                value.get_type().to_string(),
-                value.span()?,
-            ))
-        }
-    }
-}
-
 fn with_env(
     engine_state: &EngineState,
     stack: &mut Stack,
@@ -116,7 +85,7 @@ fn with_env(
     let block = engine_state.get_block(block_id).clone();
     let mut stack = stack.collect_captures(&block.captures);
 
-    let mut env: HashMap<String, EnvVar> = HashMap::new();
+    let mut env: HashMap<String, Value> = HashMap::new();
 
     match &variable {
         Value::List { vals: table, .. } => {
@@ -125,7 +94,7 @@ fn with_env(
                 match &table[0] {
                     Value::Record { cols, vals, .. } => {
                         for (k, v) in cols.iter().zip(vals.iter()) {
-                            env.insert(k.to_string(), v.try_into()?);
+                            env.insert(k.to_string(), v.clone());
                         }
                     }
                     x => {
@@ -140,15 +109,16 @@ fn with_env(
                 // primitive values([X Y W Z])
                 for row in table.chunks(2) {
                     if row.len() == 2 {
-                        env.insert(row[0].as_string()?, (&row[1]).try_into()?);
+                        env.insert(row[0].as_string()?, (&row[1]).clone());
                     }
+                    // TODO: else error?
                 }
             }
         }
         // when get object by `open x.json` or `from json`
         Value::Record { cols, vals, .. } => {
             for (k, v) in cols.iter().zip(vals) {
-                env.insert(k.clone(), v.try_into()?);
+                env.insert(k.clone(), v.clone());
             }
         }
         x => {
@@ -161,14 +131,7 @@ fn with_env(
     };
 
     for (k, v) in env {
-        match v {
-            EnvVar::Nothing => {
-                stack.remove_env_var(&k);
-            }
-            EnvVar::Proper(s) => {
-                stack.add_env_var(k, s);
-            }
-        }
+        stack.add_env_var(k, v);
     }
 
     eval_block(engine_state, &mut stack, &block, input)

crates/nu-command/src/filesystem/cd.rs

@@ -1,7 +1,7 @@
 use nu_engine::CallExt;
 use nu_protocol::ast::Call;
 use nu_protocol::engine::{Command, EngineState, Stack};
-use nu_protocol::{Category, PipelineData, Signature, SyntaxShape};
+use nu_protocol::{Category, PipelineData, Signature, SyntaxShape, Value};
 
 #[derive(Clone)]
 pub struct Cd;
@@ -28,23 +28,23 @@ impl Command for Cd {
         call: &Call,
         _input: PipelineData,
     ) -> Result<nu_protocol::PipelineData, nu_protocol::ShellError> {
-        let path: Option<String> = call.opt(engine_state, stack, 0)?;
+        let path_val: Option<Value> = call.opt(engine_state, stack, 0)?;
 
-        let path = match path {
-            Some(path) => {
-                let path = nu_path::expand_path(path);
-                path.to_string_lossy().to_string()
+        let (path, span) = match path_val {
+            Some(v) => {
+                let path = nu_path::expand_path(v.as_string()?);
+                (path.to_string_lossy().to_string(), v.span()?)
             }
             None => {
                 let path = nu_path::expand_tilde("~");
-                path.to_string_lossy().to_string()
+                (path.to_string_lossy().to_string(), call.head)
             }
         };
         let _ = std::env::set_current_dir(&path);
 
         //FIXME: this only changes the current scope, but instead this environment variable
         //should probably be a block that loads the information from the state in the overlay
-        stack.add_env_var("PWD".into(), path);
+        stack.add_env_var("PWD".into(), Value::String { val: path, span });
         Ok(PipelineData::new(call.head))
     }
 }

crates/nu-command/src/system/run_external.rs

@@ -7,6 +7,7 @@ use std::process::{Command as CommandSys, Stdio};
 use std::sync::atomic::Ordering;
 use std::sync::mpsc;
 
+use nu_engine::env_to_strings;
 use nu_protocol::engine::{EngineState, Stack};
 use nu_protocol::{ast::Call, engine::Command, ShellError, Signature, SyntaxShape, Value};
 use nu_protocol::{Category, Config, IntoInterruptiblePipelineData, PipelineData, Span, Spanned};
@@ -51,9 +52,10 @@ impl Command for External {
         let mut name: Spanned<String> = call.req(engine_state, stack, 0)?;
         let args: Vec<String> = call.rest(engine_state, stack, 1)?;
         let last_expression = call.has_flag("last_expression");
-        let env_vars = stack.get_env_vars();
 
+        // Translate environment variables from Values to Strings
         let config = stack.get_config().unwrap_or_default();
+        let env_vars_str = env_to_strings(engine_state, stack, &config)?;
 
         // Check if this is a single call to a directory, if so auto-cd
         let path = nu_path::expand_path(&name.item);
@@ -73,7 +75,13 @@ impl Command for External {
 
             //FIXME: this only changes the current scope, but instead this environment variable
             //should probably be a block that loads the information from the state in the overlay
-            stack.add_env_var("PWD".into(), name.item.clone());
+            stack.add_env_var(
+                "PWD".into(),
+                Value::String {
+                    val: name.item.clone(),
+                    span: Span::unknown(),
+                },
+            );
             return Ok(PipelineData::new(call.head));
         }
 
@@ -81,7 +89,7 @@ impl Command for External {
             name,
             args,
             last_expression,
-            env_vars,
+            env_vars: env_vars_str,
             call,
         };
         command.run_with_input(engine_state, input, config)

crates/nu-command/src/viewers/griddle.rs

@@ -1,6 +1,7 @@
 // use super::icons::{icon_for_file, iconify_style_ansi_to_nu};
 use super::icons::icon_for_file;
 use lscolors::{LsColors, Style};
+use nu_engine::env_to_string;
 use nu_engine::CallExt;
 use nu_protocol::{
     ast::{Call, PathMember},
@@ -61,7 +62,10 @@ prints out the list properly."#
         let color_param: bool = call.has_flag("color");
         let separator_param: Option<String> = call.get_flag(engine_state, stack, "separator")?;
         let config = stack.get_config().unwrap_or_default();
-        let env_str = stack.get_env_var("LS_COLORS");
+        let env_str = match stack.get_env_var("LS_COLORS") {
+            Some(v) => Some(env_to_string("LS_COLORS", v, engine_state, stack, &config)?),
+            None => None,
+        };
         let use_grid_icons = config.use_grid_icons;
 
         match input {

crates/nu-command/src/viewers/table.rs

@@ -1,5 +1,6 @@
 use lscolors::{LsColors, Style};
 use nu_color_config::{get_color_config, style_primitive};
+use nu_engine::env_to_string;
 use nu_protocol::ast::{Call, PathMember};
 use nu_protocol::engine::{Command, EngineState, Stack};
 use nu_protocol::{
@@ -74,7 +75,13 @@ impl Command for Table {
                         let ctrlc = ctrlc.clone();
 
                         let ls_colors = match stack.get_env_var("LS_COLORS") {
-                            Some(s) => LsColors::from_string(&s),
+                            Some(v) => LsColors::from_string(&env_to_string(
+                                "LS_COLORS",
+                                v,
+                                engine_state,
+                                stack,
+                                &config,
+                            )?),
                             None => LsColors::default(),
                         };