diff --git a/src/app.rs b/src/app.rs index 328d92c..b517df1 100644 --- a/src/app.rs +++ b/src/app.rs @@ -15,7 +15,10 @@ use crate::{ MacroAction, MacroContext, MacroRuntime, MenuChoice, PromptKind, key_to_source, macro_name, parse_key_binding, }, - model::{Category, DocumentSettings, Item, ItemChanges, MacroDef, ViewDef}, + model::{ + Category, DateOrder, DocumentSettings, DonePolicy, Item, ItemChanges, MacroDef, + TrashPolicy, ViewDef, ViewKind, WeekStart, + }, preferences::AppPreferences, }; @@ -397,7 +400,7 @@ impl App { let today = Local::now().date_naive(); if today != self.current_day { self.current_day = today; - if self.document_settings.trash_policy == "end-of-day" { + if self.document_settings.trash_policy == TrashPolicy::EndOfDay { let count = self.db.empty_trash()?; if count > 0 { self.status = format!("Emptied {count} Trash item(s) at end of day"); @@ -985,9 +988,9 @@ impl App { if ids.is_empty() { return Ok(()); } - let restore = self.current_view().kind == "trash"; + let restore = self.current_view().kind == ViewKind::Trash; if !restore - && self.document_settings.trash_policy == "immediate" + && self.document_settings.trash_policy == TrashPolicy::Immediate && self.preferences.confirm_destructive { self.mode = Mode::Confirm { @@ -1000,7 +1003,7 @@ impl App { } fn apply_discard(&mut self, ids: Vec, restore: bool) -> Result<()> { - let permanent = !restore && self.document_settings.trash_policy == "immediate"; + let permanent = !restore && self.document_settings.trash_policy == TrashPolicy::Immediate; self.db.discard(&ids, !restore)?; self.marked.clear(); self.refresh()?; @@ -1174,13 +1177,33 @@ impl App { props.values[props.field].pop(); } KeyCode::Enter => { - let priority = props.values[0].parse().unwrap_or(3); - let when_at = normalize_date(&self.db, &props.values[1])?; - let alarm_at = normalize_date(&self.db, &props.values[2])?; - let numeric_value = if props.values[3].trim().is_empty() { - None - } else { - props.values[3].parse().ok() + let parsed = (|| -> Result<_> { + let priority = props.values[0] + .parse::() + .context("priority must be a number from 1 to 5")?; + if !(1..=5).contains(&priority) { + bail!("priority must be a number from 1 to 5"); + } + let when_at = normalize_date(&self.db, &props.values[1])?; + let alarm_at = normalize_date(&self.db, &props.values[2])?; + let numeric_value = if props.values[3].trim().is_empty() { + None + } else { + Some( + props.values[3] + .parse::() + .context("numeric value must be a number")?, + ) + }; + Ok((priority, when_at, alarm_at, numeric_value)) + })(); + let (priority, when_at, alarm_at, numeric_value) = match parsed { + Ok(values) => values, + Err(error) => { + self.status = format!("Cannot save properties: {error:#}"); + self.mode = Mode::Properties(props); + return Ok(()); + } }; self.db.update_item( props.item_id, @@ -1436,11 +1459,11 @@ impl App { values: vec![ settings.description.clone(), yes_no(settings.backup_on_open), - settings.trash_policy.clone(), - settings.done_policy.clone(), + settings.trash_policy.to_string(), + settings.done_policy.to_string(), yes_no(settings.automatic_filing), - settings.date_order.clone(), - settings.week_start.clone(), + settings.date_order.to_string(), + settings.week_start.to_string(), settings.default_time.clone(), settings.morning_time.clone(), settings.afternoon_time.clone(), @@ -1471,7 +1494,7 @@ impl App { vec![ c.name, parent, - c.kind, + c.kind.to_string(), c.match_text, yes_no(c.exclusive), condition, @@ -1502,10 +1525,10 @@ impl App { let sections = v.sections_spec(); vec![ v.name, - v.kind, + v.kind.to_string(), v.filter_value, v.filter_expr, - v.sort_key, + v.sort_key.to_string(), yes_no(v.show_done), columns, sections, @@ -1636,11 +1659,27 @@ impl App { let settings = DocumentSettings { description: form.values[0].trim().into(), backup_on_open: parse_yes_no(&form.values[1])?, - trash_policy: form.values[2].trim().to_lowercase(), - done_policy: form.values[3].trim().to_lowercase(), + trash_policy: form.values[2] + .trim() + .to_lowercase() + .parse::() + .map_err(anyhow::Error::msg)?, + done_policy: form.values[3] + .trim() + .to_lowercase() + .parse::() + .map_err(anyhow::Error::msg)?, automatic_filing: parse_yes_no(&form.values[4])?, - date_order: form.values[5].trim().to_lowercase(), - week_start: form.values[6].trim().to_lowercase(), + date_order: form.values[5] + .trim() + .to_lowercase() + .parse::() + .map_err(anyhow::Error::msg)?, + week_start: form.values[6] + .trim() + .to_lowercase() + .parse::() + .map_err(anyhow::Error::msg)?, default_time: form.values[7].trim().into(), morning_time: form.values[8].trim().into(), afternoon_time: form.values[9].trim().into(), @@ -1707,7 +1746,7 @@ impl App { let Some(item_id) = self.selected_item().map(|i| i.id) else { return Ok(()); }; - let Some(all) = self.views.iter().find(|v| v.kind == "list") else { + let Some(all) = self.views.iter().find(|v| v.kind == ViewKind::List) else { return Ok(()); }; let choices = self @@ -1963,7 +2002,9 @@ fn normalize_date(db: &Database, value: &str) -> Result> { } else if chrono::DateTime::parse_from_rfc3339(s).is_ok() { Ok(Some(s.into())) } else { - db.interpret_date(&format!("on {s}")) + Ok(Some(db.interpret_date(&format!("on {s}"))?.with_context( + || format!("unrecognized date or time {s:?}"), + )?)) } } fn inside(r: Rect, x: u16, y: u16) -> bool { @@ -2235,4 +2276,38 @@ mod tests { assert!(matches!(input.kind, InputKind::Note(_))); assert_eq!(input.value, "x"); } + + #[test] + fn invalid_properties_remain_open_without_overwriting_the_item() { + let directory = tempdir().unwrap(); + let path = directory.path().join("invalid-properties.agnd"); + let mut db = Database::open(&path).unwrap(); + let item_id = db.add_item("Keep my properties").unwrap(); + let mut app = App::new_with_preferences( + db, + path, + AppPreferences::default(), + directory.path().join("preferences.toml"), + ) + .unwrap(); + app.mode = Mode::Properties(PropsState { + item_id, + values: [ + "not-a-priority".into(), + "not-a-date".into(), + String::new(), + "not-a-number".into(), + String::new(), + ], + field: 0, + }); + + app.handle_key(key(KeyCode::Enter)).unwrap(); + + assert!(matches!(app.mode, Mode::Properties(_))); + assert!(app.status.starts_with("Cannot save properties:")); + assert_eq!(app.items[0].priority, 3); + assert!(app.items[0].when_at.is_none()); + assert!(app.items[0].numeric_value.is_none()); + } } diff --git a/src/db.rs b/src/db.rs index 007cd93..78c9d10 100644 --- a/src/db.rs +++ b/src/db.rs @@ -1,6 +1,8 @@ use std::{ - fs, + fmt::Display, + fs, io, path::{Path, PathBuf}, + str::FromStr, }; use anyhow::{Context, Result, bail}; @@ -9,15 +11,35 @@ use rusqlite::{Connection, OptionalExtension, params}; use crate::filter; use crate::model::{ - Category, CategoryRule, DocumentSettings, Item, ItemChanges, MacroDef, ViewColumn, ViewDef, - ViewSection, + Aggregate, Category, CategoryRule, DocumentSettings, DonePolicy, Item, ItemChanges, MacroDef, + RuleActionKind, SortKey, TrashPolicy, ViewColumn, ViewDef, ViewKind, ViewSection, +}; +use crate::parser::{ + DateParseConfig, RECURRENCE_INTERVAL, extract_when_configured, next_occurrence, }; -use crate::parser::{extract_when_configured, next_occurrence}; mod presets; pub use presets::Preset; +fn enum_column(row: &rusqlite::Row<'_>, index: usize) -> rusqlite::Result +where + T: FromStr, + T::Err: Display, +{ + let value: String = row.get(index)?; + value.parse::().map_err(|error| { + rusqlite::Error::FromSqlConversionFailure( + index, + rusqlite::types::Type::Text, + Box::new(io::Error::new( + io::ErrorKind::InvalidData, + error.to_string(), + )), + ) + }) +} + pub struct Database { conn: Connection, path: PathBuf, @@ -295,12 +317,24 @@ impl Database { let settings = DocumentSettings { description: self.meta_value("document.description", &defaults.description)?, backup_on_open: self.meta_bool("document.backup_on_open", defaults.backup_on_open)?, - trash_policy: self.meta_value("document.trash_policy", &defaults.trash_policy)?, - done_policy: self.meta_value("document.done_policy", &defaults.done_policy)?, + trash_policy: self + .meta_value("document.trash_policy", defaults.trash_policy.as_str())? + .parse() + .map_err(anyhow::Error::msg)?, + done_policy: self + .meta_value("document.done_policy", defaults.done_policy.as_str())? + .parse() + .map_err(anyhow::Error::msg)?, automatic_filing: self .meta_bool("document.automatic_filing", defaults.automatic_filing)?, - date_order: self.meta_value("document.date_order", &defaults.date_order)?, - week_start: self.meta_value("document.week_start", &defaults.week_start)?, + date_order: self + .meta_value("document.date_order", defaults.date_order.as_str())? + .parse() + .map_err(anyhow::Error::msg)?, + week_start: self + .meta_value("document.week_start", defaults.week_start.as_str())? + .parse() + .map_err(anyhow::Error::msg)?, default_time: self.meta_value("document.default_time", &defaults.default_time)?, morning_time: self.meta_value("document.morning_time", &defaults.morning_time)?, afternoon_time: self.meta_value("document.afternoon_time", &defaults.afternoon_time)?, @@ -325,14 +359,14 @@ impl Database { "document.backup_on_open", settings.backup_on_open.to_string(), ), - ("document.trash_policy", settings.trash_policy.clone()), - ("document.done_policy", settings.done_policy.clone()), + ("document.trash_policy", settings.trash_policy.to_string()), + ("document.done_policy", settings.done_policy.to_string()), ( "document.automatic_filing", settings.automatic_filing.to_string(), ), - ("document.date_order", settings.date_order.clone()), - ("document.week_start", settings.week_start.clone()), + ("document.date_order", settings.date_order.to_string()), + ("document.week_start", settings.week_start.to_string()), ("document.default_time", settings.default_time.clone()), ("document.morning_time", settings.morning_time.clone()), ("document.afternoon_time", settings.afternoon_time.clone()), @@ -370,12 +404,14 @@ impl Database { let settings = self.document_settings()?; Ok(extract_when_configured( text, - &settings.date_order, - &settings.week_start, - &settings.default_time, - &settings.morning_time, - &settings.afternoon_time, - &settings.evening_time, + DateParseConfig { + date_order: settings.date_order, + week_start: settings.week_start, + default_time: &settings.default_time, + morning_time: &settings.morning_time, + afternoon_time: &settings.afternoon_time, + evening_time: &settings.evening_time, + }, )) } @@ -405,9 +441,9 @@ impl Database { Ok(ViewDef { id: r.get(0)?, name: r.get(1)?, - kind: r.get(2)?, + kind: enum_column(r, 2)?, filter_value: r.get(3)?, - sort_key: r.get(4)?, + sort_key: enum_column(r, 4)?, show_done: r.get::<_, i64>(5)? != 0, filter_expr: r.get(6)?, columns: vec![], @@ -431,7 +467,7 @@ impl Database { field: r.get(0)?, heading: r.get(1)?, width: r.get::<_, u16>(2)?, - aggregate: r.get(3)?, + aggregate: enum_column(r, 3)?, }) })? .collect::>>()?; @@ -468,7 +504,7 @@ impl Database { id: r.get(0)?, name: r.get(1)?, parent_id: r.get(2)?, - kind: r.get(3)?, + kind: enum_column(r, 3)?, match_text: r.get(4)?, exclusive: r.get::<_, i64>(5)? != 0, }) @@ -532,7 +568,7 @@ impl Database { id: r.get(0)?, category_id: r.get(1)?, condition_expr: r.get(2)?, - action_kind: r.get(3)?, + action_kind: enum_column(r, 3)?, action_value: r.get(4)?, enabled: r.get::<_, i64>(5)? != 0, }) @@ -560,7 +596,7 @@ impl Database { "DELETE FROM category_rules WHERE category_id=?1", [category_id], )?; - tx.execute("INSERT INTO category_rules(category_id,condition_expr,action_kind,action_value,enabled,sort_order) VALUES(?1,?2,?3,?4,1,0)",params![category_id,condition_expr.trim(),kind,value])?; + tx.execute("INSERT INTO category_rules(category_id,condition_expr,action_kind,action_value,enabled,sort_order) VALUES(?1,?2,?3,?4,1,0)",params![category_id,condition_expr.trim(),kind.as_str(),value])?; tx.commit()?; Ok(()) } @@ -607,7 +643,7 @@ impl Database { }; tx.execute("DELETE FROM view_columns WHERE view_id=?1", [view_id])?; for (order, column) in columns.iter().enumerate() { - tx.execute("INSERT INTO view_columns(view_id,field,heading,width,aggregate,sort_order) VALUES(?1,?2,?3,?4,?5,?6)",params![view_id,column.field,column.heading,column.width,column.aggregate,order as i64])?; + tx.execute("INSERT INTO view_columns(view_id,field,heading,width,aggregate,sort_order) VALUES(?1,?2,?3,?4,?5,?6)",params![view_id,column.field,column.heading,column.width,column.aggregate.as_str(),order as i64])?; } tx.execute("DELETE FROM view_sections WHERE view_id=?1", [view_id])?; for (order, section) in sections.iter().enumerate() { @@ -843,7 +879,7 @@ impl Database { id: r.get(0)?, category_id: r.get(1)?, condition_expr: r.get(2)?, - action_kind: r.get(3)?, + action_kind: enum_column(r, 3)?, action_value: r.get(4)?, enabled: r.get::<_, i64>(5)? != 0, }) @@ -906,8 +942,8 @@ impl Database { } fn apply_rule(&self, item_id: i64, rule: &CategoryRule, item: &Item) -> Result { - match rule.action_kind.as_str() { - "assign" | "exclude" => { + match rule.action_kind { + RuleActionKind::Assign | RuleActionKind::Exclude => { let target = if rule.action_value.trim().is_empty() { Some(rule.category_id) } else { @@ -923,7 +959,7 @@ impl Database { return Ok(false); }; let state:Option=self.conn.query_row("SELECT assignment FROM item_categories WHERE item_id=?1 AND category_id=?2",params![item_id,target],|r|r.get(0)).optional()?; - if rule.action_kind == "assign" { + if rule.action_kind == RuleActionKind::Assign { if state.is_none() { self.conn.execute("INSERT INTO item_categories(item_id,category_id,assignment) VALUES(?1,?2,'conditional')",params![item_id,target])?; return Ok(true); @@ -936,7 +972,7 @@ impl Database { } Ok(false) } - "priority" => { + RuleActionKind::Priority => { let value = rule.action_value.parse::()?.clamp(1, 5); if item.priority != value { self.conn.execute( @@ -948,7 +984,7 @@ impl Database { Ok(false) } } - "value" => { + RuleActionKind::Value => { let value = rule.action_value.parse::()?; if item.numeric_value != Some(value) { self.conn.execute( @@ -960,7 +996,7 @@ impl Database { Ok(false) } } - "when" => { + RuleActionKind::When => { let value = self .interpret_date(&rule.action_value)? .context("rule action has no recognizable date")?; @@ -974,7 +1010,7 @@ impl Database { Ok(false) } } - "alarm" => { + RuleActionKind::Alarm => { let value = self .interpret_date(&rule.action_value)? .context("rule action has no recognizable alarm date")?; @@ -988,7 +1024,7 @@ impl Database { Ok(false) } } - "repeat" => { + RuleActionKind::Repeat => { let value = rule.action_value.trim(); if let Some(when) = item.when_at.as_deref() { next_occurrence(when, value)?; @@ -1003,7 +1039,7 @@ impl Database { Ok(false) } } - "done" => { + RuleActionKind::Done => { let done = parse_bool(&rule.action_value); if item.done_at.is_some() != done { let value = if done { @@ -1020,7 +1056,6 @@ impl Database { Ok(false) } } - _ => bail!("unsupported rule action {}", rule.action_kind), } } @@ -1063,7 +1098,8 @@ impl Database { tx.execute("INSERT OR REPLACE INTO item_categories(item_id,category_id,assignment) SELECT ?1,category_id,assignment FROM item_categories WHERE item_id=?2 AND assignment IN ('explicit','excluded')",params![created,id])?; next_id = Some(created); } - let discard_completed = item.done_at.is_none() && settings.done_policy == "trash"; + let discard_completed = + item.done_at.is_none() && settings.done_policy == DonePolicy::Trash; tx.execute("UPDATE items SET done_at=CASE WHEN done_at IS NULL THEN ?1 ELSE NULL END,discarded=CASE WHEN ?3 THEN 1 ELSE discarded END,updated_at=?1 WHERE id=?2",params![now,id,discard_completed])?; tx.commit()?; if let Some(next_id) = next_id @@ -1076,7 +1112,8 @@ impl Database { } pub fn discard(&self, ids: &[i64], discarded: bool) -> Result<()> { - let permanent = discarded && self.document_settings()?.trash_policy == "immediate"; + let permanent = + discarded && self.document_settings()?.trash_policy == TrashPolicy::Immediate; for id in ids { if permanent { self.conn.execute("DELETE FROM items WHERE id=?1", [id])?; @@ -1147,34 +1184,34 @@ impl Database { "SELECT DISTINCT i.id,i.text,i.note,i.priority,i.when_at,i.done_at,i.alarm_at,i.numeric_value,i.created_at,i.updated_at,i.discarded,i.recurrence FROM items i", ); let mut values: Vec = vec![]; - if view.kind == "category" { + if view.kind == ViewKind::Category { sql.push_str( " JOIN item_categories ic ON ic.item_id=i.id AND ic.assignment<>'excluded'", ); } - sql.push_str(if view.kind == "trash" { + sql.push_str(if view.kind == ViewKind::Trash { " WHERE i.discarded=1" } else { " WHERE i.discarded=0" }); - match view.kind.as_str() { - "category" => { + match view.kind { + ViewKind::Category => { sql.push_str(" AND ic.category_id IN (WITH RECURSIVE descendants(id) AS (SELECT id FROM categories WHERE name=? UNION ALL SELECT c.id FROM categories c JOIN descendants d ON c.parent_id=d.id) SELECT id FROM descendants) AND NOT EXISTS(SELECT 1 FROM item_categories ex JOIN categories ec ON ec.id=ex.category_id WHERE ex.item_id=i.id AND ex.assignment='excluded' AND ec.name=?)"); values.extend([view.filter_value.clone(), view.filter_value.clone()]); } - "upcoming" | "datebook" => { + ViewKind::Upcoming | ViewKind::Datebook => { let days = view.filter_value.parse::().unwrap_or(30); sql.push_str(" AND i.when_at IS NOT NULL AND i.when_at <= ?"); values.push((Local::now() + Duration::days(days)).to_rfc3339()); } - "done" => { + ViewKind::Done => { let days = view.filter_value.parse::().unwrap_or(14); sql.push_str(" AND i.done_at IS NOT NULL AND i.done_at >= ?"); values.push((Local::now() - Duration::days(days)).to_rfc3339()); } - _ => {} + ViewKind::List | ViewKind::Trash => {} } - if !view.show_done && view.kind != "done" { + if !view.show_done && view.kind != ViewKind::Done { sql.push_str(" AND i.done_at IS NULL"); } if !search.is_empty() { @@ -1182,12 +1219,12 @@ impl Database { let like = format!("%{search}%"); values.extend([like.clone(), like]); } - sql.push_str(match view.sort_key.as_str() { - "when" => " ORDER BY i.when_at IS NULL,i.when_at,i.priority,i.sort_order", - "done" => " ORDER BY i.done_at DESC,i.sort_order", - "priority" => " ORDER BY i.priority,i.when_at IS NULL,i.when_at", - "updated" => " ORDER BY i.updated_at DESC", - _ => " ORDER BY i.sort_order,i.id", + sql.push_str(match view.sort_key { + SortKey::When => " ORDER BY i.when_at IS NULL,i.when_at,i.priority,i.sort_order", + SortKey::Done => " ORDER BY i.done_at DESC,i.sort_order", + SortKey::Priority => " ORDER BY i.priority,i.when_at IS NULL,i.when_at", + SortKey::Updated => " ORDER BY i.updated_at DESC", + SortKey::Manual => " ORDER BY i.sort_order,i.id", }); let mut stmt = self.conn.prepare(&sql)?; let refs: Vec<&dyn rusqlite::ToSql> = @@ -1247,7 +1284,7 @@ impl Database { } pub fn close_maintenance(&self) -> Result<()> { - if self.document_settings()?.trash_policy == "on-close" { + if self.document_settings()?.trash_policy == TrashPolicy::OnClose { self.empty_trash()?; } self.checkpoint() @@ -1306,7 +1343,10 @@ impl Database { "SUMMARY" => record.summary = value, "DESCRIPTION" => record.note = value, "DTSTART" | "DUE" => record.when_at = parse_ical_date(&value), - "COMPLETED" => record.done = true, + "COMPLETED" => { + record.done = true; + record.completed_at = parse_ical_date(&value); + } "STATUS" if value.eq_ignore_ascii_case("COMPLETED") => { record.done = true } @@ -1353,7 +1393,13 @@ impl Database { } } if record.done { - self.toggle_done(&[id])?; + let completed_at = record + .completed_at + .unwrap_or_else(|| Local::now().to_rfc3339()); + self.conn.execute( + "UPDATE items SET done_at=?1,updated_at=?1 WHERE id=?2", + params![completed_at, id], + )?; } count += 1; } @@ -1436,6 +1482,7 @@ struct IcalRecord { note: String, when_at: Option, done: bool, + completed_at: Option, priority: Option, categories: Vec, recurrence: String, @@ -1461,10 +1508,13 @@ fn parse_ical_date(value: &str) -> Option { .map(|d| d.to_rfc3339()) } fn shift_alarm(alarm: Option<&str>, old_when: Option<&str>, new_when: &str) -> Option { - let alarm = DateTime::parse_from_rfc3339(alarm?).ok()?; - let old = DateTime::parse_from_rfc3339(old_when?).ok()?; - let new = DateTime::parse_from_rfc3339(new_when).ok()?; - Some((new + (alarm - old)).to_rfc3339()) + let alarm = DateTime::parse_from_rfc3339(alarm?).ok()?.naive_local(); + let old = DateTime::parse_from_rfc3339(old_when?).ok()?.naive_local(); + let new = DateTime::parse_from_rfc3339(new_when).ok()?.naive_local(); + Local + .from_local_datetime(&(new + (alarm - old))) + .earliest() + .map(|date_time| date_time.to_rfc3339()) } fn ical_unescape(value: &str) -> String { value @@ -1493,8 +1543,10 @@ fn recurrence_to_rrule(value: &str) -> Option { "monthly" => "FREQ=MONTHLY".into(), "yearly" | "annually" => "FREQ=YEARLY".into(), _ => { - let re = regex::Regex::new(r"^every\s+(\d+)\s+(days?|weeks?|months?|years?)$").unwrap(); - let c = re.captures(&value)?; + let c = RECURRENCE_INTERVAL.captures(&value)?; + if c[1].parse::().ok()? == 0 { + return None; + } let freq = match &c[2] { "day" | "days" => "DAILY", "week" | "weeks" => "WEEKLY", @@ -1710,28 +1762,34 @@ fn default_columns() -> Vec { field: field.into(), heading: heading.into(), width, - aggregate: "none".into(), + aggregate: Aggregate::None, }) .collect() } -fn parse_action_spec(spec: &str) -> Result<(String, String)> { +fn parse_action_spec(spec: &str) -> Result<(RuleActionKind, String)> { let (kind, value) = spec.trim().split_once(':').unwrap_or((spec.trim(), "")); - let kind = kind.trim().to_lowercase(); - if !matches!( - kind.as_str(), - "assign" | "exclude" | "priority" | "value" | "when" | "alarm" | "repeat" | "done" - ) { - bail!("rule action must be assign, exclude, priority, value, when, alarm, repeat, or done") - } + let kind: RuleActionKind = kind + .trim() + .to_lowercase() + .parse() + .map_err(anyhow::Error::msg) + .context( + "rule action must be assign, exclude, priority, value, when, alarm, repeat, or done", + )?; if matches!( - kind.as_str(), - "priority" | "value" | "when" | "alarm" | "repeat" | "done" + kind, + RuleActionKind::Priority + | RuleActionKind::Value + | RuleActionKind::When + | RuleActionKind::Alarm + | RuleActionKind::Repeat + | RuleActionKind::Done ) && value.trim().is_empty() { bail!("rule action {kind} needs a value") } - if kind == "repeat" { + if kind == RuleActionKind::Repeat { next_occurrence("2026-01-05T09:00:00+00:00", value.trim())?; } Ok((kind, value.trim().into())) @@ -1745,21 +1803,6 @@ fn parse_bool(value: &str) -> bool { } fn validate_document_settings(settings: &DocumentSettings) -> Result<()> { - if !matches!( - settings.trash_policy.as_str(), - "on-demand" | "on-close" | "end-of-day" | "immediate" - ) { - bail!("trash policy must be on-demand, on-close, end-of-day, or immediate") - } - if !matches!(settings.done_policy.as_str(), "keep" | "trash") { - bail!("done policy must be keep or trash") - } - if !matches!(settings.date_order.as_str(), "ymd" | "mdy" | "dmy") { - bail!("date order must be ymd, mdy, or dmy") - } - if !matches!(settings.week_start.as_str(), "monday" | "sunday") { - bail!("week start must be monday or sunday") - } for (name, value) in [ ("default time", &settings.default_time), ("morning time", &settings.morning_time), @@ -1809,16 +1852,12 @@ fn parse_columns_spec(spec: &str) -> Result> { .map(|s| s.trim().to_owned()) .filter(|s| !s.is_empty()) .unwrap_or_else(|| default_heading(&field).into()); - let aggregate = parts + let aggregate: Aggregate = parts .get(3) .map(|s| s.trim().to_lowercase()) - .unwrap_or_else(|| "none".into()); - if !matches!( - aggregate.as_str(), - "none" | "sum" | "avg" | "count" | "min" | "max" - ) { - bail!("unsupported aggregate {aggregate}") - } + .unwrap_or_else(|| "none".into()) + .parse() + .map_err(anyhow::Error::msg)?; columns.push(ViewColumn { field, heading, @@ -2008,7 +2047,7 @@ mod tests { .find(|v| v.id == id) .unwrap(); assert_eq!(view.columns[1].heading, "Cost"); - assert_eq!(view.columns[1].aggregate, "sum"); + assert_eq!(view.columns[1].aggregate, Aggregate::Sum); assert_eq!(view.sections.len(), 2); assert!(view.sections[1].collapsed); assert_eq!(db.items(&view, "").unwrap().len(), 1); @@ -2158,11 +2197,11 @@ mod tests { db.save_document_settings(&DocumentSettings { description: "European planning file".into(), backup_on_open: true, - trash_policy: "immediate".into(), - done_policy: "trash".into(), + trash_policy: TrashPolicy::Immediate, + done_policy: DonePolicy::Trash, automatic_filing: false, - date_order: "dmy".into(), - week_start: "sunday".into(), + date_order: crate::model::DateOrder::DayMonthYear, + week_start: crate::model::WeekStart::Sunday, default_time: "08:30".into(), morning_time: "08:00".into(), afternoon_time: "14:00".into(), @@ -2257,6 +2296,7 @@ mod tests { .find(|v| v.name == "All Items") .unwrap(); let items = target.items(&all, "").unwrap(); + assert_eq!(items.len(), 1); assert_eq!(items[0].text, "Prepare launch"); assert_eq!(items[0].note, "Bring charts"); assert_eq!(items[0].priority, 1); diff --git a/src/db/presets.rs b/src/db/presets.rs index 9d2eccb..be6731a 100644 --- a/src/db/presets.rs +++ b/src/db/presets.rs @@ -289,7 +289,7 @@ impl Database { column.field, column.heading, column.width, - column.aggregate, + column.aggregate.as_str(), column_order as i64 ], )?; @@ -318,7 +318,7 @@ impl Database { params![ category_id, preset_rule.condition, - action_kind, + action_kind.as_str(), action_value, order as i64 ], diff --git a/src/filter.rs b/src/filter.rs index f084a4f..bbdacc1 100644 --- a/src/filter.rs +++ b/src/filter.rs @@ -1,5 +1,5 @@ use anyhow::{Result, bail}; -use chrono::{DateTime, Duration, Local}; +use chrono::{DateTime, Duration, Local, TimeZone}; use crate::model::Item; @@ -99,7 +99,11 @@ fn predicate(p: &Predicate, item: &Item) -> bool { fn parse_date(value: &str) -> Option> { DateTime::parse_from_rfc3339(value) .ok() - .map(|d| d.with_timezone(&Local)) + .and_then(|date_time| { + Local + .from_local_datetime(&date_time.naive_local()) + .earliest() + }) } fn compare_i64(a: i64, op: Compare, b: i64) -> bool { match op { @@ -346,7 +350,7 @@ impl Parser { #[cfg(test)] mod tests { use super::*; - use crate::model::Category; + use crate::model::{Category, CategoryKind}; fn item() -> Item { Item { id: 1, @@ -365,7 +369,7 @@ mod tests { id: 1, name: "Work".into(), parent_id: None, - kind: "standard".into(), + kind: CategoryKind::Standard, match_text: String::new(), exclusive: false, }], @@ -387,4 +391,13 @@ mod tests { assert!(parse("priority nope 3").is_err()); assert!(parse("(done or open").is_err()); } + + #[test] + fn dates_are_interpreted_as_local_wall_time() { + let date = parse_date("2026-08-14T09:00:00+00:00").unwrap(); + assert_eq!( + date.format("%Y-%m-%d %H:%M").to_string(), + "2026-08-14 09:00" + ); + } } diff --git a/src/model.rs b/src/model.rs index 1ecabab..916b60b 100644 --- a/src/model.rs +++ b/src/model.rs @@ -1,3 +1,5 @@ +use std::{fmt, str::FromStr}; + #[derive(Debug, Clone, PartialEq)] pub struct Item { pub id: i64, @@ -30,7 +32,7 @@ pub struct Category { pub id: i64, pub name: String, pub parent_id: Option, - pub kind: String, + pub kind: CategoryKind, pub match_text: String, pub exclusive: bool, } @@ -39,9 +41,9 @@ pub struct Category { pub struct ViewDef { pub id: i64, pub name: String, - pub kind: String, + pub kind: ViewKind, pub filter_value: String, - pub sort_key: String, + pub sort_key: SortKey, pub show_done: bool, pub filter_expr: String, pub columns: Vec, @@ -53,7 +55,7 @@ pub struct ViewColumn { pub field: String, pub heading: String, pub width: u16, - pub aggregate: String, + pub aggregate: Aggregate, } #[derive(Debug, Clone, PartialEq)] @@ -93,7 +95,7 @@ pub struct CategoryRule { pub id: i64, pub category_id: i64, pub condition_expr: String, - pub action_kind: String, + pub action_kind: RuleActionKind, pub action_value: String, pub enabled: bool, } @@ -121,11 +123,11 @@ pub struct ItemChanges { pub struct DocumentSettings { pub description: String, pub backup_on_open: bool, - pub trash_policy: String, - pub done_policy: String, + pub trash_policy: TrashPolicy, + pub done_policy: DonePolicy, pub automatic_filing: bool, - pub date_order: String, - pub week_start: String, + pub date_order: DateOrder, + pub week_start: WeekStart, pub default_time: String, pub morning_time: String, pub afternoon_time: String, @@ -138,11 +140,11 @@ impl Default for DocumentSettings { Self { description: String::new(), backup_on_open: false, - trash_policy: "on-demand".into(), - done_policy: "keep".into(), + trash_policy: TrashPolicy::OnDemand, + done_policy: DonePolicy::Keep, automatic_filing: true, - date_order: "ymd".into(), - week_start: "monday".into(), + date_order: DateOrder::YearMonthDay, + week_start: WeekStart::Monday, default_time: "09:00".into(), morning_time: "09:00".into(), afternoon_time: "13:00".into(), @@ -151,3 +153,102 @@ impl Default for DocumentSettings { } } } +macro_rules! string_enum { + ($name:ident { $($variant:ident => $value:literal),+ $(,)? }) => { + #[derive(Debug, Clone, Copy, PartialEq, Eq)] + pub enum $name { + $($variant),+ + } + + impl $name { + pub const fn as_str(self) -> &'static str { + match self { + $(Self::$variant => $value),+ + } + } + } + + impl fmt::Display for $name { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str(self.as_str()) + } + } + + impl FromStr for $name { + type Err = String; + + fn from_str(value: &str) -> Result { + match value { + $($value => Ok(Self::$variant),)+ + _ => Err(format!("unsupported {} {value:?}", stringify!($name))), + } + } + } + }; +} + +string_enum!(CategoryKind { + Standard => "standard", + Date => "date", + Numeric => "numeric", +}); + +string_enum!(ViewKind { + List => "list", + Category => "category", + Upcoming => "upcoming", + Done => "done", + Datebook => "datebook", + Trash => "trash", +}); + +string_enum!(SortKey { + Manual => "manual", + When => "when", + Done => "done", + Priority => "priority", + Updated => "updated", +}); + +string_enum!(Aggregate { + None => "none", + Sum => "sum", + Average => "avg", + Count => "count", + Minimum => "min", + Maximum => "max", +}); + +string_enum!(RuleActionKind { + Assign => "assign", + Exclude => "exclude", + Priority => "priority", + Value => "value", + When => "when", + Alarm => "alarm", + Repeat => "repeat", + Done => "done", +}); + +string_enum!(TrashPolicy { + OnDemand => "on-demand", + OnClose => "on-close", + EndOfDay => "end-of-day", + Immediate => "immediate", +}); + +string_enum!(DonePolicy { + Keep => "keep", + Trash => "trash", +}); + +string_enum!(DateOrder { + YearMonthDay => "ymd", + MonthDayYear => "mdy", + DayMonthYear => "dmy", +}); + +string_enum!(WeekStart { + Monday => "monday", + Sunday => "sunday", +}); diff --git a/src/parser.rs b/src/parser.rs index 677c46f..536b50c 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1,78 +1,93 @@ -use anyhow::{Result, bail}; +use std::sync::LazyLock; + +use anyhow::{Context, Result, bail}; use chrono::{ Datelike, Duration, Local, Months, NaiveDate, NaiveDateTime, NaiveTime, TimeZone, Weekday, }; use regex::Regex; +use crate::model::{DateOrder, WeekStart}; + +static ISO_DATE: LazyLock = + LazyLock::new(|| Regex::new(r"\b(\d{4})-(\d{2})-(\d{2})\b").expect("valid ISO date regex")); +static NUMERIC_DATE: LazyLock = LazyLock::new(|| { + Regex::new(r"\b(\d{1,4})[./-](\d{1,2})[./-](\d{1,4})\b").expect("valid numeric date regex") +}); +static IN_DAYS: LazyLock = + LazyLock::new(|| Regex::new(r"\bin\s+(\d+)\s+days?\b").expect("valid relative date regex")); +static TIME_12H: LazyLock = LazyLock::new(|| { + Regex::new(r"\b(?:at\s+)?(\d{1,2})(?::(\d{2}))?\s*(am|pm)\b").expect("valid 12-hour time regex") +}); +static TIME_24H: LazyLock = LazyLock::new(|| { + Regex::new(r"\b(?:at\s+)([01]?\d|2[0-3]):([0-5]\d)\b").expect("valid 24-hour time regex") +}); +pub(crate) static RECURRENCE_INTERVAL: LazyLock = LazyLock::new(|| { + Regex::new(r"^every\s+(\d+)\s+(days?|weeks?|months?|years?)$").expect("valid recurrence regex") +}); + +#[derive(Debug, Clone, Copy)] +pub struct DateParseConfig<'a> { + pub date_order: DateOrder, + pub week_start: WeekStart, + pub default_time: &'a str, + pub morning_time: &'a str, + pub afternoon_time: &'a str, + pub evening_time: &'a str, +} + #[cfg(test)] pub fn extract_when_from(text: &str, now: NaiveDateTime) -> Option { - extract_when_from_configured( - text, now, "ymd", "monday", "09:00", "09:00", "13:00", "18:00", - ) + extract_when_from_configured(text, now, DateParseConfig::default()) } -#[allow(clippy::too_many_arguments)] -pub fn extract_when_configured( - text: &str, - date_order: &str, - week_start: &str, - default_time: &str, - morning_time: &str, - afternoon_time: &str, - evening_time: &str, -) -> Option { - extract_when_from_configured( - text, - Local::now().naive_local(), - date_order, - week_start, - default_time, - morning_time, - afternoon_time, - evening_time, - ) +impl Default for DateParseConfig<'static> { + fn default() -> Self { + Self { + date_order: DateOrder::YearMonthDay, + week_start: WeekStart::Monday, + default_time: "09:00", + morning_time: "09:00", + afternoon_time: "13:00", + evening_time: "18:00", + } + } +} + +pub fn extract_when_configured(text: &str, config: DateParseConfig<'_>) -> Option { + extract_when_from_configured(text, Local::now().naive_local(), config) } -#[allow(clippy::too_many_arguments)] fn extract_when_from_configured( text: &str, now: NaiveDateTime, - date_order: &str, - week_start: &str, - default_time: &str, - morning_time: &str, - afternoon_time: &str, - evening_time: &str, + config: DateParseConfig<'_>, ) -> Option { let lower = text.to_lowercase(); let mut date = None; - let iso = Regex::new(r"\b(\d{4})-(\d{2})-(\d{2})\b").unwrap(); - if let Some(c) = iso.captures(&lower) { + if let Some(c) = ISO_DATE.captures(&lower) { date = NaiveDate::from_ymd_opt(c[1].parse().ok()?, c[2].parse().ok()?, c[3].parse().ok()?); } - if date.is_none() { - let numeric = Regex::new(r"\b(\d{1,4})[./-](\d{1,2})[./-](\d{1,4})\b").unwrap(); - if let Some(c) = numeric.captures(&lower) { - let values = [ - c[1].parse::().ok()?, - c[2].parse::().ok()?, - c[3].parse::().ok()?, - ]; - let (year, month, day) = match date_order { - "mdy" => (values[2], values[0], values[1]), - "dmy" => (values[2], values[1], values[0]), - _ => (values[0], values[1], values[2]), - }; - let year = if year < 100 { year + 2000 } else { year }; - date = NaiveDate::from_ymd_opt(year, month as u32, day as u32); - } + if date.is_none() + && let Some(c) = NUMERIC_DATE.captures(&lower) + { + let values = [ + c[1].parse::().ok()?, + c[2].parse::().ok()?, + c[3].parse::().ok()?, + ]; + let (year, month, day) = match config.date_order { + DateOrder::MonthDayYear => (values[2], values[0], values[1]), + DateOrder::DayMonthYear => (values[2], values[1], values[0]), + DateOrder::YearMonthDay => (values[0], values[1], values[2]), + }; + let year = if year < 100 { year + 2000 } else { year }; + date = NaiveDate::from_ymd_opt(year, month as u32, day as u32); } if date.is_none() { - let in_days = Regex::new(r"\bin\s+(\d+)\s+days?\b").unwrap(); - if let Some(c) = in_days.captures(&lower) { + if let Some(c) = IN_DAYS.captures(&lower) { date = Some(now.date() + Duration::days(c[1].parse().ok()?)); } else if lower.contains("day after tomorrow") { date = Some(now.date() + Duration::days(2)); @@ -84,7 +99,7 @@ fn extract_when_from_configured( } if date.is_none() && (lower.contains("this week") || lower.contains("next week")) { - let day_from_start = if week_start == "sunday" { + let day_from_start = if config.week_start == WeekStart::Sunday { now.weekday().num_days_from_sunday() as i64 } else { now.weekday().num_days_from_monday() as i64 @@ -126,9 +141,7 @@ fn extract_when_from_configured( } let date = date?; - let time_re = Regex::new(r"\b(?:at\s+)?(\d{1,2})(?::(\d{2}))?\s*(am|pm)\b").unwrap(); - let time24_re = Regex::new(r"\b(?:at\s+)([01]?\d|2[0-3]):([0-5]\d)\b").unwrap(); - let time = if let Some(c) = time_re.captures(&lower) { + let time = if let Some(c) = TIME_12H.captures(&lower) { let mut hour: u32 = c[1].parse().ok()?; let minute: u32 = c.get(2).map_or("0", |v| v.as_str()).parse().ok()?; if hour == 12 { @@ -138,19 +151,19 @@ fn extract_when_from_configured( hour += 12; } NaiveTime::from_hms_opt(hour, minute, 0)? - } else if let Some(c) = time24_re.captures(&lower) { + } else if let Some(c) = TIME_24H.captures(&lower) { NaiveTime::from_hms_opt(c[1].parse().ok()?, c[2].parse().ok()?, 0)? } else if lower.contains("morning") { - parse_clock(morning_time)? + parse_clock(config.morning_time)? } else if lower.contains("afternoon") { - parse_clock(afternoon_time)? + parse_clock(config.afternoon_time)? } else if lower.contains("evening") || lower.contains("tonight") { - parse_clock(evening_time)? + parse_clock(config.evening_time)? } else { - parse_clock(default_time)? + parse_clock(config.default_time)? }; - let local = Local.from_local_datetime(&date.and_time(time)).single()?; + let local = Local.from_local_datetime(&date.and_time(time)).earliest()?; Some(local.to_rfc3339()) } @@ -181,7 +194,9 @@ pub fn format_when(value: Option<&str>, date_format: &str, clock_24h: bool) -> S } pub fn next_occurrence(current: &str, recurrence: &str) -> Result { - let current = chrono::DateTime::parse_from_rfc3339(current)?.with_timezone(&Local); + // Recurrences are calendar events: the date and clock reading written in the + // document are authoritative, while the stored UTC offset is informational. + let current = chrono::DateTime::parse_from_rfc3339(current)?.naive_local(); let rule = recurrence.trim().to_lowercase(); let next = match rule.as_str() { "daily" => current + Duration::days(1), @@ -200,13 +215,15 @@ pub fn next_occurrence(current: &str, recurrence: &str) -> Result { .checked_add_months(Months::new(12)) .ok_or_else(|| anyhow::anyhow!("yearly date is out of range"))?, _ => { - let re = Regex::new(r"^every\s+(\d+)\s+(days?|weeks?|months?|years?)$").unwrap(); - let Some(c) = re.captures(&rule) else { + let Some(c) = RECURRENCE_INTERVAL.captures(&rule) else { bail!( "recurrence must be daily, weekdays, weekly, monthly, yearly, or 'every N days/weeks/months/years'" ) }; let n: u32 = c[1].parse()?; + if n == 0 { + bail!("recurrence interval must be greater than zero"); + } match &c[2] { "day" | "days" => current + Duration::days(n as i64), "week" | "weeks" => current + Duration::weeks(n as i64), @@ -219,6 +236,10 @@ pub fn next_occurrence(current: &str, recurrence: &str) -> Result { } } }; + let next = Local + .from_local_datetime(&next) + .earliest() + .context("next occurrence does not exist in the local timezone")?; Ok(next.to_rfc3339()) } @@ -266,17 +287,17 @@ mod tests { #[test] fn honors_document_date_and_named_time_settings() { - let got = extract_when_from_configured( - "Planning on 18/08/2026 in the evening", - base(), - "dmy", - "monday", - "08:30", - "08:00", - "14:00", - "19:15", - ) - .unwrap(); + let config = DateParseConfig { + date_order: DateOrder::DayMonthYear, + week_start: WeekStart::Monday, + default_time: "08:30", + morning_time: "08:00", + afternoon_time: "14:00", + evening_time: "19:15", + }; + let got = + extract_when_from_configured("Planning on 18/08/2026 in the evening", base(), config) + .unwrap(); assert!(got.contains("2026-08-18T19:15:00")); assert_eq!( format_when(Some(&got), "european", false), @@ -285,14 +306,19 @@ mod tests { let next_week = extract_when_from_configured( "Review next week", base(), - "ymd", - "sunday", - "09:00", - "09:00", - "13:00", - "18:00", + DateParseConfig { + week_start: WeekStart::Sunday, + ..DateParseConfig::default() + }, ) .unwrap(); assert!(next_week.contains("2026-08-23T09:00:00")); } + + #[test] + fn recurrence_uses_the_written_wall_time_and_rejects_zero_intervals() { + let next = next_occurrence("2026-08-14T09:00:00+00:00", "daily").unwrap(); + assert!(next.contains("2026-08-15T09:00:00")); + assert!(next_occurrence("2026-08-14T09:00:00+00:00", "every 0 days").is_err()); + } } diff --git a/src/ui.rs b/src/ui.rs index 68247d4..9c76c83 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -12,7 +12,7 @@ use ratatui::{ use crate::{ app::{App, FormKind, InputKind, Mode, form_choices}, filter, - model::{Item, ViewColumn}, + model::{Aggregate, Item, ViewColumn, ViewKind}, parser::format_when, }; @@ -232,7 +232,7 @@ fn draw_items(frame: &mut Frame<'_>, app: &mut App, area: Rect) { ); return; } - if app.current_view().kind == "datebook" { + if app.current_view().kind == ViewKind::Datebook { draw_datebook(frame, app, area); return; } @@ -261,7 +261,7 @@ fn draw_items(frame: &mut Frame<'_>, app: &mut App, area: Rect) { for index in 0..app.items.len() { display.push(DisplayLine::Item(index)); } - if columns.iter().any(|c| c.aggregate != "none") { + if columns.iter().any(|c| c.aggregate != Aggregate::None) { display.push(DisplayLine::Aggregate((0..app.items.len()).collect())); } } else { @@ -285,7 +285,7 @@ fn draw_items(frame: &mut Frame<'_>, app: &mut App, area: Rect) { for index in &indices { display.push(DisplayLine::Item(*index)); } - if columns.iter().any(|c| c.aggregate != "none") { + if columns.iter().any(|c| c.aggregate != Aggregate::None) { display.push(DisplayLine::Aggregate(indices)); } } @@ -448,10 +448,10 @@ fn column_value(column: &ViewColumn, item: &Item, app: &App) -> String { } } fn aggregate_value(column: &ViewColumn, indices: &[usize], items: &[Item], app: &App) -> String { - if column.aggregate == "none" { + if column.aggregate == Aggregate::None { return String::new(); } - if column.aggregate == "count" { + if column.aggregate == Aggregate::Count { return format!("count {}", indices.len()); } let values = indices @@ -465,12 +465,12 @@ fn aggregate_value(column: &ViewColumn, indices: &[usize], items: &[Item], app: if values.is_empty() { return String::new(); } - let value = match column.aggregate.as_str() { - "sum" => values.iter().sum(), - "avg" => values.iter().sum::() / values.len() as f64, - "min" => values.iter().copied().fold(f64::INFINITY, f64::min), - "max" => values.iter().copied().fold(f64::NEG_INFINITY, f64::max), - _ => return String::new(), + let value = match column.aggregate { + Aggregate::Sum => values.iter().sum(), + Aggregate::Average => values.iter().sum::() / values.len() as f64, + Aggregate::Minimum => values.iter().copied().fold(f64::INFINITY, f64::min), + Aggregate::Maximum => values.iter().copied().fold(f64::NEG_INFINITY, f64::max), + Aggregate::None | Aggregate::Count => return String::new(), }; format!("{} {}", column.aggregate, format_number(value, app)) } @@ -1787,7 +1787,7 @@ mod tests { let mut db = Database::open(&path).unwrap(); let mut settings = db.document_settings().unwrap(); settings.note_tab_width = 3; - settings.trash_policy = "immediate".into(); + settings.trash_policy = crate::model::TrashPolicy::Immediate; db.save_document_settings(&settings).unwrap(); db.add_item("Disposable item").unwrap(); let mut app = App::new_with_preferences(