From ba72cc1b29ccbe4111614802f5fabfeb7adf5b1f Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Wed, 18 Oct 2023 18:22:01 -0400 Subject: [PATCH 1/4] The variables for keywords are actually constants. These settings do not need to exist in GlobalSettings because they are actually constants in upstream Org-Mode. --- src/context/constants.rs | 20 ++++++++++++++++---- src/context/global_settings.rs | 28 ---------------------------- src/context/mod.rs | 2 +- src/parser/affiliated_keyword.rs | 11 ++++++----- src/parser/keyword.rs | 10 ++++++---- 5 files changed, 29 insertions(+), 42 deletions(-) diff --git a/src/context/constants.rs b/src/context/constants.rs index 34bb98c7..0ef8d1e9 100644 --- a/src/context/constants.rs +++ b/src/context/constants.rs @@ -1,15 +1,27 @@ use super::global_settings::EntityDefinition; -pub(crate) const DEFAULT_ORG_ELEMENT_PARSED_KEYWORDS: [&str; 1] = ["CAPTION"]; +/// Keywords that contain the standard set of objects (excluding footnote references). +/// +/// Corresponds to org-element-parsed-keywords elisp variable. +pub(crate) const ORG_ELEMENT_PARSED_KEYWORDS: [&str; 1] = ["CAPTION"]; -pub(crate) const DEFAULT_ORG_ELEMENT_DUAL_KEYWORDS: [&str; 2] = ["CAPTION", "RESULTS"]; +/// Keywords that can have a secondary value in square brackets. +/// +/// Corresponds to org-element-dual-keywords elisp variable. +pub(crate) const ORG_ELEMENT_DUAL_KEYWORDS: [&str; 2] = ["CAPTION", "RESULTS"]; -pub(crate) const DEFAULT_ORG_ELEMENT_AFFILIATED_KEYWORDS: [&str; 13] = [ +/// Keywords that can be affiliated with an element. +/// +/// Corresponds to org-element-affiliated-keywords elisp variable. +pub(crate) const ORG_ELEMENT_AFFILIATED_KEYWORDS: [&str; 13] = [ "CAPTION", "DATA", "HEADER", "HEADERS", "LABEL", "NAME", "PLOT", "RESNAME", "RESULT", "RESULTS", "SOURCE", "SRCNAME", "TBLNAME", ]; -pub(crate) const DEFAULT_ORG_ELEMENT_KEYWORD_TRANSLATION_ALIST: [(&str, &str); 8] = [ +/// Mapping of keyword names. +/// +/// Corresponds to org-element-keyword-translation-alist elisp variable. +pub(crate) const ORG_ELEMENT_KEYWORD_TRANSLATION_ALIST: [(&str, &str); 8] = [ ("DATA", "NAME"), ("LABEL", "NAME"), ("RESNAME", "NAME"), diff --git a/src/context/global_settings.rs b/src/context/global_settings.rs index fa177999..bad04bfb 100644 --- a/src/context/global_settings.rs +++ b/src/context/global_settings.rs @@ -5,10 +5,6 @@ use super::constants::DEFAULT_ORG_ENTITIES; use super::constants::DEFAULT_ORG_LINK_PARAMETERS; use super::FileAccessInterface; use super::LocalFileAccessInterface; -use crate::context::constants::DEFAULT_ORG_ELEMENT_AFFILIATED_KEYWORDS; -use crate::context::constants::DEFAULT_ORG_ELEMENT_DUAL_KEYWORDS; -use crate::context::constants::DEFAULT_ORG_ELEMENT_KEYWORD_TRANSLATION_ALIST; -use crate::context::constants::DEFAULT_ORG_ELEMENT_PARSED_KEYWORDS; use crate::types::IndentationLevel; use crate::types::Object; @@ -58,26 +54,6 @@ pub struct GlobalSettings<'g, 's> { /// /// Corresponds to org-entities elisp variable. pub entities: &'g [EntityDefinition<'s>], - - /// Keywords that contain the standard set of objects (excluding footnote references). - /// - /// Corresponds to org-element-parsed-keywords elisp variable. - pub element_parsed_keywords: &'g [&'s str], - - /// Keywords that can have a secondary value in square brackets. - /// - /// Corresponds to org-element-dual-keywords elisp variable. - pub element_dual_keywords: &'g [&'s str], - - /// Keywords that can be affiliated with an element. - /// - /// Corresponds to org-element-affiliated-keywords elisp variable. - pub element_affiliated_keywords: &'g [&'s str], - - /// Mapping of keyword names. - /// - /// Corresponds to org-element-keyword-translation-alist elisp variable. - pub element_keyword_translation_alist: &'g [(&'s str, &'s str)], } pub const DEFAULT_TAB_WIDTH: IndentationLevel = 8; @@ -112,10 +88,6 @@ impl<'g, 's> GlobalSettings<'g, 's> { link_parameters: &DEFAULT_ORG_LINK_PARAMETERS, link_templates: BTreeMap::new(), entities: &DEFAULT_ORG_ENTITIES, - element_parsed_keywords: &DEFAULT_ORG_ELEMENT_PARSED_KEYWORDS, - element_dual_keywords: &DEFAULT_ORG_ELEMENT_DUAL_KEYWORDS, - element_affiliated_keywords: &DEFAULT_ORG_ELEMENT_AFFILIATED_KEYWORDS, - element_keyword_translation_alist: &DEFAULT_ORG_ELEMENT_KEYWORD_TRANSLATION_ALIST, } } } diff --git a/src/context/mod.rs b/src/context/mod.rs index a82b5a3a..878c5366 100644 --- a/src/context/mod.rs +++ b/src/context/mod.rs @@ -1,7 +1,7 @@ use crate::error::Res; use crate::parser::OrgSource; -mod constants; +pub(crate) mod constants; #[allow(clippy::module_inception)] mod context; mod exiting; diff --git a/src/parser/affiliated_keyword.rs b/src/parser/affiliated_keyword.rs index 62fbaf05..6099588a 100644 --- a/src/parser/affiliated_keyword.rs +++ b/src/parser/affiliated_keyword.rs @@ -19,6 +19,9 @@ use super::object_parser::standard_set_object; use super::util::confine_context; use super::OrgSource; use crate::context::bind_context; +use crate::context::constants::ORG_ELEMENT_DUAL_KEYWORDS; +use crate::context::constants::ORG_ELEMENT_KEYWORD_TRANSLATION_ALIST; +use crate::context::constants::ORG_ELEMENT_PARSED_KEYWORDS; use crate::context::Context; use crate::context::ContextElement; use crate::context::GlobalSettings; @@ -156,7 +159,7 @@ fn translate_name<'g, 's>(global_settings: &'g GlobalSettings<'g, 's>, name: &'s .split_once('[') .map(|(before, _after)| before) .unwrap_or(name); - for (src, dst) in global_settings.element_keyword_translation_alist { + for (src, dst) in ORG_ELEMENT_KEYWORD_TRANSLATION_ALIST { if name_until_optval.eq_ignore_ascii_case(src) { return dst.to_lowercase(); } @@ -179,12 +182,10 @@ fn identify_keyword_type<'g, 's>( .into_iter() .any(|candidate| name.eq_ignore_ascii_case(candidate)) || name.to_lowercase().starts_with("attr_"); - let is_parsed = global_settings - .element_parsed_keywords + let is_parsed = ORG_ELEMENT_PARSED_KEYWORDS .iter() .any(|candidate| name.eq_ignore_ascii_case(candidate)); - let can_have_optval = global_settings - .element_dual_keywords + let can_have_optval = ORG_ELEMENT_DUAL_KEYWORDS .iter() .any(|candidate| name.eq_ignore_ascii_case(candidate)); match (is_multiple, is_parsed, can_have_optval) { diff --git a/src/parser/keyword.rs b/src/parser/keyword.rs index 2cf29f0f..63c608db 100644 --- a/src/parser/keyword.rs +++ b/src/parser/keyword.rs @@ -22,6 +22,8 @@ use super::util::get_consumed; use super::util::maybe_consume_trailing_whitespace_if_not_exiting; use super::util::org_line_ending; use crate::context::bind_context; +use crate::context::constants::ORG_ELEMENT_AFFILIATED_KEYWORDS; +use crate::context::constants::ORG_ELEMENT_DUAL_KEYWORDS; use crate::context::RefContext; use crate::error::CustomError; use crate::error::Res; @@ -152,9 +154,9 @@ fn plain_affiliated_key<'b, 'g, 'r, 's>( context: RefContext<'b, 'g, 'r, 's>, input: OrgSource<'s>, ) -> Res, OrgSource<'s>> { - for keyword in context.get_global_settings().element_affiliated_keywords { + for keyword in ORG_ELEMENT_AFFILIATED_KEYWORDS { let result = map( - tuple((tag_no_case::<_, _, CustomError>(*keyword), peek(tag(":")))), + tuple((tag_no_case::<_, _, CustomError>(keyword), peek(tag(":")))), |(key, _)| key, )(input); if let Ok((remaining, ent)) = result { @@ -170,9 +172,9 @@ fn dual_affiliated_key<'b, 'g, 'r, 's>( context: RefContext<'b, 'g, 'r, 's>, input: OrgSource<'s>, ) -> Res, OrgSource<'s>> { - for keyword in context.get_global_settings().element_dual_keywords { + for keyword in ORG_ELEMENT_DUAL_KEYWORDS { let result = recognize(tuple(( - tag_no_case::<_, _, CustomError>(*keyword), + tag_no_case::<_, _, CustomError>(keyword), tag("["), optval, tag("]"), From 49d1cef7aef9427eb96b6366c2c623bd29eae480 Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Wed, 18 Oct 2023 18:28:24 -0400 Subject: [PATCH 2/4] Remove context from functions that no longer need it. --- src/parser/affiliated_keyword.rs | 17 ++++++----------- src/parser/element_parser.rs | 6 ++---- src/parser/keyword.rs | 27 +++++++-------------------- 3 files changed, 15 insertions(+), 35 deletions(-) diff --git a/src/parser/affiliated_keyword.rs b/src/parser/affiliated_keyword.rs index 6099588a..a684ce41 100644 --- a/src/parser/affiliated_keyword.rs +++ b/src/parser/affiliated_keyword.rs @@ -26,7 +26,6 @@ use crate::context::Context; use crate::context::ContextElement; use crate::context::GlobalSettings; use crate::context::List; -use crate::context::RefContext; use crate::error::Res; use crate::types::AffiliatedKeywordValue; use crate::types::AffiliatedKeywords; @@ -36,15 +35,14 @@ use crate::types::Keyword; feature = "tracing", tracing::instrument(ret, level = "debug", skip(context)) )] -pub(crate) fn affiliated_keywords<'b, 'g, 'r, 's>( - context: RefContext<'b, 'g, 'r, 's>, +pub(crate) fn affiliated_keywords<'s>( input: OrgSource<'s>, ) -> Res, Vec>> { let mut ret = Vec::new(); let mut remaining = input; loop { - let result = affiliated_keyword(context, remaining); + let result = affiliated_keyword(remaining); match result { Ok((remain, kw)) => { remaining = remain; @@ -68,8 +66,8 @@ where { let mut ret = BTreeMap::new(); for kw in input { - let translated_name = translate_name(global_settings, kw.key); - let keyword_type = identify_keyword_type(global_settings, translated_name.as_str()); + let translated_name = translate_name(kw.key); + let keyword_type = identify_keyword_type(translated_name.as_str()); match keyword_type { AffiliatedKeywordType::SingleString => { ret.insert( @@ -154,7 +152,7 @@ where AffiliatedKeywords { keywords: ret } } -fn translate_name<'g, 's>(global_settings: &'g GlobalSettings<'g, 's>, name: &'s str) -> String { +fn translate_name<'g, 's>(name: &'s str) -> String { let name_until_optval = name .split_once('[') .map(|(before, _after)| before) @@ -174,10 +172,7 @@ enum AffiliatedKeywordType { ObjectTree, } -fn identify_keyword_type<'g, 's>( - global_settings: &'g GlobalSettings<'g, 's>, - name: &'s str, -) -> AffiliatedKeywordType { +fn identify_keyword_type<'g, 's>(name: &'s str) -> AffiliatedKeywordType { let is_multiple = ["CAPTION", "HEADER"] .into_iter() .any(|candidate| name.eq_ignore_ascii_case(candidate)) diff --git a/src/parser/element_parser.rs b/src/parser/element_parser.rs index 774abd81..6d5fca85 100644 --- a/src/parser/element_parser.rs +++ b/src/parser/element_parser.rs @@ -59,8 +59,7 @@ fn _element<'b, 'g, 'r, 's>( ) -> Res, Element<'s>> { #[cfg(feature = "event_count")] record_event(EventType::ElementStart, input); - let (post_affiliated_keywords_input, affiliated_keywords) = - affiliated_keywords(context, input)?; + let (post_affiliated_keywords_input, affiliated_keywords) = affiliated_keywords(input)?; let mut affiliated_keywords = affiliated_keywords.into_iter(); @@ -277,8 +276,7 @@ fn _detect_element<'b, 'g, 'r, 's>( input: OrgSource<'s>, can_be_paragraph: bool, ) -> Res, ()> { - let (post_affiliated_keywords_input, affiliated_keywords) = - affiliated_keywords(context, input)?; + let (post_affiliated_keywords_input, affiliated_keywords) = affiliated_keywords(input)?; let mut affiliated_keywords = affiliated_keywords.into_iter(); diff --git a/src/parser/keyword.rs b/src/parser/keyword.rs index 63c608db..f6cca6c4 100644 --- a/src/parser/keyword.rs +++ b/src/parser/keyword.rs @@ -21,7 +21,6 @@ use super::org_source::OrgSource; use super::util::get_consumed; use super::util::maybe_consume_trailing_whitespace_if_not_exiting; use super::util::org_line_ending; -use crate::context::bind_context; use crate::context::constants::ORG_ELEMENT_AFFILIATED_KEYWORDS; use crate::context::constants::ORG_ELEMENT_DUAL_KEYWORDS; use crate::context::RefContext; @@ -100,11 +99,8 @@ where } #[cfg_attr(feature = "tracing", tracing::instrument(ret, level = "debug"))] -pub(crate) fn affiliated_keyword<'b, 'g, 'r, 's>( - context: RefContext<'b, 'g, 'r, 's>, - input: OrgSource<'s>, -) -> Res, Keyword<'s>> { - filtered_keyword(bind_context!(affiliated_key, context))(input) +pub(crate) fn affiliated_keyword<'s>(input: OrgSource<'s>) -> Res, Keyword<'s>> { + filtered_keyword(affiliated_key)(input) } #[cfg_attr( @@ -139,21 +135,15 @@ fn regular_keyword_key<'s>(input: OrgSource<'s>) -> Res, OrgSource } #[cfg_attr(feature = "tracing", tracing::instrument(ret, level = "debug"))] -fn affiliated_key<'b, 'g, 'r, 's>( - context: RefContext<'b, 'g, 'r, 's>, - input: OrgSource<'s>, -) -> Res, OrgSource<'s>> { - element!(dual_affiliated_key, context, input); - element!(plain_affiliated_key, context, input); +fn affiliated_key<'s>(input: OrgSource<'s>) -> Res, OrgSource<'s>> { + element!(dual_affiliated_key, input); + element!(plain_affiliated_key, input); element!(export_keyword, input); Err(nom::Err::Error(CustomError::Static("No affiliated key."))) } #[cfg_attr(feature = "tracing", tracing::instrument(ret, level = "debug"))] -fn plain_affiliated_key<'b, 'g, 'r, 's>( - context: RefContext<'b, 'g, 'r, 's>, - input: OrgSource<'s>, -) -> Res, OrgSource<'s>> { +fn plain_affiliated_key<'s>(input: OrgSource<'s>) -> Res, OrgSource<'s>> { for keyword in ORG_ELEMENT_AFFILIATED_KEYWORDS { let result = map( tuple((tag_no_case::<_, _, CustomError>(keyword), peek(tag(":")))), @@ -168,10 +158,7 @@ fn plain_affiliated_key<'b, 'g, 'r, 's>( } #[cfg_attr(feature = "tracing", tracing::instrument(ret, level = "debug"))] -fn dual_affiliated_key<'b, 'g, 'r, 's>( - context: RefContext<'b, 'g, 'r, 's>, - input: OrgSource<'s>, -) -> Res, OrgSource<'s>> { +fn dual_affiliated_key<'s>(input: OrgSource<'s>) -> Res, OrgSource<'s>> { for keyword in ORG_ELEMENT_DUAL_KEYWORDS { let result = recognize(tuple(( tag_no_case::<_, _, CustomError>(keyword), From b2479e9de893516dab79d5c1e9817fb71b4bc49b Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Wed, 18 Oct 2023 18:36:25 -0400 Subject: [PATCH 3/4] Remove Debug from the context variables. Now that entities are stored in the settings struct, these variables are massive which makes them balloon trace sizes while being mostly unreadable. This removes Debug from them to serve as a static-analysis check that context is ALWAYS ignored in tracing calls. --- src/context/context.rs | 10 ---------- src/context/exiting.rs | 8 +------- src/context/file_access_interface.rs | 7 +++---- src/context/global_settings.rs | 4 ++-- src/parser/affiliated_keyword.rs | 5 +---- src/parser/in_buffer_settings.rs | 5 ++++- 6 files changed, 11 insertions(+), 28 deletions(-) diff --git a/src/context/context.rs b/src/context/context.rs index 3a066473..2ab029a2 100644 --- a/src/context/context.rs +++ b/src/context/context.rs @@ -12,7 +12,6 @@ use crate::error::CustomError; use crate::error::Res; use crate::parser::OrgSource; -#[derive(Debug)] pub(crate) enum ContextElement<'r, 's> { /// Stores a parser that indicates that children should exit upon matching an exit matcher. ExitMatcherNode(ExitMatcherNode<'r>), @@ -34,15 +33,6 @@ pub(crate) struct ExitMatcherNode<'r> { pub(crate) class: ExitClass, } -impl<'r> std::fmt::Debug for ExitMatcherNode<'r> { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let mut formatter = f.debug_struct("ExitMatcherNode"); - formatter.field("class", &self.class.to_string()); - formatter.finish() - } -} - -#[derive(Debug)] pub(crate) struct Context<'g, 'r, 's> { global_settings: &'g GlobalSettings<'g, 's>, tree: List<'r, &'r ContextElement<'r, 's>>, diff --git a/src/context/exiting.rs b/src/context/exiting.rs index d8ed86f0..b1f40a88 100644 --- a/src/context/exiting.rs +++ b/src/context/exiting.rs @@ -1,13 +1,7 @@ -#[derive(Debug, Copy, Clone)] +#[derive(Copy, Clone)] pub(crate) enum ExitClass { Document = 1, Alpha = 2, Beta = 3, Gamma = 4, } - -impl std::fmt::Display for ExitClass { - fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - write!(f, "{:?}", self) - } -} diff --git a/src/context/file_access_interface.rs b/src/context/file_access_interface.rs index 88ee09b1..78d5bf52 100644 --- a/src/context/file_access_interface.rs +++ b/src/context/file_access_interface.rs @@ -1,17 +1,16 @@ -use std::fmt::Debug; use std::path::PathBuf; #[cfg(any(feature = "compare", feature = "foreign_document_test"))] -pub trait FileAccessInterface: Sync + Debug { +pub trait FileAccessInterface: Sync { fn read_file(&self, path: &str) -> Result; } #[cfg(not(any(feature = "compare", feature = "foreign_document_test")))] -pub trait FileAccessInterface: Debug { +pub trait FileAccessInterface { fn read_file(&self, path: &str) -> Result; } -#[derive(Debug, Clone)] +#[derive(Clone)] pub struct LocalFileAccessInterface { pub working_directory: Option, } diff --git a/src/context/global_settings.rs b/src/context/global_settings.rs index bad04bfb..8eee75ca 100644 --- a/src/context/global_settings.rs +++ b/src/context/global_settings.rs @@ -10,7 +10,7 @@ use crate::types::Object; // TODO: Ultimately, I think we'll need most of this: https://orgmode.org/manual/In_002dbuffer-Settings.html -#[derive(Debug, Clone)] +#[derive(Clone)] pub struct GlobalSettings<'g, 's> { pub radio_targets: Vec<&'g Vec>>, pub file_access: &'g dyn FileAccessInterface, @@ -98,7 +98,7 @@ impl<'g, 's> Default for GlobalSettings<'g, 's> { } } -#[derive(Debug, Clone, PartialEq, Default)] +#[derive(Clone, PartialEq, Default)] pub enum HeadlineLevelFilter { Odd, diff --git a/src/parser/affiliated_keyword.rs b/src/parser/affiliated_keyword.rs index a684ce41..b9774a9f 100644 --- a/src/parser/affiliated_keyword.rs +++ b/src/parser/affiliated_keyword.rs @@ -31,10 +31,7 @@ use crate::types::AffiliatedKeywordValue; use crate::types::AffiliatedKeywords; use crate::types::Keyword; -#[cfg_attr( - feature = "tracing", - tracing::instrument(ret, level = "debug", skip(context)) -)] +#[cfg_attr(feature = "tracing", tracing::instrument(ret, level = "debug"))] pub(crate) fn affiliated_keywords<'s>( input: OrgSource<'s>, ) -> Res, Vec>> { diff --git a/src/parser/in_buffer_settings.rs b/src/parser/in_buffer_settings.rs index e6719403..76c3a7fa 100644 --- a/src/parser/in_buffer_settings.rs +++ b/src/parser/in_buffer_settings.rs @@ -84,7 +84,10 @@ fn in_buffer_settings_key<'s>(input: OrgSource<'s>) -> Res, OrgSou ))(input) } -#[cfg_attr(feature = "tracing", tracing::instrument(level = "debug"))] +#[cfg_attr( + feature = "tracing", + tracing::instrument(level = "debug", skip(original_settings)) +)] pub(crate) fn apply_in_buffer_settings<'g, 's, 'sf>( keywords: Vec>, original_settings: &'g GlobalSettings<'g, 's>, From e11de60def77997849d51c734ada389244ca0017 Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Wed, 18 Oct 2023 18:39:04 -0400 Subject: [PATCH 4/4] Clippy fixes. --- src/parser/affiliated_keyword.rs | 4 ++-- src/parser/keyword.rs | 9 +-------- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/src/parser/affiliated_keyword.rs b/src/parser/affiliated_keyword.rs index b9774a9f..346c9086 100644 --- a/src/parser/affiliated_keyword.rs +++ b/src/parser/affiliated_keyword.rs @@ -149,7 +149,7 @@ where AffiliatedKeywords { keywords: ret } } -fn translate_name<'g, 's>(name: &'s str) -> String { +fn translate_name(name: &str) -> String { let name_until_optval = name .split_once('[') .map(|(before, _after)| before) @@ -169,7 +169,7 @@ enum AffiliatedKeywordType { ObjectTree, } -fn identify_keyword_type<'g, 's>(name: &'s str) -> AffiliatedKeywordType { +fn identify_keyword_type(name: &str) -> AffiliatedKeywordType { let is_multiple = ["CAPTION", "HEADER"] .into_iter() .any(|candidate| name.eq_ignore_ascii_case(candidate)) diff --git a/src/parser/keyword.rs b/src/parser/keyword.rs index f6cca6c4..32db3613 100644 --- a/src/parser/keyword.rs +++ b/src/parser/keyword.rs @@ -221,19 +221,12 @@ mod tests { use test::Bencher; use super::*; - use crate::context::Context; - use crate::context::ContextElement; - use crate::context::GlobalSettings; - use crate::context::List; use crate::parser::OrgSource; #[bench] fn bench_affiliated_keyword(b: &mut Bencher) { let input = OrgSource::new("#+CAPTION[*foo*]: bar *baz*"); - let global_settings = GlobalSettings::default(); - let initial_context = ContextElement::document_context(); - let initial_context = Context::new(&global_settings, List::new(&initial_context)); - b.iter(|| assert!(affiliated_keyword(&initial_context, input).is_ok())); + b.iter(|| assert!(affiliated_keyword(input).is_ok())); } }