Healed field for RgbaColors, CodeRev feedback 🤝

3 files changed, 78 insertions, 62 deletions

diff --git a/src/syntax/expr.rs b/src/syntax/expr.rs
index b7256c84..52efbe53 100644
--- a/src/syntax/expr.rs
+++ b/src/syntax/expr.rs
@@ -116,42 +116,57 @@ impl Debug for Ident {
 /// ```
 #[derive(Clone, Eq, PartialEq, Hash)]
 pub struct RgbaColor {
-    r: u8,
-    g: u8,
-    b: u8,
-    a: u8,
+    /// Red channel.
+    pub r: u8,
+    /// Green channel.
+    pub g: u8,
+    /// Blue channel.
+    pub b: u8,
+    /// Alpha channel.
+    pub a: u8,
+    /// Indicates whether this is a user-provided value or a
+    /// default value provided as a fail-over by the parser.
+    /// This color may be overwritten if this property is true.
+    pub healed: bool,
 }
 
 impl RgbaColor {
+    /// Constructs a new color.
     pub fn new(r: u8, g: u8, b: u8, a: u8) -> RgbaColor {
-        RgbaColor { r, g, b, a }
+        RgbaColor { r, g, b, a, healed: false }
     }
 
+    /// Constructs a new color with the healed property set to true.
+    pub fn new_healed(r: u8, g: u8, b: u8, a: u8) -> RgbaColor {
+        RgbaColor { r, g, b, a, healed: true }
+    }
+
+    /// Constructs a new color from a hex string like `7a03c2`.
+    /// Do not specify a leading `#`.
     pub fn from_str(hex_str: &str) -> Option<RgbaColor> {
-        let len = hex_str.len();
-        let permissable = &[3, 4, 6, 8];
-        
-        if !permissable.contains(&len) {
+        if !hex_str.is_ascii() {
             return None;
         }
 
-        let long = len == 6 || len == 8;
+        let len = hex_str.len();
+        let long =  len == 6 || len == 8;
+        let short = len == 3 || len == 4;
         let alpha = len == 4 || len == 8;
+
+        if !long && !short {
+            return None;
+        }
+
         let mut values: [u8; 4] = [255; 4];
 
         for elem in if alpha { 0..4 } else { 0..3 } {
             let item_len = if long { 2 } else { 1 };
             let pos = elem * item_len;
 
-            if let Ok(val) = u8::from_str_radix(
-                &hex_str[pos..(pos+item_len)], 16) {
-                values[elem] = val;
-            } else {
-                // Some non-hexadecimal characters slipped into the color
-                return None;
-            }
+            let item = &hex_str[pos..(pos+item_len)];
+            values[elem] = u8::from_str_radix(item, 16).ok()?;
             
-            if !long {
+            if short {
                 // Duplicate number for shorthand notation, i.e. `a` -> `aa`
                 values[elem] += values[elem] * 16;
             }
@@ -171,13 +186,18 @@ impl Debug for RgbaColor {
             f.write_fmt(format_args!("g: {:02}, ", self.g))?;
             f.write_fmt(format_args!("b: {:02}, ", self.b))?;
             f.write_fmt(format_args!("a: {:02}",   self.a))?;
-            f.write_char(')')
+            f.write_char(')')?;
         } else {
             f.write_char('#')?;
             f.write_fmt(format_args!("{:02x}", self.r))?;
             f.write_fmt(format_args!("{:02x}", self.g))?;
             f.write_fmt(format_args!("{:02x}", self.b))?;
-            f.write_fmt(format_args!("{:02x}", self.a))
+            f.write_fmt(format_args!("{:02x}", self.a))?;
+        }
+        if self.healed {
+            f.write_fmt(format_args!(" [healed]"))
+        } else {
+            Ok(())
         }
     }
 }
@@ -276,8 +296,9 @@ impl Debug for Tuple {
 /// ```typst
 /// hsl(93, 10, 19.4)
 /// ```
-#[derive(Clone, PartialEq)]
+#[derive(Clone, PartialEq, Debug)]
 pub struct NamedTuple {
+    /// The name of the tuple and where it is in the user source.
     pub name: Spanned<Ident>,
     /// The elements of the tuple.
     pub tuple: Spanned<Tuple>,
@@ -298,15 +319,6 @@ impl Deref for NamedTuple {
     }
 }
 
-impl Debug for NamedTuple {
-    fn fmt(&self, f: &mut Formatter) -> fmt::Result {
-        f.debug_struct("named tuple")
-            .field("name", &self.name)
-            .field("values", &self.tuple)
-            .finish()
-    }
-}
-
 /// A key-value collection of identifiers and associated expressions.
 ///
 /// The pairs themselves are not spanned, but the combined spans can easily be
diff --git a/src/syntax/parsing.rs b/src/syntax/parsing.rs
index c85a9c31..772f2dc0 100644
--- a/src/syntax/parsing.rs
+++ b/src/syntax/parsing.rs
@@ -253,15 +253,13 @@ impl<'s> FuncParser<'s> {
             Token::ExprSize(s) => take!(Expr::Size(s)),
             Token::ExprBool(b) => take!(Expr::Bool(b)),
             Token::ExprHex(s) => {
-                let color_opt = RgbaColor::from_str(s);
-
-                if let Some(color) = color_opt {
+                if let Some(color) = RgbaColor::from_str(s) {
                     take!(Expr::Color(color))
                 } else {
                     // Heal color by assuming black
                     self.feedback.errors.push(err!(first.span;
-                        "expected valid color, found invalid color #{}", s));
-                    take!(Expr::Color(RgbaColor::new(0, 0, 0, 255)))
+                        "invalid color"));
+                    take!(Expr::Color(RgbaColor::new_healed(0, 0, 0, 255)))
                 }
             },
 
@@ -286,8 +284,8 @@ impl<'s> FuncParser<'s> {
     /// Parse a tuple expression: `name(<expr>, ...)` with a given identifier.
     fn parse_named_tuple(&mut self, name: Spanned<Ident>) -> Spanned<NamedTuple> {
         let tuple = self.parse_tuple();
-        let start = name.span.start.clone();
-        let end = tuple.span.end.clone();
+        let start = name.span.start;
+        let end = tuple.span.end;
         Spanned::new(NamedTuple::new(name, tuple), Span::new(start, end))
     }
 
@@ -549,14 +547,22 @@ mod tests {
     fn Id(text: &str) -> Expr { Expr::Ident(Ident(text.to_string())) }
     fn Str(text: &str) -> Expr { Expr::Str(text.to_string()) }
     fn Pt(points: f32) -> Expr { Expr::Size(Size::pt(points)) }
+
     fn ClrS(color: &str) -> Expr {
         Expr::Color(
             RgbaColor::from_str(color).expect("Test color invalid")
         )
     }
+    fn ClrS_Healed() -> Expr {
+        let mut c = RgbaColor::from_str("000f")
+            .expect("Test color invalid");
+        c.healed = true;
+        Expr::Color(c)
+    }
     fn Clr(r: u8, g: u8, b: u8, a: u8) -> Expr {
         Expr::Color(RgbaColor::new(r, g, b, a))
     }
+
     fn T(text: &str) -> Node { Node::Text(text.to_string()) }
 
     /// Create a raw text node.
@@ -819,17 +825,17 @@ mod tests {
         ]);
 
         //Invalid colors
-        p!("[val: #12345]" => [func!("val": (ClrS("000f")), {})], [
-            (0:6, 0:12, "expected valid color, found invalid color #12345"),
+        p!("[val: #12345]" => [func!("val": (ClrS_Healed()), {})], [
+            (0:6, 0:12, "invalid color"),
         ]);
-        p!("[val: #a5]" => [func!("val": (ClrS("000f")), {})], [
-            (0:6, 0:9, "expected valid color, found invalid color #a5"),
+        p!("[val: #a5]" => [func!("val": (ClrS_Healed()), {})], [
+            (0:6, 0:9, "invalid color"),
         ]);
-        p!("[val: #14b2ah]" => [func!("val": (ClrS("000f")), {})], [
-            (0:6, 0:13, "expected valid color, found invalid color #14b2a"),
+        p!("[val: #14b2ah]" => [func!("val": (ClrS_Healed()), {})], [
+            (0:6, 0:13, "invalid color"),
         ]);
-        p!("[val: #f075ff011]" => [func!("val": (ClrS("000f")), {})], [
-            (0:6, 0:16, "expected valid color, found invalid color #f075ff011"),
+        p!("[val: #f075ff011]" => [func!("val": (ClrS_Healed()), {})], [
+            (0:6, 0:16, "invalid color"),
         ]);
     }
 
diff --git a/src/syntax/tokens.rs b/src/syntax/tokens.rs
index d3e062a8..07a1ac92 100644
--- a/src/syntax/tokens.rs
+++ b/src/syntax/tokens.rs
@@ -224,24 +224,13 @@ impl<'s> Iterator for Tokens<'s> {
             // An escaped thing.
             '\\' if self.mode == Body => self.parse_escaped(),
 
+            // A hex expression.
+            '#' if self.mode == Header => self.parse_hex_value(),
+
             // Expressions or just strings.
             c => {
                 let body = self.mode == Body;
                 
-                // This may be a hex expression
-                let hex_expr = if c == '#' && !body {
-                    let payload = self.read_string_until(|n| {
-                        match n {
-                            '0'..='9' | 'a'..='f' | 'A'..='F' => false,
-                            _ => true,
-                        }
-                    }, false, 0, 0).0;
-
-                    Some(ExprHex(payload))
-                } else {
-                    None
-                };
-
                 let text = self.read_string_until(|n| {
                     match n {
                         c if c.is_whitespace() => true,
@@ -253,9 +242,7 @@ impl<'s> Iterator for Tokens<'s> {
                     }
                 }, false, -(c.len_utf8() as isize), 0).0;
 
-                if let Some(hex_expr) = hex_expr {
-                    hex_expr
-                } else if self.mode == Header {
+                if self.mode == Header {
                     self.parse_expr(text)
                 } else {
                     Text(text)
@@ -399,6 +386,17 @@ impl<'s> Tokens<'s> {
         }
     }
 
+    fn parse_hex_value(&mut self) -> Token<'s> {
+        // This will parse more than the permissable 0-9, a-f, A-F character
+        // ranges to provide nicer error messages later.
+        let payload = self.read_string_until(
+            |n| !n.is_ascii_alphanumeric(), 
+            false, 0, 0
+        ).0;
+
+        ExprHex(payload)
+    }
+
     fn parse_expr(&mut self, text: &'s str) -> Token<'s> {
         if let Ok(b) = text.parse::<bool>() {
             ExprBool(b)