Skip to content

Commit

Permalink
Clean up use of reference to elements. Passing elements is cheap and …
Browse files Browse the repository at this point in the history 
…some functions passed them and some passed references. I changed the ones I found to pass elements.
  • Loading branch information
@NSoiffer
NSoiffer committed Jan 29, 2025
1 parent 7fdac2d commit 741ddfa
Show file tree
Hide file tree
Showing 12 changed files with 481 additions and 481 deletions.
94 changes: 47 additions & 47 deletions src/braille.rs
View file

Large diffs are not rendered by default.

408 changes: 204 additions & 204 deletions src/canonicalize.rs
View file

Large diffs are not rendered by default.

240 changes: 120 additions & 120 deletions src/chemistry.rs
View file

Large diffs are not rendered by default.

48 changes: 24 additions & 24 deletions src/infer_intent.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,18 @@ pub fn infer_intent<'r, 'c, 's:'c, 'm:'c>(rules_with_context: &'r mut SpeechRule

fn catch_errors_building_intent<'r, 'c, 's:'c, 'm:'c>(rules_with_context: &'r mut SpeechRulesWithContext<'c,'s,'m>, mathml: Element<'c>) -> Result<Element<'m>> {
if let Some(intent_str) = mathml.attribute_value(INTENT_ATTR) {
// debug!("Before intent: {}", crate::pretty_print::mml_to_string(&mathml));
// debug!("Before intent: {}", crate::pretty_print::mml_to_string(mathml));
let mut lex_state = LexState::init(intent_str.trim())?;
let result = build_intent(rules_with_context, &mut lex_state, mathml)
.chain_err(|| format!("occurs before '{}' in intent attribute value '{}'", lex_state.remaining_str, intent_str))?;
if lex_state.token != Token::None {
bail!("Error in intent value: extra unparsed intent '{}' in intent attribute value '{}'", lex_state.remaining_str, intent_str);
}
assert!(lex_state.remaining_str.is_empty());
// debug!("Resulting intent:\n{}", crate::pretty_print::mml_to_string(&result));
// debug!("Resulting intent:\n{}", crate::pretty_print::mml_to_string(result));
return Ok(result);
}
bail!("Internal error: infer_intent() called on MathML with no intent arg:\n{}", mml_to_string(&mathml));
bail!("Internal error: infer_intent() called on MathML with no intent arg:\n{}", mml_to_string(mathml));
}
}

Expand Down Expand Up @@ -214,7 +214,7 @@ fn build_intent<'b, 'r, 'c, 's:'c, 'm:'c>(rules_with_context: &'r mut SpeechRule
// debug!(" start build_intent: state: {}", lex_state);
let doc = rules_with_context.get_document();
let mut intent;
// debug!(" build_intent: start mathml name={}", name(&mathml));
// debug!(" build_intent: start mathml name={}", name(mathml));
match lex_state.token {
Token::Property(_) => {
// We only have a property -- we want to keep this tag/element
Expand All @@ -224,15 +224,15 @@ fn build_intent<'b, 'r, 'c, 's:'c, 'm:'c>(rules_with_context: &'r mut SpeechRule
// Note: to avoid infinite loop, we need to remove the 'intent' so we don't end up back here; we put it back later
let properties = get_properties(lex_state)?; // advance state to see if funcall
if lex_state.is_terminal("(") {
intent = create_mathml_element(&doc, name(&mathml));
intent = create_mathml_element(&doc, name(mathml));
intent.set_attribute_value(INTENT_PROPERTY, &properties);
intent.set_attribute_value(MATHML_FROM_NAME_ATTR, name(&mathml));
intent.set_attribute_value(MATHML_FROM_NAME_ATTR, name(mathml));
} else {
let saved_intent = mathml.attribute_value(INTENT_ATTR).unwrap();
mathml.remove_attribute(INTENT_ATTR);
mathml.set_attribute_value(INTENT_PROPERTY, &properties); // needs to be set before the pattern match
intent = rules_with_context.match_pattern::<Element<'m>>(mathml)?;
// debug!("Intent after pattern match:\n{}", mml_to_string(&intent));
// debug!("Intent after pattern match:\n{}", mml_to_string(intent));
mathml.set_attribute_value(INTENT_ATTR, saved_intent);
}
return Ok(intent); // if we start with properties, then there can only be properties
Expand All @@ -243,7 +243,7 @@ fn build_intent<'b, 'r, 'c, 's:'c, 'm:'c>(rules_with_context: &'r mut SpeechRule
// if the str is part of a larger intent and not the head (e.g., "a" in "f($x, a)", but not the "f" in it), then it is "made up"
// debug!(" Token::ConceptOrLiteral, word={}, leaf_name={}", word, leaf_name);
intent.set_attribute_value(MATHML_FROM_NAME_ATTR,
if word == mathml.attribute_value(INTENT_ATTR).unwrap_or_default() {name(&mathml)} else {leaf_name});
if word == mathml.attribute_value(INTENT_ATTR).unwrap_or_default() {name(mathml)} else {leaf_name});
intent.set_text(word); // '-' and '_' get removed by the rules.
lex_state.get_next()?;
if let Token::Property(_) = lex_state.token {
Expand All @@ -269,7 +269,7 @@ fn build_intent<'b, 'r, 'c, 's:'c, 'm:'c>(rules_with_context: &'r mut SpeechRule
if lex_state.is_terminal("(") {
intent = build_function(intent, rules_with_context, lex_state, mathml)?;
}
// debug!(" end build_intent: state: {} piece: {}", lex_state, mml_to_string(&intent));
// debug!(" end build_intent: state: {} piece: {}", lex_state, mml_to_string(intent));
return Ok(intent);
}

Expand Down Expand Up @@ -305,29 +305,29 @@ fn build_function<'b, 'r, 'c, 's:'c, 'm:'c>(
rules_with_context: &'r mut SpeechRulesWithContext<'c,'s,'m>,
lex_state: &mut LexState<'b>,
mathml: Element<'c>) -> Result<Element<'m>> {
// debug!(" start build_function: name: {}, state: {}", name(&function_name), lex_state);
// debug!(" start build_function: name: {}, state: {}", name(function_name), lex_state);
// application := intent '(' arguments? S ')' where 'function_name' is 'intent'
assert!(lex_state.is_terminal("("));
let mut function = function_name;
function.set_attribute_value(MATHML_FROM_NAME_ATTR, name(&mathml));
function.set_attribute_value(MATHML_FROM_NAME_ATTR, name(mathml));
while lex_state.is_terminal("(") {
lex_state.get_next()?;
if lex_state.is_terminal(")") {
// grammar requires at least one argument
bail!("Illegal 'intent' syntax: missing argument for intent name '{}'", name(&function_name));
bail!("Illegal 'intent' syntax: missing argument for intent name '{}'", name(function_name));
}
let children = build_arguments(rules_with_context, lex_state, mathml)?;
function = lift_function_name(rules_with_context.get_document(), function, children);

if !lex_state.is_terminal(")") {
bail!("Illegal 'intent' syntax: missing ')' for intent name '{}'", name(&function_name));
bail!("Illegal 'intent' syntax: missing ')' for intent name '{}'", name(function_name));
}
lex_state.get_next()?;
}

let function = add_fixity_children(function);
// debug!(" end build_function/# children: {}, #state: {} ..[bfa] function name: {}",
// function.children().len(), lex_state, mml_to_string(&function));
// function.children().len(), lex_state, mml_to_string(function));
return Ok(function);

fn add_fixity_children(mathml: Element) -> Element {
Expand All @@ -341,7 +341,7 @@ fn build_function<'b, 'r, 'c, 's:'c, 'm:'c>(
}
let doc = mathml.document();
if let Some(properties) = mathml.attribute_value(INTENT_PROPERTY) {
let op_name = name(&mathml);
let op_name = name(mathml);
if let Some(property) = properties.rsplit(':').find(|&property| FIXITIES.contains(property)) {
// fixities don't do anything it there is just one child
// we also exclude fixity on mtable because they mess up the counts (see 'en::mtable::unknown_mtable_property')
Expand Down Expand Up @@ -411,15 +411,15 @@ fn build_arguments<'b, 'r, 'c, 's:'c, 'm:'c>(

/// lift the children up to LITERAL_NAME
fn lift_function_name<'m>(doc: Document<'m>, function_name: Element<'m>, children: Vec<Element<'m>>) -> Element<'m> {
// debug!(" lift_function_name: {}", name(&function_name));
// debug!(" lift_function_name: {}", mml_to_string(&function_name));
if name(&function_name) == "mi" || name(&function_name) == "mn" { // FIX -- really want to test for all leaves, but not "data-from-mathml"
// debug!(" lift_function_name: {}", name(function_name));
// debug!(" lift_function_name: {}", mml_to_string(function_name));
if name(function_name) == "mi" || name(function_name) == "mn" { // FIX -- really want to test for all leaves, but not "data-from-mathml"
// simple/normal case of f(x,y)
// don't want to say that this is a leaf -- doing so messes up because it potentially has children
set_mathml_name(function_name, as_text(function_name));
function_name.set_text("");
function_name.replace_children(children);
if name(&function_name).find(|ch| ch!='_' && ch!='-').is_none() {
if name(function_name).find(|ch| ch!='_' && ch!='-').is_none() {
let properties = function_name.attribute_value(INTENT_PROPERTY).unwrap_or(":").to_owned();
function_name.set_attribute_value(INTENT_PROPERTY, &(properties + "silent:"));
}
Expand All @@ -443,7 +443,7 @@ fn lift_function_name<'m>(doc: Document<'m>, function_name: Element<'m>, childre
/// look for @arg=name in mathml
/// if 'check_intent', then look at an @intent for this element (typically false for non-recursive calls)
fn find_arg<'r, 'c, 's:'c, 'm:'c>(rules_with_context: &'r mut SpeechRulesWithContext<'c,'s,'m>, name: &str, mathml: Element<'c>, skip_self: bool, no_check_inside: bool) -> Result<Option<Element<'m>>> {
// debug!("Looking for '{}' in\n{}", name, mml_to_string(&mathml));
// debug!("Looking for '{}' in\n{}", name, mml_to_string(mathml));
if !skip_self {
if let Some(arg_val) = mathml.attribute_value("arg") {
// debug!("looking for '{}', found arg='{}'", name, arg_val);
Expand Down Expand Up @@ -496,12 +496,12 @@ mod tests {
let package1 = &parser::parse(mathml).expect("Failed to parse test input");
let mathml = get_element(package1);
trim_element(mathml, false);
debug!("test:\n{}", crate::pretty_print::mml_to_string(&mathml));
debug!("test:\n{}", crate::pretty_print::mml_to_string(mathml));

let package2 = &parser::parse(target).expect("Failed to parse target input");
let target = get_element(package2);
trim_element(target,true);
debug!("target:\n{}", crate::pretty_print::mml_to_string(&target));
debug!("target:\n{}", crate::pretty_print::mml_to_string(target));

let result = match crate::speech::intent_from_mathml(mathml, package2.as_document()) {
Ok(e) => e,
Expand All @@ -510,8 +510,8 @@ mod tests {
return false; // could be intentional failure
}
};
debug!("result:\n{}", crate::pretty_print::mml_to_string(&result));
match is_same_element(&result, &target) {
debug!("result:\n{}", crate::pretty_print::mml_to_string(result));
match is_same_element(result, target) {
Ok(_) => return true,
Err(e) => panic!("{}", e),
}
Expand Down
34 changes: 17 additions & 17 deletions src/interface.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ pub fn set_mathml(mathml_str: String) -> Result<String> {
let new_package = new_package.unwrap();
let mathml = get_element(&new_package);
let mathml = cleanup_mathml(mathml)?;
let mathml_string = mml_to_string(&mathml);
let mathml_string = mml_to_string(mathml);
old_package.replace(new_package);

return Ok(mathml_string);
Expand All @@ -162,7 +162,7 @@ pub fn get_spoken_text() -> Result<String> {
let mathml = get_element(&package_instance);
let new_package = Package::new();
let intent = crate::speech::intent_from_mathml(mathml, new_package.as_document())?;
debug!("Intent tree:\n{}", mml_to_string(&intent));
debug!("Intent tree:\n{}", mml_to_string(intent));
let speech = crate::speech::speak_mathml(intent, "")?;
// info!("Time taken: {}ms", instant.elapsed().as_millis());
return Ok(speech);
Expand Down Expand Up @@ -330,7 +330,7 @@ pub fn get_navigation_braille() -> Result<String> {
// get the MathML node and wrap it inside of a <math> element
// if the offset is given, we need to get the character it references
if offset == 0 {
if name(&found) == "math" {
if name(found) == "math" {
Ok(found)
} else {
let new_mathml = create_mathml_element(&new_doc, "math");
Expand All @@ -342,10 +342,10 @@ pub fn get_navigation_braille() -> Result<String> {
bail!(
"Internal error: non-zero offset '{}' on a non-leaf element '{}'",
offset,
name(&found)
name(found)
);
} else if let Some(ch) = as_text(found).chars().nth(offset) {
let internal_mathml = create_mathml_element(&new_doc, name(&found));
let internal_mathml = create_mathml_element(&new_doc, name(found));
internal_mathml.set_text(&ch.to_string());
let new_mathml = create_mathml_element(&new_doc, "math");
new_mathml.append_child(internal_mathml);
Expand All @@ -355,7 +355,7 @@ pub fn get_navigation_braille() -> Result<String> {
bail!(
"Internal error: offset '{}' on leaf element '{}' doesn't exist",
offset,
mml_to_string(&found)
mml_to_string(found)
);
}
}
Expand Down Expand Up @@ -451,7 +451,7 @@ pub fn get_navigation_mathml() -> Result<(String, usize)> {
return NAVIGATION_STATE.with(|nav_stack| {
return match nav_stack.borrow_mut().get_navigation_mathml(mathml) {
Err(e) => Err(e),
Ok((found, offset)) => Ok((mml_to_string(&found), offset)),
Ok((found, offset)) => Ok((mml_to_string(found), offset)),
};
});
});
Expand Down Expand Up @@ -502,7 +502,7 @@ pub fn get_navigation_node_from_braille_position(position: usize) -> Result<(Str
fn copy_mathml(mathml: Element) -> Element {
// If it represents MathML, the 'Element' can only have Text and Element children along with attributes
let children = mathml.children();
let new_mathml = create_mathml_element(&mathml.document(), name(&mathml));
let new_mathml = create_mathml_element(&mathml.document(), name(mathml));
if is_leaf(mathml) {
mathml.attributes().iter().for_each(|attr| {
new_mathml.set_attribute_value(attr.name(), attr.value());
Expand Down Expand Up @@ -641,7 +641,7 @@ pub fn trim_element(e: Element, allow_structure_in_leaves: bool) {
}

// CSS considers only space, tab, linefeed, and carriage return as collapsable whitespace
if !(is_leaf(e) || name(&e) == "intent-literal" || single_text.is_empty()) {
if !(is_leaf(e) || name(e) == "intent-literal" || single_text.is_empty()) {
// intent-literal comes from testing intent
// FIX: we have a problem -- what should happen???
// FIX: For now, just keep the children and ignore the text and log an error -- shouldn't panic/crash
Expand Down Expand Up @@ -673,7 +673,7 @@ pub fn trim_element(e: Element, allow_structure_in_leaves: bool) {
for child in children {
let child_text = match child {
ChildOfElement::Element(child) => {
if name(&child) == "mglyph" {
if name(child) == "mglyph" {
child.attribute_value("alt").unwrap_or("").to_string()
} else {
gather_text(child)
Expand Down Expand Up @@ -737,7 +737,7 @@ fn is_same_doc(doc1: &Document, doc2: &Document) -> Result<()> {
match c1 {
ChildOfRoot::Element(e1) => {
if let ChildOfRoot::Element(e2) = c2 {
is_same_element(e1, e2)?;
is_same_element(*e1, *e2)?;
} else {
bail!("child #{}, first is element, second is something else", i);
}
Expand Down Expand Up @@ -771,7 +771,7 @@ fn is_same_doc(doc1: &Document, doc2: &Document) -> Result<()> {
/// returns Ok() if two Documents are equal or some info where they differ in the Err
// Not really meant to be public -- used by tests in some packages
#[allow(dead_code)]
pub fn is_same_element(e1: &Element, e2: &Element) -> Result<()> {
pub fn is_same_element(e1: Element, e2: Element) -> Result<()> {
enable_logs();
if name(e1) != name(e2) {
bail!("Names not the same: {}, {}", name(e1), name(e2));
Expand All @@ -796,7 +796,7 @@ pub fn is_same_element(e1: &Element, e2: &Element) -> Result<()> {
match c1 {
ChildOfElement::Element(child1) => {
if let ChildOfElement::Element(child2) = c2 {
is_same_element(child1, child2)?;
is_same_element(*child1, *child2)?;
} else {
bail!("{} child #{}, first is element, second is something else", name(e1), i);
}
Expand Down Expand Up @@ -886,12 +886,12 @@ mod tests {
let target_package = &parser::parse(target).expect("Failed to parse input");
let target_doc = target_package.as_document();
trim_doc(&target_doc);
debug!("target:\n{}", mml_to_string(&get_element(&target_package)));
debug!("target:\n{}", mml_to_string(get_element(&target_package)));

let test_package = &parser::parse(test).expect("Failed to parse input");
let test_doc = test_package.as_document();
trim_doc(&test_doc);
debug!("test:\n{}", mml_to_string(&get_element(&test_package)));
debug!("test:\n{}", mml_to_string(get_element(&test_package)));

match is_same_doc(&test_doc, &target_doc) {
Ok(_) => return true,
Expand Down Expand Up @@ -962,12 +962,12 @@ mod tests {
let package1 = &parser::parse(whitespace_str).expect("Failed to parse input");
let doc1 = package1.as_document();
trim_doc(&doc1);
debug!("doc1:\n{}", mml_to_string(&get_element(&package1)));
debug!("doc1:\n{}", mml_to_string(get_element(&package1)));

let package2 = parser::parse(different_str).expect("Failed to parse input");
let doc2 = package2.as_document();
trim_doc(&doc2);
debug!("doc2:\n{}", mml_to_string(&get_element(&package2)));
debug!("doc2:\n{}", mml_to_string(get_element(&package2)));

assert!(is_same_doc(&doc1, &doc2).is_err());
}
Expand Down
8 changes: 4 additions & 4 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,17 +102,17 @@ pub fn are_strs_canonically_equal_with_locale(test: &str, target: &str, block_se
let package1 = &parser::parse(test).expect("Failed to parse test input");
let mathml = get_element(package1);
trim_element(mathml, false);
// debug!("test:\n{}", mml_to_string(&mathml));
// debug!("test:\n{}", mml_to_string(mathml));
let mathml_test = canonicalize(mathml).unwrap();

let package2 = &parser::parse(target).expect("Failed to parse target input");
let mathml_target = get_element(package2);
trim_element(mathml_target, false);
// debug!("target:\n{}", mml_to_string(&mathml_target));
// debug!("target:\n{}", mml_to_string(mathml_target));

match is_same_element(&mathml_test, &mathml_target) {
match is_same_element(mathml_test, mathml_target) {
Ok(_) => return true,
Err(e) => panic!("{}\nResult:\n{}\nTarget:\n{}", e, mml_to_string(&mathml_test), mml_to_string(&mathml_target)),
Err(e) => panic!("{}\nResult:\n{}\nTarget:\n{}", e, mml_to_string(mathml_test), mml_to_string(mathml_target)),
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ fn main() {
// </math>";

let expr = r#"
<math display='block' intent:>
<math display='block'>
<mi data-chem-element='3' id='id-5' data-id-added='true'>Co</mi>
</math>
Expand Down
Loading

0 comments on commit 741ddfa

Please sign in to comment.