From c2caf94db7d5c30a916722567df9fcf4e2f126a4 Mon Sep 17 00:00:00 2001 From: hello-world-dot-c Date: Wed, 29 Apr 2026 19:36:13 +0200 Subject: [PATCH] Fix Visual tab parsing for Sieve flags and body tests The Visual tab was treating some valid Sieve as raw because it did not know about the imap4flags hasflag test, and it parsed body :contains :text like :text was the match type. Added AST/model/parser/emitter support for hasflag, converted body tests into visual conditions, exposed Body and Has Flag in the condition UI, and taught the parser to read Rulename-style comments. Also added small synthetic regression tests, because I am new enough to Rust that future me will absolutely need the reminders. --- src/model/enums.rs | 3 ++ src/sieve/ast.rs | 6 +++ src/sieve/converter.rs | 66 +++++++++++++++++++++++++++++++ src/sieve/emitter.rs | 20 ++++++++++ src/sieve/parser.rs | 86 +++++++++++++++++++++++++++++++++++++++++ src/ui/condition_row.rs | 27 ++++++++++--- 6 files changed, 202 insertions(+), 6 deletions(-) diff --git a/src/model/enums.rs b/src/model/enums.rs index 8ec3558..bf5ab14 100644 --- a/src/model/enums.rs +++ b/src/model/enums.rs @@ -191,6 +191,7 @@ pub enum ConditionTest { False, Not, Body, + Hasflag, } impl ConditionTest { @@ -205,6 +206,7 @@ impl ConditionTest { Self::False => "false", Self::Not => "not", Self::Body => "body", + Self::Hasflag => "hasflag", } } @@ -219,6 +221,7 @@ impl ConditionTest { "false" => Some(Self::False), "not" => Some(Self::Not), "body" => Some(Self::Body), + "hasflag" => Some(Self::Hasflag), _ => None, } } diff --git a/src/sieve/ast.rs b/src/sieve/ast.rs index 26e5c4c..b8f5a3e 100644 --- a/src/sieve/ast.rs +++ b/src/sieve/ast.rs @@ -90,6 +90,12 @@ pub enum TestExpr { match_type: String, keys: Vec, }, + /// `hasflag :match_type ["variable"] "flag"` + HasFlag { + match_type: String, + variable_names: Vec, + flags: Vec, + }, /// `true` True, /// `false` diff --git a/src/sieve/converter.rs b/src/sieve/converter.rs index 53e701d..a1778f9 100644 --- a/src/sieve/converter.rs +++ b/src/sieve/converter.rs @@ -164,6 +164,23 @@ fn single_test_to_condition(expr: &TestExpr) -> Option { header_names: header_names.clone(), ..Default::default() }), + TestExpr::Body { match_type, keys } => Some(Condition { + test_type: ConditionTest::Body, + keys: keys.clone(), + match_type: MatchType::from_sieve(match_type).unwrap_or(MatchType::Contains), + ..Default::default() + }), + TestExpr::HasFlag { + match_type, + variable_names, + flags, + } => Some(Condition { + test_type: ConditionTest::Hasflag, + header_names: variable_names.clone(), + keys: flags.clone(), + match_type: MatchType::from_sieve(match_type).unwrap_or(MatchType::Contains), + ..Default::default() + }), TestExpr::Not(inner) => { single_test_to_condition(inner).map(|mut c| { c.negate = true; @@ -304,6 +321,11 @@ fn condition_to_test_expr(cond: &Condition) -> TestExpr { match_type: cond.match_type.as_sieve().to_string(), keys: cond.keys.clone(), }, + ConditionTest::Hasflag => TestExpr::HasFlag { + match_type: cond.match_type.as_sieve().to_string(), + variable_names: cond.header_names.clone(), + flags: cond.keys.clone(), + }, ConditionTest::Not => TestExpr::True, // fallback }; @@ -348,6 +370,7 @@ fn collect_requires(rules: &[SieveRule]) -> Vec { for cond in &rule.conditions { match cond.test_type { ConditionTest::Body => { requires.insert("body".to_string()); } + ConditionTest::Hasflag => { requires.insert("imap4flags".to_string()); } ConditionTest::Envelope => { requires.insert("envelope".to_string()); } _ => {} } @@ -397,6 +420,19 @@ if anyof (header :contains "From" "news@a.com", header :contains "From" "news@b. if address :is :domain "From" "hapimag.com" { fileinto "INBOX/Hapimag"; } +"#; + + const IMAP4FLAGS_AND_BODY_SCRIPT: &str = r#"require ["fileinto", "body", "imap4flags"]; + +# Filter: Flag marker +if allof (not hasflag :is "$label", header :contains "from" ["example.invalid", "example.test"]) { + addflag "$label"; +} + +# Filter: Body text match +if anyof (header :contains "From" "sender.example", body :contains :text "token.example") { + fileinto "Matched"; +} "#; #[test] @@ -518,4 +554,34 @@ if address :is :domain "From" "hapimag.com" { assert_eq!(r.conditions[0].header_names, vec!["From"]); assert_eq!(r.conditions[0].keys, vec!["hapimag.com"]); } + + #[test] + fn test_parse_imap4flags_and_body_constructs() { + let script = text_to_script(IMAP4FLAGS_AND_BODY_SCRIPT, "server"); + assert_eq!(script.rules.len(), 2); + + let flag_marker = &script.rules[0]; + assert_eq!(flag_marker.name, "Flag marker"); + assert!(flag_marker.raw_block.is_none()); + assert_eq!(flag_marker.conditions.len(), 2); + assert_eq!(flag_marker.conditions[0].test_type, ConditionTest::Hasflag); + assert!(flag_marker.conditions[0].negate); + assert_eq!(flag_marker.conditions[0].match_type, MatchType::Is); + assert_eq!(flag_marker.conditions[0].keys, vec!["$label"]); + assert_eq!(flag_marker.actions[0].action_type, ActionType::Addflag); + assert_eq!(flag_marker.actions[0].argument, "$label"); + + let body_match = &script.rules[1]; + assert_eq!(body_match.name, "Body text match"); + assert!(body_match.raw_block.is_none()); + assert_eq!(body_match.logic, LogicOperator::AnyOf); + assert_eq!(body_match.conditions.len(), 2); + assert_eq!(body_match.conditions[1].test_type, ConditionTest::Body); + assert_eq!(body_match.conditions[1].match_type, MatchType::Contains); + assert_eq!(body_match.conditions[1].keys, vec!["token.example"]); + + let roundtrip = script_to_text(&script); + assert!(roundtrip.contains("not hasflag :is \"$label\"")); + assert!(roundtrip.contains("body :contains \"token.example\"")); + } } diff --git a/src/sieve/emitter.rs b/src/sieve/emitter.rs index c55bdb7..a2f11cb 100644 --- a/src/sieve/emitter.rs +++ b/src/sieve/emitter.rs @@ -199,6 +199,20 @@ fn emit_test_expr(out: &mut String, expr: &TestExpr) { out.push(' '); emit_string_or_list(out, keys); } + TestExpr::HasFlag { + match_type, + variable_names, + flags, + } => { + out.push_str("hasflag "); + out.push_str(match_type); + if !variable_names.is_empty() { + out.push(' '); + emit_string_or_list(out, variable_names); + } + out.push(' '); + emit_string_or_list(out, flags); + } TestExpr::True => out.push_str("true"), TestExpr::False => out.push_str("false"), } @@ -288,6 +302,12 @@ fn collect_test_requires(expr: &TestExpr, requires: &mut std::collections::BTree TestExpr::Body { .. } => { requires.insert("body".to_string()); } + TestExpr::HasFlag { match_type, .. } => { + requires.insert("imap4flags".to_string()); + if match_type == ":regex" { + requires.insert("regex".to_string()); + } + } TestExpr::Header { match_type, .. } | TestExpr::Address { match_type, .. } => { if match_type == ":regex" { diff --git a/src/sieve/parser.rs b/src/sieve/parser.rs index 77edf88..8de73c0 100644 --- a/src/sieve/parser.rs +++ b/src/sieve/parser.rs @@ -87,6 +87,13 @@ fn extract_filter_name(comment: &Option) -> Option { } else { Some(name.to_string()) } + } else if let Some((_, rest)) = trimmed.split_once("Rulename:") { + let name = rest.split('|').next().unwrap_or("").trim(); + if name.is_empty() { + None + } else { + Some(name.to_string()) + } } else { None } @@ -216,6 +223,10 @@ fn parse_test_expr(tokens: &[&Token], pos: &mut usize) -> Result { + *pos += 1; + parse_hasflag_test(tokens, pos) + } "true" => { *pos += 1; Ok(TestExpr::True) @@ -375,6 +386,15 @@ fn parse_body_test(tokens: &[&Token], pos: &mut usize) -> Result Result Result { + let mut match_type = ":is".to_string(); + + while let Some(Token::Tag(tag)) = tokens.get(*pos) { + if tag == ":comparator" { + *pos += 1; + if matches!(tokens.get(*pos), Some(Token::QuotedString(_))) { + *pos += 1; + } + continue; + } + match_type = tag.clone(); + *pos += 1; + } + + let first = parse_string_or_list(tokens, pos)?; + let second = parse_string_or_list(tokens, pos)?; + let (variable_names, flags) = if second.is_empty() { + (Vec::new(), first) + } else { + (first, second) + }; + + Ok(TestExpr::HasFlag { + match_type, + variable_names, + flags, + }) +} + fn parse_string_or_list(tokens: &[&Token], pos: &mut usize) -> Result, String> { match tokens.get(*pos) { Some(Token::QuotedString(s)) => { @@ -591,4 +641,40 @@ if allof (header :is "From" "boss@example.com", header :contains "Subject" "urge } } } + + #[test] + fn test_parse_hasflag_and_body_text() { + let input = r#"if allof (not hasflag :is "$label", body :contains :text "token.example") { + addflag "$label"; +}"#; + let script = parse(input).unwrap(); + let if_cmd = script.commands.iter().find(|c| matches!(c, Command::If(_))); + if let Some(Command::If(block)) = if_cmd { + match &block.condition { + TestExpr::AllOf(tests) => { + assert_eq!(tests.len(), 2); + match &tests[0] { + TestExpr::Not(inner) => match inner.as_ref() { + TestExpr::HasFlag { + match_type, flags, .. + } => { + assert_eq!(match_type, ":is"); + assert_eq!(flags, &["$label"]); + } + _ => panic!("Expected HasFlag test"), + }, + _ => panic!("Expected negated HasFlag"), + } + match &tests[1] { + TestExpr::Body { match_type, keys } => { + assert_eq!(match_type, ":contains"); + assert_eq!(keys, &["token.example"]); + } + _ => panic!("Expected Body test"), + } + } + _ => panic!("Expected AllOf"), + } + } + } } diff --git a/src/ui/condition_row.rs b/src/ui/condition_row.rs index b63be53..7da5848 100644 --- a/src/ui/condition_row.rs +++ b/src/ui/condition_row.rs @@ -29,6 +29,7 @@ impl std::fmt::Display for ConditionTestOption { ConditionTest::Size => write!(f, "Size"), ConditionTest::Exists => write!(f, "Exists"), ConditionTest::Body => write!(f, "Body"), + ConditionTest::Hasflag => write!(f, "Has Flag"), other => write!(f, "{}", other.as_sieve()), } } @@ -40,6 +41,8 @@ pub const TEST_OPTIONS: &[ConditionTestOption] = &[ ConditionTestOption(ConditionTest::Envelope), ConditionTestOption(ConditionTest::Size), ConditionTestOption(ConditionTest::Exists), + ConditionTestOption(ConditionTest::Body), + ConditionTestOption(ConditionTest::Hasflag), ]; #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -104,6 +107,8 @@ pub fn view(cond: &Condition, number: usize) -> Element<'_, ConditionMessage> { let test_type = ConditionTestOption(cond.test_type); let is_size = cond.test_type == ConditionTest::Size; let is_exists = cond.test_type == ConditionTest::Exists; + let is_body = cond.test_type == ConditionTest::Body; + let is_hasflag = cond.test_type == ConditionTest::Hasflag; let is_address = matches!( cond.test_type, ConditionTest::Address | ConditionTest::Envelope @@ -165,13 +170,16 @@ pub fn view(cond: &Condition, number: usize) -> Element<'_, ConditionMessage> { ); } - // Header name (not for size) - if !is_size { + // Header/variable name (not for size or body) + if !is_size && !is_body { let headers = cond.header_names.join(", "); fields = fields.push( column![ - label_text("Header"), - text_input("Header name", &headers) + label_text(if is_hasflag { "Variable" } else { "Header" }), + text_input( + if is_hasflag { "Optional variable" } else { "Header name" }, + &headers + ) .on_input(ConditionMessage::SetHeaders) .width(140), ] @@ -218,10 +226,17 @@ pub fn view(cond: &Condition, number: usize) -> Element<'_, ConditionMessage> { } else { cond.keys.first().map(String::as_str).unwrap_or("") }; + let value_label = if is_hasflag { + "Flag" + } else if is_body { + "Text" + } else { + "Value" + }; fields = fields.push( column![ - label_text("Value"), - text_input("Value", value) + label_text(value_label), + text_input(value_label, value) .on_input(ConditionMessage::SetValue) .width(Length::Fill), ]