From e39562c85d342591b66c87fde7bbbf2a8c1ade71 Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Mon, 9 Oct 2023 13:08:45 -0400 Subject: [PATCH 1/5] Roll out the new children functions to all compare functions using the new compare_properties macro. We will roll it out to the rest of them when we move them over to the new compare_properties macro. --- src/compare/diff.rs | 103 +++++++++++++++++++++++++++++--------------- 1 file changed, 68 insertions(+), 35 deletions(-) diff --git a/src/compare/diff.rs b/src/compare/diff.rs index f76efd7..c615290 100644 --- a/src/compare/diff.rs +++ b/src/compare/diff.rs @@ -2584,20 +2584,24 @@ fn compare_bold<'b, 's>( emacs: &'b Token<'s>, rust: &'b Bold<'s>, ) -> Result, Box> { - let children = emacs.as_list()?; let mut this_status = DiffStatus::Good; let mut child_status = Vec::new(); let mut message = None; + compare_children( + source, + emacs, + &rust.children, + &mut child_status, + &mut this_status, + &mut message, + )?; + if let Some((new_status, new_message)) = compare_properties!(emacs)? { this_status = new_status; message = new_message; } - for (emacs_child, rust_child) in children.iter().skip(2).zip(rust.children.iter()) { - child_status.push(compare_ast_node(source, emacs_child, rust_child.into())?); - } - Ok(DiffResult { status: this_status, name: rust.get_elisp_name(), @@ -2614,20 +2618,24 @@ fn compare_italic<'b, 's>( emacs: &'b Token<'s>, rust: &'b Italic<'s>, ) -> Result, Box> { - let children = emacs.as_list()?; let mut this_status = DiffStatus::Good; let mut child_status = Vec::new(); let mut message = None; + compare_children( + source, + emacs, + &rust.children, + &mut child_status, + &mut this_status, + &mut message, + )?; + if let Some((new_status, new_message)) = compare_properties!(emacs)? { this_status = new_status; message = new_message; } - for (emacs_child, rust_child) in children.iter().skip(2).zip(rust.children.iter()) { - child_status.push(compare_ast_node(source, emacs_child, rust_child.into())?); - } - Ok(DiffResult { status: this_status, name: rust.get_elisp_name(), @@ -2644,20 +2652,24 @@ fn compare_underline<'b, 's>( emacs: &'b Token<'s>, rust: &'b Underline<'s>, ) -> Result, Box> { - let children = emacs.as_list()?; let mut this_status = DiffStatus::Good; let mut child_status = Vec::new(); let mut message = None; + compare_children( + source, + emacs, + &rust.children, + &mut child_status, + &mut this_status, + &mut message, + )?; + if let Some((new_status, new_message)) = compare_properties!(emacs)? { this_status = new_status; message = new_message; } - for (emacs_child, rust_child) in children.iter().skip(2).zip(rust.children.iter()) { - child_status.push(compare_ast_node(source, emacs_child, rust_child.into())?); - } - Ok(DiffResult { status: this_status, name: rust.get_elisp_name(), @@ -2677,6 +2689,8 @@ fn compare_verbatim<'b, 's>( let mut this_status = DiffStatus::Good; let mut message = None; + assert_no_children(emacs, &mut this_status, &mut message)?; + if let Some((new_status, new_message)) = compare_properties!( emacs, rust, @@ -2709,6 +2723,8 @@ fn compare_code<'b, 's>( let mut this_status = DiffStatus::Good; let mut message = None; + assert_no_children(emacs, &mut this_status, &mut message)?; + if let Some((new_status, new_message)) = compare_properties!( emacs, rust, @@ -2738,20 +2754,24 @@ fn compare_strike_through<'b, 's>( emacs: &'b Token<'s>, rust: &'b StrikeThrough<'s>, ) -> Result, Box> { - let children = emacs.as_list()?; let mut this_status = DiffStatus::Good; let mut child_status = Vec::new(); let mut message = None; + compare_children( + source, + emacs, + &rust.children, + &mut child_status, + &mut this_status, + &mut message, + )?; + if let Some((new_status, new_message)) = compare_properties!(emacs)? { this_status = new_status; message = new_message; } - for (emacs_child, rust_child) in children.iter().skip(2).zip(rust.children.iter()) { - child_status.push(compare_ast_node(source, emacs_child, rust_child.into())?); - } - Ok(DiffResult { status: this_status, name: rust.get_elisp_name(), @@ -2768,11 +2788,19 @@ fn compare_regular_link<'b, 's>( emacs: &'b Token<'s>, rust: &'b RegularLink<'s>, ) -> Result, Box> { - let children = emacs.as_list()?; let mut this_status = DiffStatus::Good; let mut child_status = Vec::new(); let mut message = None; + compare_children( + source, + emacs, + &rust.children, + &mut child_status, + &mut this_status, + &mut message, + )?; + if let Some((new_status, new_message)) = compare_properties!( emacs, rust, @@ -2820,10 +2848,6 @@ fn compare_regular_link<'b, 's>( message = new_message; } - for (emacs_child, rust_child) in children.iter().skip(2).zip(rust.children.iter()) { - child_status.push(compare_ast_node(source, emacs_child, rust_child.into())?); - } - Ok(DiffResult { status: this_status, name: rust.get_elisp_name(), @@ -2840,11 +2864,19 @@ fn compare_radio_link<'b, 's>( emacs: &'b Token<'s>, rust: &'b RadioLink<'s>, ) -> Result, Box> { - let children = emacs.as_list()?; let mut this_status = DiffStatus::Good; let mut child_status = Vec::new(); let mut message = None; + compare_children( + source, + emacs, + &rust.children, + &mut child_status, + &mut this_status, + &mut message, + )?; + if let Some((new_status, new_message)) = compare_properties!( emacs, rust, @@ -2883,10 +2915,6 @@ fn compare_radio_link<'b, 's>( message = new_message; } - for (emacs_child, rust_child) in children.iter().skip(2).zip(rust.children.iter()) { - child_status.push(compare_ast_node(source, emacs_child, rust_child.into())?); - } - Ok(DiffResult { status: this_status, name: rust.get_elisp_name(), @@ -2903,7 +2931,6 @@ fn compare_radio_target<'b, 's>( emacs: &'b Token<'s>, rust: &'b RadioTarget<'s>, ) -> Result, Box> { - let children = emacs.as_list()?; let mut this_status = DiffStatus::Good; let mut child_status = Vec::new(); let mut message = None; @@ -2930,10 +2957,6 @@ fn compare_radio_target<'b, 's>( message = new_message; } - for (emacs_child, rust_child) in children.iter().skip(2).zip(rust.children.iter()) { - child_status.push(compare_ast_node(source, emacs_child, rust_child.into())?); - } - Ok(DiffResult { status: this_status, name: rust.get_elisp_name(), @@ -2953,6 +2976,8 @@ fn compare_plain_link<'b, 's>( let mut this_status = DiffStatus::Good; let mut message = None; + assert_no_children(emacs, &mut this_status, &mut message)?; + if let Some((new_status, new_message)) = compare_properties!( emacs, rust, @@ -3019,6 +3044,8 @@ fn compare_angle_link<'b, 's>( let mut this_status = DiffStatus::Good; let mut message = None; + assert_no_children(emacs, &mut this_status, &mut message)?; + if let Some((new_status, new_message)) = compare_properties!( emacs, rust, @@ -3085,6 +3112,8 @@ fn compare_org_macro<'b, 's>( let mut this_status = DiffStatus::Good; let mut message = None; + assert_no_children(emacs, &mut this_status, &mut message)?; + if let Some((new_status, new_message)) = compare_properties!( emacs, rust, @@ -3131,6 +3160,8 @@ fn compare_entity<'b, 's>( let mut this_status = DiffStatus::Good; let mut message = None; + assert_no_children(emacs, &mut this_status, &mut message)?; + if let Some((new_status, new_message)) = compare_properties!( emacs, rust, @@ -3199,6 +3230,8 @@ fn compare_latex_fragment<'b, 's>( let mut this_status = DiffStatus::Good; let mut message = None; + assert_no_children(emacs, &mut this_status, &mut message)?; + if let Some((new_status, new_message)) = compare_properties!( emacs, rust, From 9565435526e2e1bbd6f076133ec22b3d607b7884 Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Mon, 9 Oct 2023 13:14:35 -0400 Subject: [PATCH 2/5] Compare footnote reference properties. --- src/compare/diff.rs | 36 +++++++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/src/compare/diff.rs b/src/compare/diff.rs index c615290..74ac74f 100644 --- a/src/compare/diff.rs +++ b/src/compare/diff.rs @@ -3296,20 +3296,46 @@ fn compare_export_snippet<'b, 's>( } fn compare_footnote_reference<'b, 's>( - _source: &'s str, + source: &'s str, emacs: &'b Token<'s>, rust: &'b FootnoteReference<'s>, ) -> Result, Box> { - let this_status = DiffStatus::Good; - let message = None; + let mut child_status = Vec::new(); + let mut this_status = DiffStatus::Good; + let mut message = None; - // TODO: Compare :label :type + compare_children( + source, + emacs, + &rust.definition, + &mut child_status, + &mut this_status, + &mut message, + )?; + + if let Some((new_status, new_message)) = compare_properties!( + emacs, + rust, + ( + EmacsField::Required(":label"), + |r| r.label, + compare_property_quoted_string + ), + ( + EmacsField::Required(":type"), + |_| Some("inline"), + compare_property_unquoted_atom + ) + )? { + this_status = new_status; + message = new_message; + } Ok(DiffResult { status: this_status, name: rust.get_elisp_name(), message, - children: Vec::new(), + children: child_status, rust_source: rust.get_source(), emacs_token: emacs, } From 5ac12229f4e1f927375ccffb92a79c50469681ab Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Mon, 9 Oct 2023 13:23:08 -0400 Subject: [PATCH 3/5] Fix footnote reference type. --- src/compare/diff.rs | 6 +++++- src/types/mod.rs | 1 + src/types/object.rs | 16 ++++++++++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/compare/diff.rs b/src/compare/diff.rs index 74ac74f..9a0fe22 100644 --- a/src/compare/diff.rs +++ b/src/compare/diff.rs @@ -54,6 +54,7 @@ use crate::types::ExportSnippet; use crate::types::FixedWidthArea; use crate::types::FootnoteDefinition; use crate::types::FootnoteReference; +use crate::types::FootnoteReferenceType; use crate::types::GetStandardProperties; use crate::types::Heading; use crate::types::HorizontalRule; @@ -3323,7 +3324,10 @@ fn compare_footnote_reference<'b, 's>( ), ( EmacsField::Required(":type"), - |_| Some("inline"), + |r| Some(match r.get_type() { + FootnoteReferenceType::Standard => "standard", + FootnoteReferenceType::Inline => "inline", + }), compare_property_unquoted_atom ) )? { diff --git a/src/types/mod.rs b/src/types/mod.rs index 8148c0d..fc93c5a 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -69,6 +69,7 @@ pub use object::DayOfMonthInner; pub use object::Entity; pub use object::ExportSnippet; pub use object::FootnoteReference; +pub use object::FootnoteReferenceType; pub use object::Hour; pub use object::HourInner; pub use object::InlineBabelCall; diff --git a/src/types/object.rs b/src/types/object.rs index 62cf507..f8b88d9 100644 --- a/src/types/object.rs +++ b/src/types/object.rs @@ -764,3 +764,19 @@ impl<'s> OrgMacro<'s> { .map(|arg| coalesce_whitespace_escaped('\\', |c| ",".contains(c))(*arg)) } } + +#[derive(Debug, PartialEq)] +pub enum FootnoteReferenceType { + Standard, + Inline, +} + +impl<'s> FootnoteReference<'s> { + pub fn get_type(&self) -> FootnoteReferenceType { + if self.definition.is_empty() { + FootnoteReferenceType::Standard + } else { + FootnoteReferenceType::Inline + } + } +} From adc5a383c3a3166cf501742ae489a0218716fe94 Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Mon, 9 Oct 2023 13:44:14 -0400 Subject: [PATCH 4/5] Allow text markup at the start of a radio target. --- src/context/context.rs | 5 +++++ src/parser/radio_link.rs | 14 +++++++++----- src/parser/text_markup.rs | 12 +++++++++++- 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/context/context.rs b/src/context/context.rs index def26bf..138338f 100644 --- a/src/context/context.rs +++ b/src/context/context.rs @@ -33,6 +33,11 @@ pub(crate) enum ContextElement<'r, 's> { /// The value stored is the start of the element after the affiliated keywords. In this way, we can ensure that we do not exit an element immediately after the affiliated keyword had been consumed. HasAffiliatedKeyword(HasAffiliatedKeywordInner<'r, 's>), + /// Indicate the position that we started parsing a radio target. + /// + /// This value is stored because "<<<" is not a valid prefix for text markup UNLESS it is starting a radio target. + StartRadioTarget(OrgSource<'s>), + /// This is just here to use the 's lifetime until I'm sure we can eliminate it from ContextElement. #[allow(dead_code)] Placeholder(PhantomData<&'s str>), diff --git a/src/parser/radio_link.rs b/src/parser/radio_link.rs index 5955344..15eb6cd 100644 --- a/src/parser/radio_link.rs +++ b/src/parser/radio_link.rs @@ -103,11 +103,15 @@ pub(crate) fn radio_target<'b, 'g, 'r, 's>( input: OrgSource<'s>, ) -> Res, RadioTarget<'s>> { let (remaining, _opening) = tag("<<<")(input)?; - let parser_context = ContextElement::ExitMatcherNode(ExitMatcherNode { - class: ExitClass::Gamma, - exit_matcher: &radio_target_end, - }); - let parser_context = context.with_additional_node(&parser_context); + let contexts = [ + ContextElement::ExitMatcherNode(ExitMatcherNode { + class: ExitClass::Gamma, + exit_matcher: &radio_target_end, + }), + ContextElement::StartRadioTarget(remaining), + ]; + let parser_context = context.with_additional_node(&contexts[0]); + let parser_context = parser_context.with_additional_node(&contexts[1]); let (remaining, (raw_value, children)) = consumed(verify( map( diff --git a/src/parser/text_markup.rs b/src/parser/text_markup.rs index a32c4c1..d666633 100644 --- a/src/parser/text_markup.rs +++ b/src/parser/text_markup.rs @@ -283,7 +283,7 @@ fn _text_markup_string<'b, 'g, 'r, 's, 'c>( #[cfg_attr(feature = "tracing", tracing::instrument(ret, level = "debug"))] fn pre<'b, 'g, 'r, 's>( - _context: RefContext<'b, 'g, 'r, 's>, + context: RefContext<'b, 'g, 'r, 's>, input: OrgSource<'s>, ) -> Res, ()> { if start_of_line(input).is_ok() { @@ -292,6 +292,16 @@ fn pre<'b, 'g, 'r, 's>( if preceded_by_whitespace(true)(input).is_ok() { return Ok((input, ())); } + let radio_target_start = context + .iter() + .find_map(|c| match c { + ContextElement::StartRadioTarget(text) => Some(text), + _ => None, + }) + .map(|text| text.get_byte_offset()); + if Some(input.get_byte_offset()) == radio_target_start { + return Ok((input, ())); + } let preceding_character = input.get_preceding_character(); match preceding_character { // If None, we are at the start of the file which is technically the beginning of a line. From 840dc0a7508fcac259f1f7453fde40b077f58b4d Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Mon, 9 Oct 2023 14:02:27 -0400 Subject: [PATCH 5/5] Support text markup at the start of a regular link description. --- src/context/context.rs | 6 +++--- src/parser/radio_link.rs | 2 +- src/parser/regular_link.rs | 14 +++++++++----- src/parser/text_markup.rs | 2 +- 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/context/context.rs b/src/context/context.rs index 138338f..6787273 100644 --- a/src/context/context.rs +++ b/src/context/context.rs @@ -33,10 +33,10 @@ pub(crate) enum ContextElement<'r, 's> { /// The value stored is the start of the element after the affiliated keywords. In this way, we can ensure that we do not exit an element immediately after the affiliated keyword had been consumed. HasAffiliatedKeyword(HasAffiliatedKeywordInner<'r, 's>), - /// Indicate the position that we started parsing a radio target. + /// Indicate the position that we started parsing a text section. /// - /// This value is stored because "<<<" is not a valid prefix for text markup UNLESS it is starting a radio target. - StartRadioTarget(OrgSource<'s>), + /// This value is stored because "<<<" is not a valid prefix for text markup UNLESS it is starting a radio target. Likewise "[" is not a valid prefix for text markup UNLESS it is the start of a regular link description. + StartTextSection(OrgSource<'s>), /// This is just here to use the 's lifetime until I'm sure we can eliminate it from ContextElement. #[allow(dead_code)] diff --git a/src/parser/radio_link.rs b/src/parser/radio_link.rs index 15eb6cd..c25ec3c 100644 --- a/src/parser/radio_link.rs +++ b/src/parser/radio_link.rs @@ -108,7 +108,7 @@ pub(crate) fn radio_target<'b, 'g, 'r, 's>( class: ExitClass::Gamma, exit_matcher: &radio_target_end, }), - ContextElement::StartRadioTarget(remaining), + ContextElement::StartTextSection(remaining), ]; let parser_context = context.with_additional_node(&contexts[0]); let parser_context = parser_context.with_additional_node(&contexts[1]); diff --git a/src/parser/regular_link.rs b/src/parser/regular_link.rs index 199829c..ce9ce37 100644 --- a/src/parser/regular_link.rs +++ b/src/parser/regular_link.rs @@ -397,11 +397,15 @@ fn description<'b, 'g, 'r, 's>( context: RefContext<'b, 'g, 'r, 's>, input: OrgSource<'s>, ) -> Res, Vec>> { - let parser_context = ContextElement::ExitMatcherNode(ExitMatcherNode { - class: ExitClass::Beta, - exit_matcher: &description_end, - }); - let parser_context = context.with_additional_node(&parser_context); + let contexts = [ + ContextElement::ExitMatcherNode(ExitMatcherNode { + class: ExitClass::Beta, + exit_matcher: &description_end, + }), + ContextElement::StartTextSection(input), + ]; + let parser_context = context.with_additional_node(&contexts[0]); + let parser_context = parser_context.with_additional_node(&contexts[1]); let (remaining, (children, _exit_contents)) = verify( many_till( parser_with_context!(regular_link_description_set_object)(&parser_context), diff --git a/src/parser/text_markup.rs b/src/parser/text_markup.rs index d666633..7bf7bf1 100644 --- a/src/parser/text_markup.rs +++ b/src/parser/text_markup.rs @@ -295,7 +295,7 @@ fn pre<'b, 'g, 'r, 's>( let radio_target_start = context .iter() .find_map(|c| match c { - ContextElement::StartRadioTarget(text) => Some(text), + ContextElement::StartTextSection(text) => Some(text), _ => None, }) .map(|text| text.get_byte_offset());