From b93ee2885f36d2b305429615a7d629d46a24ed54 Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Wed, 19 Apr 2023 14:14:16 -0400 Subject: [PATCH 01/11] Add a script to trigger specific integration tests. --- scripts/run_integration_test.bash | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100755 scripts/run_integration_test.bash diff --git a/scripts/run_integration_test.bash b/scripts/run_integration_test.bash new file mode 100755 index 0000000..bfb1029 --- /dev/null +++ b/scripts/run_integration_test.bash @@ -0,0 +1,23 @@ +#!/usr/bin/env bash +# +set -euo pipefail +IFS=$'\n\t' +DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" + +cd "$DIR/../" + +samples_dir=$(readlink -f "org_mode_samples") + +function get_test_names { + for test_file in "$@" + do + test_file_full_path=$(readlink -f "$test_file") + relative_to_samples=$(realpath --relative-to "$samples_dir" "$test_file_full_path") + without_extension="${relative_to_samples%.org}" + echo "${without_extension/\//_}" + done +} + +get_test_names "$@" | while read test; do + cargo test --no-fail-fast --test test_loader "$test" +done From 71bc1a5679eae65584e3c3d4f9202138db1d6a64 Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Wed, 19 Apr 2023 14:17:31 -0400 Subject: [PATCH 02/11] Support passing in test names instead of files. --- scripts/run_integration_test.bash | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/scripts/run_integration_test.bash b/scripts/run_integration_test.bash index bfb1029..9b19ecc 100755 --- a/scripts/run_integration_test.bash +++ b/scripts/run_integration_test.bash @@ -11,10 +11,14 @@ samples_dir=$(readlink -f "org_mode_samples") function get_test_names { for test_file in "$@" do - test_file_full_path=$(readlink -f "$test_file") - relative_to_samples=$(realpath --relative-to "$samples_dir" "$test_file_full_path") - without_extension="${relative_to_samples%.org}" - echo "${without_extension/\//_}" + if [ -e "$test_file" ]; then + test_file_full_path=$(readlink -f "$test_file") + relative_to_samples=$(realpath --relative-to "$samples_dir" "$test_file_full_path") + without_extension="${relative_to_samples%.org}" + echo "${without_extension/\//_}" + else + echo "$test_file" + fi done } From c6fd04e9ce0cef1519af77dec6c0f2b0c168fdb8 Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Wed, 19 Apr 2023 14:30:02 -0400 Subject: [PATCH 03/11] Fix footnote definition end matcher detecting the next footnote definition. --- src/parser/footnote_definition.rs | 16 +++++++++++++--- toy_language.txt | 13 ++++++++----- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/parser/footnote_definition.rs b/src/parser/footnote_definition.rs index 6f4fa12..ab07ab6 100644 --- a/src/parser/footnote_definition.rs +++ b/src/parser/footnote_definition.rs @@ -83,10 +83,18 @@ fn footnote_definition_end<'r, 's>( input: &'s str, ) -> Res<&'s str, &'s str> { let start_of_line_matcher = parser_with_context!(start_of_line)(context); - let footnote_definition_matcher = parser_with_context!(footnote_definition)(context); + let allow_nesting_context = + context.with_additional_node(ContextElement::Context("allow nesting footnotes")); + let footnote_definition_matcher = parser_with_context!(footnote_definition)( + if immediate_in_section(context, "footnote definition") { + &allow_nesting_context + } else { + context + }, + ); let maybe_consume_trailing_whitespace_matcher = parser_with_context!(maybe_consume_trailing_whitespace)(context); - alt(( + let (remaining, source) = alt(( recognize(tuple(( maybe_consume_trailing_whitespace_matcher, footnote_definition_matcher, @@ -95,7 +103,9 @@ fn footnote_definition_end<'r, 's>( start_of_line_matcher, verify(many1(blank_line), |lines: &Vec<&str>| lines.len() >= 2), ))), - ))(input) + ))(input)?; + + Ok((remaining, source)) } #[cfg(test)] diff --git a/toy_language.txt b/toy_language.txt index 8b8048b..0c7d856 100644 --- a/toy_language.txt +++ b/toy_language.txt @@ -1,5 +1,8 @@ -:firstdrawer: -:seconddrawer: -foo -:end: -:end: +[fn:1] A footnote. + +[fn:2] A multi- + +line footnote. + + +not in the footnote. From 35aab10155b2c1387c75c6196e5e9718749178ea Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Wed, 19 Apr 2023 14:40:10 -0400 Subject: [PATCH 04/11] Add support for marking integration tests as expected to fail. --- build.rs | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/build.rs b/build.rs index 3a56b09..4681b90 100644 --- a/build.rs +++ b/build.rs @@ -22,17 +22,28 @@ fn main() { .unwrap_or(false) } Err(_) => true, - }).collect::, _>>().unwrap(); + }) + .collect::, _>>() + .unwrap(); for test in test_files { write_test(&mut test_file, &test); } - } fn write_test(test_file: &mut File, test: &walkdir::DirEntry) { - let test_name = test.path().strip_prefix("org_mode_samples/").expect("Paths should be under org_mode_samples/").to_string_lossy().to_lowercase().strip_suffix(".org").expect("Should have .org extension").replace("/", "_"); - + let test_name = test + .path() + .strip_prefix("org_mode_samples/") + .expect("Paths should be under org_mode_samples/") + .to_string_lossy() + .to_lowercase() + .strip_suffix(".org") + .expect("Should have .org extension") + .replace("/", "_"); + if let Some(reason) = is_expect_fail(test_name.as_str()) { + write!(test_file, "#[ignore]\n").unwrap(); + } write!( test_file, include_str!("./tests/test_template"), @@ -56,3 +67,10 @@ use organic::sexp; ) .unwrap(); } + +fn is_expect_fail(name: &str) -> Option<&str> { + match name { + "drawer_drawer_with_headline_inside" => Some("Apparently lines with :end: become their own paragraph. This odd behavior needs to be investigated more."), + _ => None, + } +} From 4a31dd6db07de943fc61bd7708971b621c3107e9 Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Wed, 19 Apr 2023 14:45:32 -0400 Subject: [PATCH 05/11] Mark some tests that need keyword implemented as expect fail. --- build.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/build.rs b/build.rs index 4681b90..3794492 100644 --- a/build.rs +++ b/build.rs @@ -71,6 +71,10 @@ use organic::sexp; fn is_expect_fail(name: &str) -> Option<&str> { match name { "drawer_drawer_with_headline_inside" => Some("Apparently lines with :end: become their own paragraph. This odd behavior needs to be investigated more."), + "element_container_priority_drawer_dynamic_block" => Some("Keyword needs to be implemented."), + "element_container_priority_dynamic_block_dynamic_block" => Some("Keyword needs to be implemented."), + "element_container_priority_footnote_definition_dynamic_block" => Some("Keyword needs to be implemented."), + "element_container_priority_greater_block_dynamic_block" => Some("Keyword needs to be implemented."), _ => None, } } From 2b08b52c16db2879b3356daba08f503e20afefe6 Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Wed, 19 Apr 2023 14:52:31 -0400 Subject: [PATCH 06/11] Support drawers with only whitespace. --- src/parser/drawer.rs | 17 +++++++++++++++-- toy_language.txt | 9 +++------ 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/parser/drawer.rs b/src/parser/drawer.rs index 44871e7..d8f2f87 100644 --- a/src/parser/drawer.rs +++ b/src/parser/drawer.rs @@ -3,6 +3,7 @@ use nom::bytes::complete::tag; use nom::bytes::complete::take_while; use nom::character::complete::line_ending; use nom::character::complete::space0; +use nom::combinator::consumed; use nom::combinator::eof; use nom::combinator::recognize; use nom::multi::many_till; @@ -17,6 +18,7 @@ use crate::parser::exiting::ExitClass; use crate::parser::parser_context::ContextElement; use crate::parser::parser_context::ExitMatcherNode; use crate::parser::parser_with_context::parser_with_context; +use crate::parser::util::blank_line; use crate::parser::util::exit_matcher_parser; use crate::parser::util::get_consumed; use crate::parser::util::immediate_in_section; @@ -24,6 +26,8 @@ use crate::parser::util::maybe_consume_trailing_whitespace_if_not_exiting; use crate::parser::util::start_of_line; use crate::parser::util::WORD_CONSTITUENT_CHARACTERS; use crate::parser::Drawer; +use crate::parser::Element; +use crate::parser::Paragraph; #[tracing::instrument(ret, level = "debug")] pub fn drawer<'r, 's>(context: Context<'r, 's>, input: &'s str) -> Res<&'s str, Drawer<'s>> { @@ -51,8 +55,17 @@ pub fn drawer<'r, 's>(context: Context<'r, 's>, input: &'s str) -> Res<&'s str, let element_matcher = parser_with_context!(element)(&parser_context); let exit_matcher = parser_with_context!(exit_matcher_parser)(&parser_context); - let (remaining, (children, _exit_contents)) = - many_till(element_matcher, exit_matcher)(remaining)?; + let (remaining, children) = match consumed(many_till(blank_line, exit_matcher))(remaining) { + Ok((remaining, (whitespace, (_children, _exit_contents)))) => ( + remaining, + vec![Element::Paragraph(Paragraph::of_text(whitespace))], + ), + Err(_) => { + let (remaining, (children, _exit_contents)) = + many_till(element_matcher, exit_matcher)(remaining)?; + (remaining, children) + } + }; let (remaining, _end) = drawer_end(&parser_context, remaining)?; let (remaining, _trailing_ws) = diff --git a/toy_language.txt b/toy_language.txt index 0c7d856..574a2f2 100644 --- a/toy_language.txt +++ b/toy_language.txt @@ -1,8 +1,5 @@ -[fn:1] A footnote. - -[fn:2] A multi- - -line footnote. +foo +:drawername: -not in the footnote. +:end: From 7ac0cee223580a668dc84d71b9f22b4295342c62 Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Wed, 19 Apr 2023 15:00:02 -0400 Subject: [PATCH 07/11] Support nesting greater blocks of different names. --- src/parser/greater_block.rs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/parser/greater_block.rs b/src/parser/greater_block.rs index 73a7deb..f13c644 100644 --- a/src/parser/greater_block.rs +++ b/src/parser/greater_block.rs @@ -35,11 +35,6 @@ pub fn greater_block<'r, 's>( input: &'s str, ) -> Res<&'s str, GreaterBlock<'s>> { // TODO: Do I need to differentiate between different greater block types. - if immediate_in_section(context, "greater block") { - return Err(nom::Err::Error(CustomError::MyError(MyError( - "Cannot nest objects of the same element", - )))); - } start_of_line(context, input)?; let (remaining, _leading_whitespace) = space0(input)?; // TODO: Not handling indentation before start of block @@ -50,11 +45,21 @@ pub fn greater_block<'r, 's>( _ => true, }), ))(remaining)?; + let context_name = match name.to_lowercase().as_str() { + "center" => "center block", + "quote" => "quote block", + _ => "greater block", + }; + if immediate_in_section(context, context_name) { + return Err(nom::Err::Error(CustomError::MyError(MyError( + "Cannot nest objects of the same element", + )))); + } let (remaining, parameters) = opt(tuple((space1, parameters)))(remaining)?; let (remaining, _nl) = line_ending(remaining)?; let parser_context = context .with_additional_node(ContextElement::ConsumeTrailingWhitespace(true)) - .with_additional_node(ContextElement::Context("greater block")) + .with_additional_node(ContextElement::Context(context_name)) .with_additional_node(ContextElement::GreaterBlock(name)) .with_additional_node(ContextElement::ExitMatcherNode(ExitMatcherNode { class: ExitClass::Alpha, From 821eacf1614ba163c38ef599fb570f4dd03b78a8 Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Wed, 19 Apr 2023 15:03:45 -0400 Subject: [PATCH 08/11] Mark more tests as expect fail. --- build.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/build.rs b/build.rs index 3794492..5994fbd 100644 --- a/build.rs +++ b/build.rs @@ -75,6 +75,9 @@ fn is_expect_fail(name: &str) -> Option<&str> { "element_container_priority_dynamic_block_dynamic_block" => Some("Keyword needs to be implemented."), "element_container_priority_footnote_definition_dynamic_block" => Some("Keyword needs to be implemented."), "element_container_priority_greater_block_dynamic_block" => Some("Keyword needs to be implemented."), + "element_container_priority_section_dynamic_block" => Some("Keyword needs to be implemented."), + "exit_matcher_investigation_table_list" => Some("Table needs to be implemented."), + "element_container_priority_readme" => Some("Table needs to be implemented."), _ => None, } } From a4c6e899ac75239bbb1df44972a694d8087c2e5a Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Wed, 19 Apr 2023 15:19:21 -0400 Subject: [PATCH 09/11] Add a utility function to check the first element in an sexp. --- src/compare/diff.rs | 114 ++++++++++++-------------------------------- src/compare/util.rs | 18 +++++++ 2 files changed, 49 insertions(+), 83 deletions(-) diff --git a/src/compare/diff.rs b/src/compare/diff.rs index 66dfa06..99287dc 100644 --- a/src/compare/diff.rs +++ b/src/compare/diff.rs @@ -1,4 +1,5 @@ use super::sexp::Token; +use super::util::assert_name; use crate::compare::util::get_offsets; use crate::parser::Comment; use crate::parser::Document; @@ -18,6 +19,7 @@ use crate::DynamicBlock; pub struct DiffResult { status: DiffStatus, name: String, + message: Option, children: Vec, } @@ -71,11 +73,7 @@ pub fn compare_document<'s>( rust: &'s Document<'s>, ) -> Result> { let children = emacs.as_list()?; - let first_child = children.first().ok_or("Should have at least one child.")?; - let first_child_text = first_child.as_atom()?; - if first_child_text != "org-data" { - return Err("Document should correspond to an org-data cell.".into()); - } + assert_name(emacs, "org-data")?; let mut child_status = Vec::new(); let mut this_status = DiffStatus::Good; @@ -112,6 +110,7 @@ pub fn compare_document<'s>( Ok(DiffResult { status: this_status, name: "document".to_owned(), + message: None, children: child_status, }) } @@ -122,11 +121,7 @@ fn compare_section<'s>( rust: &'s Section<'s>, ) -> Result> { let children = emacs.as_list()?; - let first_child = children.first().ok_or("Should have at least one child.")?; - let first_child_text = first_child.as_atom()?; - if first_child_text != "section" { - return Err("Section should correspond to a section cell.".into()); - } + assert_name(emacs, "section")?; let mut child_status = Vec::new(); let mut this_status = DiffStatus::Good; @@ -155,6 +150,7 @@ fn compare_section<'s>( Ok(DiffResult { status: this_status, name: "section".to_owned(), + message: None, children: child_status, }) } @@ -165,11 +161,7 @@ fn compare_heading<'s>( rust: &'s Heading<'s>, ) -> Result> { let children = emacs.as_list()?; - let first_child = children.first().ok_or("Should have at least one child.")?; - let first_child_text = first_child.as_atom()?; - if first_child_text != "headline" { - return Err("Heading should correspond to a headline cell.".into()); - } + assert_name(emacs, "headline")?; let mut child_status = Vec::new(); let mut this_status = DiffStatus::Good; @@ -205,6 +197,7 @@ fn compare_heading<'s>( Ok(DiffResult { status: this_status, name: "heading".to_owned(), + message: None, children: child_status, }) } @@ -231,13 +224,7 @@ fn compare_paragraph<'s>( rust: &'s Paragraph<'s>, ) -> Result> { let children = emacs.as_list()?; - let first_child = children - .first() - .ok_or("Should have at least one child.")? - .as_atom()?; - if first_child != "paragraph" { - return Err("Paragraph should correspond to a paragraph cell.".into()); - } + assert_name(emacs, "paragraph")?; let mut child_status = Vec::new(); let mut this_status = DiffStatus::Good; @@ -264,6 +251,7 @@ fn compare_paragraph<'s>( Ok(DiffResult { status: this_status, name: "paragraph".to_owned(), + message: None, children: child_status, }) } @@ -274,13 +262,7 @@ fn compare_plain_list<'s>( rust: &'s PlainList<'s>, ) -> Result> { let children = emacs.as_list()?; - let first_child = children - .first() - .ok_or("Should have at least one child.")? - .as_atom()?; - if first_child != "plain-list" { - return Err("Paragraph should correspond to a paragraph cell.".into()); - } + assert_name(emacs, "plain-list")?; let mut child_status = Vec::new(); let mut this_status = DiffStatus::Good; @@ -309,6 +291,7 @@ fn compare_plain_list<'s>( Ok(DiffResult { status: this_status, name: "plain-list".to_owned(), + message: None, children: child_status, }) } @@ -319,13 +302,7 @@ fn compare_plain_list_item<'s>( rust: &'s PlainListItem<'s>, ) -> Result> { let children = emacs.as_list()?; - let first_child = children - .first() - .ok_or("Should have at least one child.")? - .as_atom()?; - if first_child != "item" { - return Err("PlainListItem should correspond to an item cell.".into()); - } + assert_name(emacs, "item")?; let mut child_status = Vec::new(); let mut this_status = DiffStatus::Good; @@ -354,6 +331,7 @@ fn compare_plain_list_item<'s>( Ok(DiffResult { status: this_status, name: "plain-list-item".to_owned(), + message: None, children: child_status, }) } @@ -364,25 +342,14 @@ fn compare_greater_block<'s>( rust: &'s GreaterBlock<'s>, ) -> Result> { let children = emacs.as_list()?; - let first_child = children - .first() - .ok_or("Should have at least one child.")? - .as_atom()?; - match rust.name.to_lowercase().as_str() { - "center" => { - if first_child != "center-block" { - return Err( - "Center greater blocks should correspond to a center-block cell.".into(), - ); - } - } - "quote" => { - if first_child != "quote-block" { - return Err("Quote greater blocks should correspond to a quote-block cell.".into()); - } - } - _ => todo!(), - } + assert_name( + emacs, + match rust.name.to_lowercase().as_str() { + "center" => "center-block", + "quote" => "quote-block", + _ => todo!(), + }, + )?; let mut child_status = Vec::new(); let mut this_status = DiffStatus::Good; @@ -411,6 +378,7 @@ fn compare_greater_block<'s>( Ok(DiffResult { status: this_status, name: "greater-block".to_owned(), + message: None, children: child_status, }) } @@ -421,13 +389,7 @@ fn compare_dynamic_block<'s>( rust: &'s DynamicBlock<'s>, ) -> Result> { let children = emacs.as_list()?; - let first_child = children - .first() - .ok_or("Should have at least one child.")? - .as_atom()?; - if first_child != "dynamic-block" { - return Err("Dynamic block should correspond to a dynamic-block cell.".into()); - } + assert_name(emacs, "dynamic-block")?; let mut child_status = Vec::new(); let mut this_status = DiffStatus::Good; @@ -456,6 +418,7 @@ fn compare_dynamic_block<'s>( Ok(DiffResult { status: this_status, name: "dynamic-block".to_owned(), + message: None, children: child_status, }) } @@ -466,13 +429,7 @@ fn compare_footnote_definition<'s>( rust: &'s FootnoteDefinition<'s>, ) -> Result> { let children = emacs.as_list()?; - let first_child = children - .first() - .ok_or("Should have at least one child.")? - .as_atom()?; - if first_child != "footnote-definition" { - return Err("Paragraph should correspond to a paragraph cell.".into()); - } + assert_name(emacs, "footnote-definition")?; let mut child_status = Vec::new(); let mut this_status = DiffStatus::Good; @@ -501,6 +458,7 @@ fn compare_footnote_definition<'s>( Ok(DiffResult { status: this_status, name: "footnote-definition".to_owned(), + message: None, children: child_status, }) } @@ -511,13 +469,7 @@ fn compare_comment<'s>( rust: &'s Comment<'s>, ) -> Result> { let children = emacs.as_list()?; - let first_child = children - .first() - .ok_or("Should have at least one child.")? - .as_atom()?; - if first_child != "comment" { - return Err("Comment should correspond to a comment cell.".into()); - } + assert_name(emacs, "comment")?; let mut child_status = Vec::new(); let mut this_status = DiffStatus::Good; @@ -542,6 +494,7 @@ fn compare_comment<'s>( Ok(DiffResult { status: this_status, name: "comment".to_owned(), + message: None, children: child_status, }) } @@ -552,13 +505,7 @@ fn compare_drawer<'s>( rust: &'s Drawer<'s>, ) -> Result> { let children = emacs.as_list()?; - let first_child = children - .first() - .ok_or("Should have at least one child.")? - .as_atom()?; - if first_child != "drawer" { - return Err("Drawer should correspond to a drawer cell.".into()); - } + assert_name(emacs, "drawer")?; let mut child_status = Vec::new(); let mut this_status = DiffStatus::Good; @@ -587,6 +534,7 @@ fn compare_drawer<'s>( Ok(DiffResult { status: this_status, name: "drawer".to_owned(), + message: None, children: child_status, }) } diff --git a/src/compare/util.rs b/src/compare/util.rs index 1dd9463..2b665e5 100644 --- a/src/compare/util.rs +++ b/src/compare/util.rs @@ -1,5 +1,7 @@ use crate::parser::Source; +use super::sexp::Token; + /// Check if the child string slice is a slice of the parent string slice. fn is_slice_of(parent: &str, child: &str) -> bool { let parent_start = parent.as_ptr() as usize; @@ -19,3 +21,19 @@ pub fn get_offsets<'s, S: Source<'s>>(source: &'s str, rust_object: &'s S) -> (u let end = offset + rust_object_source.len(); (offset, end) } + +pub fn assert_name<'s>(emacs: &'s Token<'s>, name: &str) -> Result<(), Box> { + let children = emacs.as_list()?; + let first_child = children + .first() + .ok_or("Should have at least one child.")? + .as_atom()?; + if first_child != name { + Err(format!( + "Expected a {expected} cell, but found a {found} cell.", + expected = name, + found = first_child + ))?; + } + Ok(()) +} From a6d8c873d449a22e41274ac59a4ce7e5398a1c39 Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Wed, 19 Apr 2023 15:29:46 -0400 Subject: [PATCH 10/11] Add an assert_bounds utility function. --- Makefile | 6 +- src/compare/diff.rs | 161 +++----------------------------------------- src/compare/util.rs | 27 ++++++++ 3 files changed, 43 insertions(+), 151 deletions(-) diff --git a/Makefile b/Makefile index 724f9cd..b20be8c 100644 --- a/Makefile +++ b/Makefile @@ -24,12 +24,16 @@ clean: .PHONY: test test: -> cargo test --lib +> cargo test --lib --test test_loader .PHONY: integrationtest integrationtest: > cargo test --no-fail-fast --test test_loader +.PHONY: unittest +unittest: +> cargo test --lib + .PHONY: run run: > cargo run diff --git a/src/compare/diff.rs b/src/compare/diff.rs index 99287dc..186898e 100644 --- a/src/compare/diff.rs +++ b/src/compare/diff.rs @@ -1,4 +1,5 @@ use super::sexp::Token; +use super::util::assert_bounds; use super::util::assert_name; use crate::compare::util::get_offsets; use crate::parser::Comment; @@ -125,21 +126,7 @@ fn compare_section<'s>( let mut child_status = Vec::new(); let mut this_status = DiffStatus::Good; - let attributes_child = children - .iter() - .nth(1) - .ok_or("Should have an attributes child.")?; - let attributes_map = attributes_child.as_map()?; - let begin = attributes_map - .get(":begin") - .ok_or("Missing :begin attribute.")? - .as_atom()?; - let end = attributes_map - .get(":end") - .ok_or("Missing :end attribute.")? - .as_atom()?; - let (rust_begin, rust_end) = get_offsets(source, rust); - if (rust_begin + 1).to_string() != begin || (rust_end + 1).to_string() != end { + if assert_bounds(source, emacs, rust).is_err() { this_status = DiffStatus::Bad; } @@ -165,21 +152,7 @@ fn compare_heading<'s>( let mut child_status = Vec::new(); let mut this_status = DiffStatus::Good; - let attributes_child = children - .iter() - .nth(1) - .ok_or("Should have an attributes child.")?; - let attributes_map = attributes_child.as_map()?; - let begin = attributes_map - .get(":begin") - .ok_or("Missing :begin attribute.")? - .as_atom()?; - let end = attributes_map - .get(":end") - .ok_or("Missing :end attribute.")? - .as_atom()?; - let (rust_begin, rust_end) = get_offsets(source, rust); - if (rust_begin + 1).to_string() != begin || (rust_end + 1).to_string() != end { + if assert_bounds(source, emacs, rust).is_err() { this_status = DiffStatus::Bad; } @@ -228,21 +201,7 @@ fn compare_paragraph<'s>( let mut child_status = Vec::new(); let mut this_status = DiffStatus::Good; - let attributes_child = children - .iter() - .nth(1) - .ok_or("Should have an attributes child.")?; - let attributes_map = attributes_child.as_map()?; - let begin = attributes_map - .get(":begin") - .ok_or("Missing :begin attribute.")? - .as_atom()?; - let end = attributes_map - .get(":end") - .ok_or("Missing :end attribute.")? - .as_atom()?; - let (rust_begin, rust_end) = get_offsets(source, rust); - if (rust_begin + 1).to_string() != begin || (rust_end + 1).to_string() != end { + if assert_bounds(source, emacs, rust).is_err() { this_status = DiffStatus::Bad; } @@ -266,21 +225,7 @@ fn compare_plain_list<'s>( let mut child_status = Vec::new(); let mut this_status = DiffStatus::Good; - let attributes_child = children - .iter() - .nth(1) - .ok_or("Should have an attributes child.")?; - let attributes_map = attributes_child.as_map()?; - let begin = attributes_map - .get(":begin") - .ok_or("Missing :begin attribute.")? - .as_atom()?; - let end = attributes_map - .get(":end") - .ok_or("Missing :end attribute.")? - .as_atom()?; - let (rust_begin, rust_end) = get_offsets(source, rust); - if (rust_begin + 1).to_string() != begin || (rust_end + 1).to_string() != end { + if assert_bounds(source, emacs, rust).is_err() { this_status = DiffStatus::Bad; } @@ -306,21 +251,7 @@ fn compare_plain_list_item<'s>( let mut child_status = Vec::new(); let mut this_status = DiffStatus::Good; - let attributes_child = children - .iter() - .nth(1) - .ok_or("Should have an attributes child.")?; - let attributes_map = attributes_child.as_map()?; - let begin = attributes_map - .get(":begin") - .ok_or("Missing :begin attribute.")? - .as_atom()?; - let end = attributes_map - .get(":end") - .ok_or("Missing :end attribute.")? - .as_atom()?; - let (rust_begin, rust_end) = get_offsets(source, rust); - if (rust_begin + 1).to_string() != begin || (rust_end + 1).to_string() != end { + if assert_bounds(source, emacs, rust).is_err() { this_status = DiffStatus::Bad; } @@ -353,21 +284,7 @@ fn compare_greater_block<'s>( let mut child_status = Vec::new(); let mut this_status = DiffStatus::Good; - let attributes_child = children - .iter() - .nth(1) - .ok_or("Should have an attributes child.")?; - let attributes_map = attributes_child.as_map()?; - let begin = attributes_map - .get(":begin") - .ok_or("Missing :begin attribute.")? - .as_atom()?; - let end = attributes_map - .get(":end") - .ok_or("Missing :end attribute.")? - .as_atom()?; - let (rust_begin, rust_end) = get_offsets(source, rust); - if (rust_begin + 1).to_string() != begin || (rust_end + 1).to_string() != end { + if assert_bounds(source, emacs, rust).is_err() { this_status = DiffStatus::Bad; } @@ -393,21 +310,7 @@ fn compare_dynamic_block<'s>( let mut child_status = Vec::new(); let mut this_status = DiffStatus::Good; - let attributes_child = children - .iter() - .nth(1) - .ok_or("Should have an attributes child.")?; - let attributes_map = attributes_child.as_map()?; - let begin = attributes_map - .get(":begin") - .ok_or("Missing :begin attribute.")? - .as_atom()?; - let end = attributes_map - .get(":end") - .ok_or("Missing :end attribute.")? - .as_atom()?; - let (rust_begin, rust_end) = get_offsets(source, rust); - if (rust_begin + 1).to_string() != begin || (rust_end + 1).to_string() != end { + if assert_bounds(source, emacs, rust).is_err() { this_status = DiffStatus::Bad; } @@ -433,21 +336,7 @@ fn compare_footnote_definition<'s>( let mut child_status = Vec::new(); let mut this_status = DiffStatus::Good; - let attributes_child = children - .iter() - .nth(1) - .ok_or("Should have an attributes child.")?; - let attributes_map = attributes_child.as_map()?; - let begin = attributes_map - .get(":begin") - .ok_or("Missing :begin attribute.")? - .as_atom()?; - let end = attributes_map - .get(":end") - .ok_or("Missing :end attribute.")? - .as_atom()?; - let (rust_begin, rust_end) = get_offsets(source, rust); - if (rust_begin + 1).to_string() != begin || (rust_end + 1).to_string() != end { + if assert_bounds(source, emacs, rust).is_err() { this_status = DiffStatus::Bad; } @@ -473,21 +362,7 @@ fn compare_comment<'s>( let mut child_status = Vec::new(); let mut this_status = DiffStatus::Good; - let attributes_child = children - .iter() - .nth(1) - .ok_or("Should have an attributes child.")?; - let attributes_map = attributes_child.as_map()?; - let begin = attributes_map - .get(":begin") - .ok_or("Missing :begin attribute.")? - .as_atom()?; - let end = attributes_map - .get(":end") - .ok_or("Missing :end attribute.")? - .as_atom()?; - let (rust_begin, rust_end) = get_offsets(source, rust); - if (rust_begin + 1).to_string() != begin || (rust_end + 1).to_string() != end { + if assert_bounds(source, emacs, rust).is_err() { this_status = DiffStatus::Bad; } @@ -509,21 +384,7 @@ fn compare_drawer<'s>( let mut child_status = Vec::new(); let mut this_status = DiffStatus::Good; - let attributes_child = children - .iter() - .nth(1) - .ok_or("Should have an attributes child.")?; - let attributes_map = attributes_child.as_map()?; - let begin = attributes_map - .get(":begin") - .ok_or("Missing :begin attribute.")? - .as_atom()?; - let end = attributes_map - .get(":end") - .ok_or("Missing :end attribute.")? - .as_atom()?; - let (rust_begin, rust_end) = get_offsets(source, rust); - if (rust_begin + 1).to_string() != begin || (rust_end + 1).to_string() != end { + if assert_bounds(source, emacs, rust).is_err() { this_status = DiffStatus::Bad; } diff --git a/src/compare/util.rs b/src/compare/util.rs index 2b665e5..830556a 100644 --- a/src/compare/util.rs +++ b/src/compare/util.rs @@ -37,3 +37,30 @@ pub fn assert_name<'s>(emacs: &'s Token<'s>, name: &str) -> Result<(), Box>( + source: &'s str, + emacs: &'s Token<'s>, + rust: &'s S, +) -> Result<(), Box> { + 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 begin = attributes_map + .get(":begin") + .ok_or("Missing :begin attribute.")? + .as_atom()?; + let end = attributes_map + .get(":end") + .ok_or("Missing :end attribute.")? + .as_atom()?; + let (rust_begin, rust_end) = get_offsets(source, rust); + if (rust_begin + 1).to_string() != begin || (rust_end + 1).to_string() != end { + Err(format!("Rust bounds ({rust_begin}, {rust_end}) do not match emacs bounds ({emacs_begin}, {emacs_end})", rust_begin = rust_begin + 1, rust_end = rust_end + 1, emacs_begin=begin, emacs_end=end))?; + } + + Ok(()) +} From 94193d4fa570425a183ecd9a3c87f901c38b9c6f Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Wed, 19 Apr 2023 15:38:36 -0400 Subject: [PATCH 11/11] Make name mismatch report an error rather than abort everything. --- src/compare/diff.rs | 52 ++++++++++++++++++++++++++++++++------------- 1 file changed, 37 insertions(+), 15 deletions(-) diff --git a/src/compare/diff.rs b/src/compare/diff.rs index 186898e..c5a4111 100644 --- a/src/compare/diff.rs +++ b/src/compare/diff.rs @@ -74,9 +74,11 @@ pub fn compare_document<'s>( rust: &'s Document<'s>, ) -> Result> { let children = emacs.as_list()?; - assert_name(emacs, "org-data")?; let mut child_status = Vec::new(); let mut this_status = DiffStatus::Good; + if assert_name(emacs, "org-data").is_err() { + this_status = DiffStatus::Bad; + } // Skipping "org-data" and the first parameter which is often nil for (i, token) in children.iter().skip(2).enumerate() { @@ -122,9 +124,11 @@ fn compare_section<'s>( rust: &'s Section<'s>, ) -> Result> { let children = emacs.as_list()?; - assert_name(emacs, "section")?; - let mut child_status = Vec::new(); let mut this_status = DiffStatus::Good; + let mut child_status = Vec::new(); + if assert_name(emacs, "section").is_err() { + this_status = DiffStatus::Bad; + } if assert_bounds(source, emacs, rust).is_err() { this_status = DiffStatus::Bad; @@ -148,9 +152,11 @@ fn compare_heading<'s>( rust: &'s Heading<'s>, ) -> Result> { let children = emacs.as_list()?; - assert_name(emacs, "headline")?; let mut child_status = Vec::new(); let mut this_status = DiffStatus::Good; + if assert_name(emacs, "headline").is_err() { + this_status = DiffStatus::Bad; + } if assert_bounds(source, emacs, rust).is_err() { this_status = DiffStatus::Bad; @@ -197,9 +203,11 @@ fn compare_paragraph<'s>( rust: &'s Paragraph<'s>, ) -> Result> { let children = emacs.as_list()?; - assert_name(emacs, "paragraph")?; let mut child_status = Vec::new(); let mut this_status = DiffStatus::Good; + if assert_name(emacs, "paragraph").is_err() { + this_status = DiffStatus::Bad; + } if assert_bounds(source, emacs, rust).is_err() { this_status = DiffStatus::Bad; @@ -221,9 +229,11 @@ fn compare_plain_list<'s>( rust: &'s PlainList<'s>, ) -> Result> { let children = emacs.as_list()?; - assert_name(emacs, "plain-list")?; let mut child_status = Vec::new(); let mut this_status = DiffStatus::Good; + if assert_name(emacs, "plain-list").is_err() { + this_status = DiffStatus::Bad; + } if assert_bounds(source, emacs, rust).is_err() { this_status = DiffStatus::Bad; @@ -247,9 +257,11 @@ fn compare_plain_list_item<'s>( rust: &'s PlainListItem<'s>, ) -> Result> { let children = emacs.as_list()?; - assert_name(emacs, "item")?; let mut child_status = Vec::new(); let mut this_status = DiffStatus::Good; + if assert_name(emacs, "item").is_err() { + this_status = DiffStatus::Bad; + } if assert_bounds(source, emacs, rust).is_err() { this_status = DiffStatus::Bad; @@ -273,16 +285,18 @@ fn compare_greater_block<'s>( rust: &'s GreaterBlock<'s>, ) -> Result> { let children = emacs.as_list()?; - assert_name( + let mut child_status = Vec::new(); + let mut this_status = DiffStatus::Good; + if assert_name( emacs, match rust.name.to_lowercase().as_str() { "center" => "center-block", "quote" => "quote-block", _ => todo!(), }, - )?; - let mut child_status = Vec::new(); - let mut this_status = DiffStatus::Good; + ).is_err() { + this_status = DiffStatus::Bad; + } if assert_bounds(source, emacs, rust).is_err() { this_status = DiffStatus::Bad; @@ -306,9 +320,11 @@ fn compare_dynamic_block<'s>( rust: &'s DynamicBlock<'s>, ) -> Result> { let children = emacs.as_list()?; - assert_name(emacs, "dynamic-block")?; let mut child_status = Vec::new(); let mut this_status = DiffStatus::Good; + if assert_name(emacs, "dynamic-block").is_err() { + this_status = DiffStatus::Bad; + } if assert_bounds(source, emacs, rust).is_err() { this_status = DiffStatus::Bad; @@ -332,9 +348,11 @@ fn compare_footnote_definition<'s>( rust: &'s FootnoteDefinition<'s>, ) -> Result> { let children = emacs.as_list()?; - assert_name(emacs, "footnote-definition")?; let mut child_status = Vec::new(); let mut this_status = DiffStatus::Good; + if assert_name(emacs, "footnote-definition").is_err() { + this_status = DiffStatus::Bad; + } if assert_bounds(source, emacs, rust).is_err() { this_status = DiffStatus::Bad; @@ -358,9 +376,11 @@ fn compare_comment<'s>( rust: &'s Comment<'s>, ) -> Result> { let children = emacs.as_list()?; - assert_name(emacs, "comment")?; let mut child_status = Vec::new(); let mut this_status = DiffStatus::Good; + if assert_name(emacs, "comment").is_err() { + this_status = DiffStatus::Bad; + } if assert_bounds(source, emacs, rust).is_err() { this_status = DiffStatus::Bad; @@ -380,9 +400,11 @@ fn compare_drawer<'s>( rust: &'s Drawer<'s>, ) -> Result> { let children = emacs.as_list()?; - assert_name(emacs, "drawer")?; let mut child_status = Vec::new(); let mut this_status = DiffStatus::Good; + if assert_name(emacs, "drawer").is_err() { + this_status = DiffStatus::Bad; + } if assert_bounds(source, emacs, rust).is_err() { this_status = DiffStatus::Bad;