From 16f3488d201e5b196bc9fd559e2f8626749771e1 Mon Sep 17 00:00:00 2001
From: Olivier <olivier@librepush.net>
Date: Sun, 18 May 2025 10:43:48 +0100
Subject: [PATCH] Allow using non-Some-wrapped values in optional KV attributes

---
 hornbeam_grammar/src/ast.rs        |  5 +++++
 hornbeam_interpreter/src/engine.rs | 27 +++++++++++++------------
 hornbeam_ir/src/ast_to_ir.rs       | 32 ++++++++++++++++++++++++++----
 3 files changed, 47 insertions(+), 17 deletions(-)

diff --git a/hornbeam_grammar/src/ast.rs b/hornbeam_grammar/src/ast.rs
index 8ec63de..e1f44a3 100644
--- a/hornbeam_grammar/src/ast.rs
+++ b/hornbeam_grammar/src/ast.rs
@@ -83,6 +83,11 @@ pub enum MatchBinding {
     /// `None =>`
     UnitVariant { name: IStr },
 
+    /// `$var`
+    /// (A fallback case)
+    /// TODO this is not implemented in the grammar yet, but is used in generated rules
+    Variable { name: IStr },
+
     /// `_ =>`
     Ignore,
 }
diff --git a/hornbeam_interpreter/src/engine.rs b/hornbeam_interpreter/src/engine.rs
index e9cbd70..f49ce14 100644
--- a/hornbeam_interpreter/src/engine.rs
+++ b/hornbeam_interpreter/src/engine.rs
@@ -363,10 +363,10 @@ impl<'a, O: OutputSystem + Send, LS: LocalisationSystem + Sync + Send> Interpret
                                     }
                                 },
                                 _ => {
-                                    warn!(
-                                        "trying to `match` non-reflective vs {name} at {}",
-                                        step.locator
-                                    );
+                                    // warn!(
+                                    //     "trying to `match` non-reflective vs {name} at {}",
+                                    //     step.locator
+                                    // );
                                     continue;
                                 }
                             }
@@ -416,14 +416,19 @@ impl<'a, O: OutputSystem + Send, LS: LocalisationSystem + Sync + Send> Interpret
                                     }
                                 },
                                 _ => {
-                                    warn!(
-                                        "trying to `match` non-reflective vs {name}(...) at {}",
-                                        step.locator
-                                    );
+                                    // warn!(
+                                    //     "trying to `match` non-reflective vs {name}(...) at {}",
+                                    //     step.locator
+                                    // );
                                     continue;
                                 }
                             }
                         }
+                        MatchBinding::Variable { name } => binder.bind(
+                            &mut self.scopes[scope_idx].variables,
+                            &Binding::Variable(name.clone()),
+                            matchable_evaled,
+                        ),
                         MatchBinding::Ignore => {
                             // always matches: no variable to bind, no conditions to check!
                         }
@@ -782,11 +787,7 @@ impl<'a, O: OutputSystem + Send, LS: LocalisationSystem + Sync + Send> Interpret
             Expression::Variable { name, loc } => {
                 let locals = &self.scopes[scope_idx].variables;
                 match locals.get(name as &str) {
-                    Some(variable_value) => {
-                        let new = variable_value.clone();
-                        eprintln!("{variable_value:?} -> {new:?}");
-                        Ok(variable_value.clone())
-                    }
+                    Some(variable_value) => Ok(variable_value.clone()),
                     None => {
                         let locals_list = locals.keys().join(", ");
                         Err(InterpreterError::TypeError {
diff --git a/hornbeam_ir/src/ast_to_ir.rs b/hornbeam_ir/src/ast_to_ir.rs
index 1398863..2d40cd3 100644
--- a/hornbeam_ir/src/ast_to_ir.rs
+++ b/hornbeam_ir/src/ast_to_ir.rs
@@ -215,20 +215,30 @@ fn compile_ast_block_to_steps(
                 let extra_inject = (attr_name.as_str() == "class" && !he.classes.is_empty())
                     .then(|| intern(he.classes.iter().map(|istr| istr.as_str()).join(" ") + " "));
                 if attr_flags.optional {
+                    // For optional fields:
+                    // - if it matches Some($x), unwrap to $x and emit $x
+                    // - if it matches None, skip
+                    // - if it matches neither, assume it's already unwrapped and emit directly
+                    // A little bit ugly to say the least...
                     let mut some_stage = Vec::new();
+                    let virtual_varname = intern("___attrval");
+
                     gen_steps_to_write_attribute(
                         he,
                         attr_name,
-                        attr_expr,
+                        &Expression::Variable {
+                            name: virtual_varname.clone(),
+                            loc: he.loc.clone(),
+                        },
                         extra_inject.clone(),
                         &mut some_stage,
                     );
                     let binding = MatchBinding::TupleVariant {
                         name: intern("Some"),
-                        pieces: vec![Binding::Variable(intern("___attrval"))],
+                        pieces: vec![Binding::Variable(virtual_varname.clone())],
                     };
 
-                    let mut arms = vec![(binding, some_stage)];
+                    let mut arms = vec![(binding, some_stage.clone())];
 
                     if let Some(extra_inject) = extra_inject {
                         let mut none_stage = Vec::new();
@@ -244,15 +254,29 @@ fn compile_ast_block_to_steps(
                             },
                             none_stage,
                         ));
+                    } else {
+                        arms.push((
+                            MatchBinding::UnitVariant {
+                                name: intern("None"),
+                            },
+                            Vec::new(),
+                        ));
                     }
 
+                    arms.push((
+                        MatchBinding::Variable {
+                            name: virtual_varname.clone(),
+                        },
+                        some_stage,
+                    ));
+
                     instructions.push(Step {
                         def: StepDef::Match {
                             matchable: attr_expr.clone(),
                             arms,
                         },
                         locator: he.loc.clone(),
-                    })
+                    });
                 } else {
                     gen_steps_to_write_attribute(
                         he,