Merge branch 'headline_extra_properties'

src/compare/compare_field.rs

@@ -353,6 +353,15 @@ where
     let rust_value: Option<Vec<RV>> = rust_value.map(|it| it.collect());
     match (value, rust_value) {
         (None, None) => {}
+        (Some(el), None)
+            if el.len() == 1
+                && el.into_iter().all(|t| {
+                    if let Ok(r#""""#) = t.as_atom() {
+                        true
+                    } else {
+                        false
+                    }
+                }) => {}
         (None, rv @ Some(_)) | (Some(_), rv @ None) => {
             let this_status = DiffStatus::Bad;
             let message = Some(format!(

src/compare/diff.rs

@@ -2,7 +2,6 @@ use std::borrow::Cow;
 // TODO: Update all to use the macro to assert there are no unexpected keys.
 // TODO: Check all compare funtions for whether they correctly iterate children.
 use std::collections::BTreeSet;
-use std::collections::HashSet;
 
 use super::compare_field::compare_identity;
 use super::compare_field::compare_noop;
@@ -25,8 +24,6 @@ use super::util::assert_no_children;
 use super::util::compare_children;
 use super::util::compare_children_iter;
 use super::util::compare_standard_properties;
-use super::util::get_property;
-use super::util::get_property_boolean;
 use super::util::get_property_quoted_string;
 use crate::compare::compare_field::ComparePropertiesResult;
 use crate::compare::compare_field::EmacsField;
@@ -48,7 +45,6 @@ use crate::types::Date;
 use crate::types::DayOfMonth;
 use crate::types::DiarySexp;
 use crate::types::Document;
-use crate::types::DocumentElement;
 use crate::types::Drawer;
 use crate::types::DynamicBlock;
 use crate::types::Entity;
@@ -82,7 +78,6 @@ use crate::types::PlainListItem;
 use crate::types::PlainListType;
 use crate::types::PlainText;
 use crate::types::Planning;
-use crate::types::PriorityCookie;
 use crate::types::PropertyDrawer;
 use crate::types::QuoteBlock;
 use crate::types::RadioLink;
@@ -533,7 +528,7 @@ fn compare_section<'b, 's>(
 }
 
 #[allow(dead_code)]
-fn new_compare_heading<'b, 's>(
+fn compare_heading<'b, 's>(
     source: &'s str,
     emacs: &'b Token<'s>,
     rust: &'b Heading<'s>,
@@ -542,7 +537,10 @@ fn new_compare_heading<'b, 's>(
     let mut child_status = Vec::new();
     let mut message = None;
 
-    // TODO: This needs to support additional properties from the property drawer
+    let additional_property_names: Vec<String> = rust
+        .get_additional_properties()
+        .map(|node_property| format!(":{}", node_property.property_name.to_uppercase()))
+        .collect();
 
     compare_children(
         source,
@@ -557,6 +555,10 @@ fn new_compare_heading<'b, 's>(
         source,
         emacs,
         rust,
+        additional_property_names
+            .iter()
+            .map(String::as_str)
+            .map(EmacsField::Required),
         (
             EmacsField::Required(":level"),
             |r| Some(r.level),
@@ -664,278 +666,6 @@ fn new_compare_heading<'b, 's>(
     .into())
 }
 
-fn compare_heading<'b, 's>(
-    source: &'s str,
-    emacs: &'b Token<'s>,
-    rust: &'b Heading<'s>,
-) -> Result<DiffEntry<'b, 's>, Box<dyn std::error::Error>> {
-    let children = emacs.as_list()?;
-    let mut child_status = Vec::new();
-    let mut this_status = DiffStatus::Good;
-    let mut message = None;
-
-    // Compare level
-    let level = get_property(emacs, ":level")?
-        .ok_or("Level should not be nil")?
-        .as_atom()?;
-    if rust.level.to_string() != level {
-        this_status = DiffStatus::Bad;
-        message = Some(format!(
-            "Headline level do not match (emacs != rust): {} != {}",
-            level, rust.level
-        ))
-    }
-
-    // Compare tags
-    let emacs_tags = get_tags_from_heading(emacs)?;
-    let emacs_tags: HashSet<_> = emacs_tags.iter().map(|val| val.as_str()).collect();
-    let rust_tags: HashSet<&str> = rust.tags.iter().map(|val| *val).collect();
-    let difference: Vec<&str> = emacs_tags
-        .symmetric_difference(&rust_tags)
-        .map(|val| *val)
-        .collect();
-    if !difference.is_empty() {
-        this_status = DiffStatus::Bad;
-        message = Some(format!("Mismatched tags: {}", difference.join(", ")));
-    }
-
-    // Compare todo-keyword
-    let todo_keyword = get_property(emacs, ":todo-keyword")?
-        .map(Token::as_atom)
-        .map_or(Ok(None), |r| r.map(Some))?
-        .unwrap_or("nil");
-    match (todo_keyword, &rust.todo_keyword, unquote(todo_keyword)) {
-        ("nil", None, _) => {}
-        (_, Some((_rust_todo_type, rust_todo)), Ok(emacs_todo)) if emacs_todo == *rust_todo => {}
-        (emacs_todo, rust_todo, _) => {
-            this_status = DiffStatus::Bad;
-            message = Some(format!(
-                "(emacs != rust) {:?} != {:?}",
-                emacs_todo, rust_todo
-            ));
-        }
-    };
-    // Compare todo-type
-    let todo_type = get_property(emacs, ":todo-type")?
-        .map(Token::as_atom)
-        .map_or(Ok(None), |r| r.map(Some))?
-        .unwrap_or("nil");
-    // todo-type is an unquoted string either todo, done, or nil
-    match (todo_type, &rust.todo_keyword) {
-        ("nil", None) => {}
-        ("todo", Some((TodoKeywordType::Todo, _))) => {}
-        ("done", Some((TodoKeywordType::Done, _))) => {}
-        (emacs_todo, rust_todo) => {
-            this_status = DiffStatus::Bad;
-            message = Some(format!(
-                "(emacs != rust) {:?} != {:?}",
-                emacs_todo, rust_todo
-            ));
-        }
-    };
-
-    // Compare title
-    let title = get_property(emacs, ":title")?;
-    match (title, rust.title.len()) {
-        (None, 0) => {}
-        (None, _) => {
-            this_status = DiffStatus::Bad;
-            message = Some(format!(
-                "Titles do not match (emacs != rust): {:?} != {:?}",
-                title, rust.title
-            ))
-        }
-        (Some(title), _) => {
-            let title_status = title
-                .as_list()?
-                .iter()
-                .zip(rust.title.iter())
-                .map(|(emacs_child, rust_child)| {
-                    compare_ast_node(source, emacs_child, rust_child.into())
-                })
-                .collect::<Result<Vec<_>, _>>()?;
-            child_status.push(artificial_diff_scope("title", title_status)?);
-        }
-    };
-
-    // Compare priority
-    let priority = get_property(emacs, ":priority")?;
-    match (priority, rust.priority_cookie) {
-        (None, None) => {}
-        (None, Some(_)) | (Some(_), None) => {
-            this_status = DiffStatus::Bad;
-            message = Some(format!(
-                "Priority cookie mismatch (emacs != rust) {:?} != {:?}",
-                priority, rust.priority_cookie
-            ));
-        }
-        (Some(emacs_priority_cookie), Some(rust_priority_cookie)) => {
-            let emacs_priority_cookie =
-                emacs_priority_cookie.as_atom()?.parse::<PriorityCookie>()?;
-            if emacs_priority_cookie != rust_priority_cookie {
-                this_status = DiffStatus::Bad;
-                message = Some(format!(
-                    "Priority cookie mismatch (emacs != rust) {:?} != {:?}",
-                    emacs_priority_cookie, rust_priority_cookie
-                ));
-            }
-        }
-    }
-
-    // Compare archived
-    let archived = get_property(emacs, ":archivedp")?;
-    match (archived, rust.is_archived) {
-        (None, true) | (Some(_), false) => {
-            this_status = DiffStatus::Bad;
-            message = Some(format!(
-                "archived mismatch (emacs != rust) {:?} != {:?}",
-                archived, rust.is_archived
-            ));
-        }
-        (None, false) | (Some(_), true) => {}
-    }
-
-    // Compare commented
-    let commented = get_property(emacs, ":commentedp")?;
-    match (commented, rust.is_comment) {
-        (None, true) | (Some(_), false) => {
-            this_status = DiffStatus::Bad;
-            message = Some(format!(
-                "commented mismatch (emacs != rust) {:?} != {:?}",
-                commented, rust.is_comment
-            ));
-        }
-        (None, false) | (Some(_), true) => {}
-    }
-
-    // Compare raw-value
-    let raw_value = get_property_quoted_string(emacs, ":raw-value")?
-        .ok_or("Headlines should have :raw-value.")?;
-    let rust_raw_value = rust.get_raw_value();
-    if raw_value != rust_raw_value {
-        this_status = DiffStatus::Bad;
-        message = Some(format!(
-            "raw-value mismatch (emacs != rust) {:?} != {:?}",
-            raw_value, rust_raw_value
-        ));
-    }
-
-    // Compare footnote-section-p
-    let footnote_section = get_property_boolean(emacs, ":footnote-section-p")?;
-    if footnote_section != rust.is_footnote_section {
-        this_status = DiffStatus::Bad;
-        message = Some(format!(
-            "footnote section mismatch (emacs != rust) {:?} != {:?}",
-            footnote_section, rust.is_footnote_section
-        ));
-    }
-
-    // Compare scheduled
-    let scheduled = get_property(emacs, ":scheduled")?;
-    match (scheduled, &rust.scheduled) {
-        (None, None) => {}
-        (None, Some(_)) | (Some(_), None) => {
-            this_status = DiffStatus::Bad;
-            message = Some(format!(
-                "Scheduled mismatch (emacs != rust) {:?} != {:?}",
-                scheduled, rust.scheduled
-            ));
-        }
-        (Some(emacs_child), Some(rust_child)) => {
-            let result = compare_ast_node(source, emacs_child, rust_child.into())?;
-            child_status.push(artificial_diff_scope("scheduled", vec![result])?);
-        }
-    }
-
-    // Compare deadline
-    let deadline = get_property(emacs, ":deadline")?;
-    match (deadline, &rust.deadline) {
-        (None, None) => {}
-        (None, Some(_)) | (Some(_), None) => {
-            this_status = DiffStatus::Bad;
-            message = Some(format!(
-                "Deadline mismatch (emacs != rust) {:?} != {:?}",
-                deadline, rust.deadline
-            ));
-        }
-        (Some(emacs_child), Some(rust_child)) => {
-            let result = compare_ast_node(source, emacs_child, rust_child.into())?;
-            child_status.push(artificial_diff_scope("deadline", vec![result])?);
-        }
-    }
-
-    // Compare closed
-    let closed = get_property(emacs, ":closed")?;
-    match (closed, &rust.closed) {
-        (None, None) => {}
-        (None, Some(_)) | (Some(_), None) => {
-            this_status = DiffStatus::Bad;
-            message = Some(format!(
-                "Closed mismatch (emacs != rust) {:?} != {:?}",
-                closed, rust.closed
-            ));
-        }
-        (Some(emacs_child), Some(rust_child)) => {
-            let result = compare_ast_node(source, emacs_child, rust_child.into())?;
-            child_status.push(artificial_diff_scope("closed", vec![result])?);
-        }
-    }
-
-    // TODO: Compare :pre-blank
-
-    // Compare section
-    let section_status = children
-        .iter()
-        .skip(2)
-        .zip(rust.children.iter())
-        .map(|(emacs_child, rust_child)| match rust_child {
-            DocumentElement::Heading(rust_heading) => {
-                compare_ast_node(source, emacs_child, rust_heading.into())
-            }
-            DocumentElement::Section(rust_section) => {
-                compare_ast_node(source, emacs_child, rust_section.into())
-            }
-        })
-        .collect::<Result<Vec<_>, _>>()?;
-    child_status.push(artificial_diff_scope("section", section_status)?);
-
-    Ok(DiffResult {
-        status: this_status,
-        name: rust.get_elisp_name(),
-        message,
-        children: child_status,
-        rust_source: rust.get_source(),
-        emacs_token: emacs,
-    }
-    .into())
-}
-
-fn get_tags_from_heading<'b, 's>(
-    emacs: &'b Token<'s>,
-) -> Result<HashSet<String>, Box<dyn std::error::Error>> {
-    let tags = match get_property(emacs, ":tags")? {
-        Some(prop) => prop,
-        None => return Ok(HashSet::new()),
-    };
-
-    match tags.as_atom() {
-        Ok(val) => panic!("Unexpected value for tags: {:?}", val),
-        Err(_) => {}
-    };
-    let tags = {
-        let tags = tags.as_list()?;
-        let strings = tags
-            .iter()
-            .map(Token::as_atom)
-            .collect::<Result<Vec<&str>, _>>()?;
-        strings
-            .into_iter()
-            .map(unquote)
-            .collect::<Result<HashSet<String>, _>>()?
-    };
-    Ok(tags)
-}
-
 fn compare_paragraph<'b, 's>(
     source: &'s str,
     emacs: &'b Token<'s>,

src/compare/macros.rs

@@ -96,6 +96,90 @@ macro_rules! compare_properties {
             new_status
         }
     };
+    // Specifies additional properties
+    ($source:expr, $emacs:expr, $rust:expr, $additionalproperties: expr, $(($emacs_field:expr, $rust_value_getter:expr, $compare_fn: expr)),+) => {
+        {
+            let mut new_status = Vec::new();
+            let children = $emacs.as_list()?;
+            let attributes_child = children
+            .iter()
+            .nth(1)
+            .ok_or("Should have an attributes child.")?;
+            let attributes_map = attributes_child.as_map()?;
+            let mut emacs_keys: BTreeSet<&str> = attributes_map.keys().map(|s| *s).collect();
+            if emacs_keys.contains(":standard-properties") {
+                emacs_keys.remove(":standard-properties");
+            } else {
+                new_status.push(ComparePropertiesResult::SelfChange(DiffStatus::Bad, Some(format!(
+                    "Emacs token lacks :standard-properties field.",
+                ))));
+            }
+            for additional_field in $additionalproperties {
+                match additional_field {
+                    EmacsField::Required(name) if emacs_keys.contains(name) => {
+                        emacs_keys.remove(name);
+                    },
+                    EmacsField::Optional(name) if emacs_keys.contains(name) => {
+                        emacs_keys.remove(name);
+                    },
+                    EmacsField::Required(name) => {
+                        new_status.push(ComparePropertiesResult::SelfChange(DiffStatus::Bad, Some(format!(
+                            "Emacs token lacks required field: {}",
+                            name
+                        ))));
+                    },
+                    EmacsField::Optional(_name) => {},
+                }
+            }
+            $(
+            match $emacs_field {
+                EmacsField::Required(name) if emacs_keys.contains(name) => {
+                    emacs_keys.remove(name);
+                },
+                EmacsField::Optional(name) if emacs_keys.contains(name) => {
+                    emacs_keys.remove(name);
+                },
+                EmacsField::Required(name) => {
+                    new_status.push(ComparePropertiesResult::SelfChange(DiffStatus::Bad, Some(format!(
+                        "Emacs token lacks required field: {}",
+                        name
+                    ))));
+                },
+                EmacsField::Optional(_name) => {},
+            }
+            )+
+
+            if !emacs_keys.is_empty() {
+                let unexpected_keys: Vec<&str> = emacs_keys.into_iter().collect();
+                let unexpected_keys = unexpected_keys.join(", ");
+                new_status.push(ComparePropertiesResult::SelfChange(DiffStatus::Bad, Some(format!(
+                    "Emacs token had extra field(s): {}",
+                    unexpected_keys
+                ))));
+            }
+
+            $(
+            let emacs_name = match $emacs_field {
+                EmacsField::Required(name) => {
+                    name
+                },
+                EmacsField::Optional(name) => {
+                    name
+                },
+            };
+            let result = $compare_fn($source, $emacs, $rust, emacs_name, $rust_value_getter)?;
+            match result {
+                ComparePropertiesResult::SelfChange(DiffStatus::Good, _) => unreachable!("No comparison functions should return SelfChange() when DiffStatus is good."),
+                ComparePropertiesResult::NoChange => {},
+                result => {
+                    new_status.push(result);
+                }
+            }
+            )+
+
+            new_status
+        }
+    };
     // Default case for when there are no expected properties except for :standard-properties
     ($emacs:expr) => {
         {

src/compare/util.rs

@@ -221,22 +221,6 @@ pub(crate) fn get_property_quoted_string<'b, 's, 'x>(
         .map_or(Ok(None), |r| r.map(Some))?)
 }
 
-/// Get a named property containing a boolean value.
-///
-/// This uses the elisp convention of nil == false, non-nil == true.
-///
-/// Returns false if key is not found.
-pub(crate) fn get_property_boolean<'b, 's, 'x>(
-    emacs: &'b Token<'s>,
-    key: &'x str,
-) -> Result<bool, Box<dyn std::error::Error>> {
-    Ok(get_property(emacs, key)?
-        .map(Token::as_atom)
-        .map_or(Ok(None), |r| r.map(Some))?
-        .unwrap_or("nil")
-        != "nil")
-}
-
 /// Get a named property containing an unquoted numeric value.
 ///
 /// Returns None if key is not found.

src/types/document.rs

@@ -2,6 +2,7 @@ use std::path::PathBuf;
 
 use super::Element;
 use super::GetStandardProperties;
+use super::NodeProperty;
 use super::Object;
 use super::StandardProperties;
 use super::Timestamp;
@@ -90,4 +91,23 @@ impl<'s> Heading<'s> {
             .collect();
         title_source
     }
+
+    pub fn get_additional_properties(&self) -> impl Iterator<Item = &NodeProperty<'s>> {
+        let foo = self
+            .children
+            .iter()
+            .take(1)
+            .filter_map(|c| match c {
+                DocumentElement::Section(section) => Some(section),
+                _ => None,
+            })
+            .flat_map(|section| section.children.iter())
+            .take(1)
+            .filter_map(|element| match element {
+                Element::PropertyDrawer(property_drawer) => Some(property_drawer),
+                _ => None,
+            })
+            .flat_map(|property_drawer| property_drawer.children.iter());
+        foo
+    }
 }