Error msgs improved, cli light vals and enf defaults use static syntax, to allow coersion

py_rust/Cargo.toml

@@ -44,6 +44,7 @@ minijinja = { version = '1.0.10', features = [
   'preserve_order',
   'json',
   'urlencode',
+  'debug',
 ] }
 minijinja-contrib = { version = '1.0.10', features = ['datetime'] }
 

py_rust/src/config/context.rs

@@ -23,7 +23,7 @@ impl CtxStaticVar {
 #[derive(Clone, Debug, Deserialize, Serialize)]
 pub struct CtxEnvVar {
     pub env_name: Option<String>,
-    pub default: Option<serde_json::Value>,
+    pub default: Option<CtxStaticVar>,
     pub coerce: Option<Coerce>,
 }
 
@@ -45,7 +45,7 @@ impl CtxEnvVar {
                     ));
                 } else {
                     match &self.default {
-                        Some(value) => return Ok(value.clone()),
+                        Some(value) => return value.read(),
                         None => {
                             return Err(zerr!(
                                 Zerr::ContextLoadError,
@@ -66,7 +66,7 @@ impl CtxEnvVar {
 pub struct CtxCliVar {
     pub commands: Vec<String>,
     pub coerce: Option<Coerce>,
-    pub light: Option<serde_json::Value>,
+    pub light: Option<CtxStaticVar>,
 }
 
 impl CtxCliVar {

py_rust/src/config/schema.json

@@ -92,21 +92,7 @@
                 "static": {
                     "description": "Statically configured global variables.",
                     "patternProperties": {
-                        "^.*$": {
-                            "type": "object",
-                            "properties": {
-                                "value": {
-                                    "description": "The value of the variable. Can be any valid toml value."
-                                },
-                                "coerce": {
-                                    "type": "string",
-                                    "description": "The type to coerce the value to. If not specified, the value kept as defined in the toml.",
-                                    "enum": ["json", "str", "int", "float", "bool"]
-                                }
-                            },
-                            "required": ["value"],
-                            "additionalProperties": false
-                        }
+                        "^.*$": { "$ref": "#/$defs/static_value" }
                     },
                     "additionalProperties": false
                 },
@@ -121,7 +107,8 @@
                                     "description": "The name of the environment variable to load into this context var, this defaults to the name of the config var."
                                 },
                                 "default": {
-                                    "description": "The default value of the variable if the environment variable is not set."
+                                    "description": "The default value of the variable if the environment variable is not set.",
+                                    "$ref": "#/$defs/static_value"
                                 },
                                 "coerce": {
                                     "type": "string",
@@ -149,7 +136,8 @@
                                     "minItems": 1
                                 },
                                 "light": {
-                                    "description": "The value to use when in rendering in --light or --superlight mode. If not set, the var will be treated as an empty string."
+                                    "description": "The value to use when in rendering in --light or --superlight mode. If not set, the var will be treated as an empty string.",
+                                    "$ref": "#/$defs/static_value"
                                 },
                                 "coerce": {
                                     "type": "string",
@@ -183,6 +171,21 @@
                 }
             },
             "additionalProperties": false
+        },
+        "static_value": {
+            "type": "object",
+            "properties": {
+                "value": {
+                    "description": "The value of the variable. Can be any valid toml value."
+                },
+                "coerce": {
+                    "type": "string",
+                    "description": "The type to coerce the value to. If not specified, the value kept as defined in the toml.",
+                    "enum": ["json", "str", "int", "float", "bool"]
+                }
+            },
+            "required": ["value"],
+            "additionalProperties": false
         }
     }
 }

py_rust/src/config/validate.rs

@@ -61,17 +61,6 @@ pub fn post_validate(conf: &mut Config, config_path: &Path) -> Result<(), Zerr>
         }
     }
 
-    // Check stat.value is not empty string, plus same for env.default (if provided):
-    for (key, value) in conf.context.stat.iter() {
-        validate_not_empty_string(format!("[context.static.{}.value]", key), &value.value)?;
-    }
-
-    for (key, value) in conf.context.env.iter() {
-        if let Some(default) = &value.default {
-            validate_not_empty_string(format!("[context.env.{}.default]", key), default)?;
-        }
-    }
-
     // ignore_files and engine.custom_extensions should be resolved relative to the config file, so rewrite the paths if needed and make sure they exist:
     let validate_and_rewrite = |in_path: String| -> Result<String, Zerr> {
         // Make relative to config file if not absolute:
@@ -225,19 +214,3 @@ fn run_against_schema(
         .change_context(Zerr::InternalError)?;
     Ok(schema.validate(json))
 }
-
-fn validate_not_empty_string(context: String, value: &serde_json::Value) -> Result<(), Zerr> {
-    let valid = match &value {
-        serde_json::Value::String(s) => !s.trim().is_empty(),
-        _ => true,
-    };
-    if valid {
-        Ok(())
-    } else {
-        Err(zerr!(
-            Zerr::ConfigInvalid,
-            "{}: Cannot be an empty string.",
-            context
-        ))
-    }
-}

py_rust/src/init.rs

@@ -95,7 +95,7 @@ custom_extensions = []
 FOO = {{ value = "foo" }}
 
 [context.env]
-BAR = {{ default = "bar" }}
+BAR = {{ default = {{ value = "bar" }} }}
 
 [context.cli]
 BAZ = {{ commands = ["echo 1"], coerce = "int" }}

py_rust/src/render/mini_env.rs

@@ -42,7 +42,7 @@ pub fn new_mini_env<'a>(root: &Path, state: &'a State) -> Result<minijinja::Envi
     }
 
     // Load in custom rust functions:
-    env.add_function("env_default", gen_env_default_fn(state));
+    env.add_function("env_default", gen_env_default_fn(state)?);
 
     // Load in any custom extensions to the PY_USER_FUNCS global:
     let custom_funcs = py_interface::load_custom_exts(&state.conf.engine.custom_extensions, state)?;
@@ -121,16 +121,19 @@ fn custom_loader<'x, P: AsRef<Path> + 'x>(
 
 fn gen_env_default_fn(
     state: &State,
-) -> impl Fn(String) -> core::result::Result<minijinja::Value, minijinja::Error> {
+) -> Result<impl Fn(String) -> core::result::Result<minijinja::Value, minijinja::Error>, Zerr> {
     // Get a simple dict of available env vars to their defaults as minijinja::Value(s):
     let mut env_defaults = HashMap::new();
     for (key, value) in state.conf.context.env.iter() {
-        if let Some(default) = value.default.as_ref() {
-            env_defaults.insert(key.clone(), minijinja::Value::from_serializable(default));
+        if let Some(var) = value.default.as_ref() {
+            env_defaults.insert(
+                key.clone(),
+                minijinja::Value::from_serializable(&var.read()?),
+            );
         }
     }
 
-    move |name: String| match env_defaults.get(&name) {
+    Ok(move |name: String| match env_defaults.get(&name) {
         Some(default) => Ok(default.clone()),
         None => {
             let mut env_keys = env_defaults
@@ -147,5 +150,5 @@ fn gen_env_default_fn(
                     ),
                 ))
         }
-    }
+    })
 }

py_rust/src/render/mod.rs

@@ -23,8 +23,6 @@ pub fn render(args: &crate::args::Args, render_args: &RenderCommand) -> Result<b
         self::lockfile::Lockfile::load(render_args.root.clone(), render_args.force)
     });
 
-    // TODO double prints what's that about
-
     let mut state = State::new(args)?;
     state.load_all_vars()?;
     debug!("State: {:#?}", state);
@@ -154,14 +152,48 @@ fn render_inner(
                         let end_line_no = (err_line_no + 3).min(lines.len());
                         let mut s = String::new();
                         for line_no in start_line_no..(end_line_no + 1) {
+                            // Handle keeping aligned, e.g. line numbers start in single digits but go into double digits:
+                            let extra_indent = " "
+                                .repeat(end_line_no.to_string().len() - line_no.to_string().len());
                             let line = lines[line_no - 1];
                             if line_no == err_line_no {
+                                // If possible, identify the portion of the line causing the error, making it bright red and underlined:
+                                // This will only be known in some cases, e.g. mj exposes it for {{ REE + 1 }} when REE is undefined but not {{ REE }}
+                                let fmtted_line = if let Some(source_range) = e.range() {
+                                    // The range is of the entire template, not the line, so need to normalise it for the line:
+                                    let mut offset = 0;
+                                    for l in lines.iter().take(line_no - 1) {
+                                        offset += l.len() + 1;
+                                    }
+                                    let line_range =
+                                        source_range.start - offset..source_range.end - offset;
+                                    // If line range finishes greater than the length of the line, raise internal error as it's something wrong with this block:
+                                    if line_range.end > line.len() {
+                                        return Err(zerr!(
+                                            Zerr::InternalError,
+                                            "Line range end is greater than line length."
+                                        ));
+                                    }
+                                    format!(
+                                        "{}{}{}",
+                                        &line[..line_range.start],
+                                        &line[line_range.clone()].underline().bright_red(),
+                                        &line[line_range.end..]
+                                    )
+                                } else {
+                                    line.to_string()
+                                };
                                 s.push_str(&format!(
                                     "{}",
-                                    format!("{}| {} <-- ERR\n", line_no, line).red().bold()
+                                    format!(
+                                        "{}{}| {} <-- ERR\n",
+                                        extra_indent, line_no, fmtted_line
+                                    )
+                                    .red()
+                                    .bold()
                                 ));
                             } else {
-                                s.push_str(&format!("{}: {}\n", line_no, line));
+                                s.push_str(&format!("{}{}| {}\n", extra_indent, line_no, line));
                             }
                         }
                         out_e = out_e.attach_printable(s);

py_rust/src/state/active_state.rs

@@ -115,7 +115,7 @@ impl State {
                 // In light mode use the user provided default or an empty string, rather than running a user command:
                 if self.light {
                     if let Some(light_val) = &value.light {
-                        Ok(light_val.clone())
+                        Ok(light_val.read()?)
                     } else {
                         Ok(serde_json::Value::String("".to_string()))
                     }
@@ -246,7 +246,7 @@ impl State {
                     // If light mode, need to use the light user replacement otherwise an empty string: (no need for threads)
                     if self.light {
                         let value = if let Some(light_val) = &var.light {
-                            light_val.clone()
+                            light_val.read()?
                         } else {
                             serde_json::Value::String("".to_string())
                         };

py_rust/tests/helpers/types.py

@@ -6,12 +6,12 @@
 class CliCtx(tp.TypedDict):
     commands: "list[str]"
     coerce: tp.NotRequired[Coerce_T]
-    light: tp.NotRequired[tp.Any]
+    light: tp.NotRequired["StaticCtx"]
 
 
 class EnvCtx(tp.TypedDict):
     env_name: tp.NotRequired[str]
-    default: tp.NotRequired[tp.Any]
+    default: tp.NotRequired["StaticCtx"]
     coerce: tp.NotRequired[Coerce_T]
 
 

py_rust/tests/render/test_ban_defaults.py

@@ -85,8 +85,8 @@ def run():
                         {
                             "context": {
                                 "env": {
-                                    "TEST_RAND_1": {"default": "Hello"},
-                                    "TEST_RAND_2": {"default": "World"},
+                                    "TEST_RAND_1": {"default": {"value": "Hello"}},
+                                    "TEST_RAND_2": {"default": {"value": "World"}},
                                 }
                             }
                         }

py_rust/tests/render/test_custom_extensions.py

@@ -437,7 +437,7 @@ def capitalize(s):
                         "cli": {
                             "var_with_light": {
                                 "commands": ["echo bar"],
-                                "light": "init",
+                                "light": {"value": "init"},
                             }
                         }
                     },

py_rust/tests/render/test_extra_builtins.py

@@ -180,12 +180,12 @@ def now() -> dt.datetime:
     "functions": {
         # Custom to zetch:
         "env_default": {
-            "description": "Load the default context env var, rather than the active one. \n\nE.g. if you have context.env.FOO = { default = 'bar' } and the env variable is set to 'baz':\n\n{{ env_default('FOO') }} -> 'bar'\n{{ FOO }} -> 'baz'.",
+            "description": "Load the default context env var, rather than the active one. \n\nE.g. if you have context.env.FOO = { default = { value = 'bar' } } and the env variable is set to 'baz':\n\n{{ env_default('FOO') }} -> 'bar'\n{{ FOO }} -> 'baz'.",
             "tests": [
                 lambda: {
                     "input": "default: {{ env_default('FOO') }}, actual: {{ FOO }}",
                     "env": {"FOO": "baz"},
-                    "env_ctx": {"FOO": {"default": "bar"}},
+                    "env_ctx": {"FOO": {"default": {"value": "bar"}}},
                     "expected": "default: bar, actual: baz",
                 }
             ],

py_rust/tests/render/test_light_superlight.py

@@ -22,7 +22,7 @@
         ({"context": {"cli": {"VAR": {"commands": ["echo foo"]}}}}, "var: {{ VAR }}", "var: "),
         # Light value should work:
         (
-            {"context": {"cli": {"VAR": {"commands": ["echo foo"], "light": "LIGHT"}}}},
+            {"context": {"cli": {"VAR": {"commands": ["echo foo"], "light": {"value": "LIGHT"}}}}},
             "var: {{ VAR }}",
             "var: LIGHT",
         ),
@@ -31,11 +31,11 @@
             {
                 "context": {
                     "static": {"VAR_STATIC": {"value": "STATIC"}},
-                    "env": {"VAR_ENV": {"default": "ENV"}},
+                    "env": {"VAR_ENV": {"default": {"value": "ENV"}}},
                     "cli": {
                         "VAR": {
                             "commands": ["echo foo"],
-                            "light": "LIGHT",
+                            "light": {"value": "LIGHT"},
                         }
                     },
                 }
@@ -165,7 +165,7 @@ def run(light: bool):
                                     ),
                                 )
                             ],
-                            "light": "LIGHT",
+                            "light": {"value": "LIGHT"},
                         },
                     }
                 }

py_rust/tests/test_config_invalid.py

@@ -86,22 +86,6 @@ def test_incorrect_config():
                     ),
                 )
 
-        # 'value' is empty string/null for static / env 'default':
-        for ctx_type, key_name in [("static", "value"), ("env", "default")]:
-            with pytest.raises(
-                ValueError,
-                match=re.escape(
-                    "[context.{}.FOO.{}]: Cannot be an empty string.".format(ctx_type, key_name)
-                ),
-            ):
-                cli.render(
-                    manager.root_dir,
-                    manager.tmpfile(
-                        "[context]\n[context.{}]\nFOO = {{ {} = '' }}\n".format(ctx_type, key_name),
-                        suffix=".toml",
-                    ),
-                )
-
         # Unknown key for static/env/cli:
         for ctx_key in ["static", "env", "cli"]:
             with pytest.raises(