Skip to content

Critical important if fits the RC cicle before next RC subrelease: I fixed few rules with definitions.yaml and navigate.yaml, and implemented navigate.rs hungarian tests - #753

Merged
NSoiffer merged 4 commits into
daisy:hufrom
hammera:hu
Sep 7, 2026

Conversation

@hammera

@hammera hammera commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Hi boys,

@NSoiffer, @MartheGjelstad, or @moritz-gross, I doed few smaller modifications into Rules/languages/hu/definitions.yaml file with navigation parts related, and doed few modifications the Rules/Languages/hu/navigate.yaml file.
Final I implemented tests/Languages/hu/navigate.rs test file, based with english tests.
I proofread the tests outputs, Most of the tests are completely fine, except for some of the zoom tests where I can't remove the "in" part for some reason.
Few examples in hungarian navigate.rs test file:

fn parts_prefix_logarithm_with_base() -> Result<()> {
    // Intent/general.yaml log-with-base → logarithm-with-base:prefix; parts "base"
    let expr = r#"
      <math>
        <msub id="log">
          <mi>log</mi>
          <mi id="b">b</mi>
        </msub>
      </math>
    "#;
    assert_zoom_in("ZoomIn", expr, "nagyítás; in alap; b")
}

#[test]
fn parts_infix_power() -> Result<()> {
    // power:infix; parts "base; exponent"
    let expr = r#"
      <math>
        <msup id="pow">
          <mi id="alap">x</mi>
          <mn id="kitevő">2</mn>
        </msup>
      </math>
    "#;
    assert_zoom_in("ZoomIn", expr, "nagyítás; in alap; x")
}

In the Hungarian navigate.yaml rule, I intentionally specified the string "in" as a translation string "" in one place, because in this case, the suffix "ban" is not necessary in Hungarian after the word "nagyítás" (zoom in), since it is not possible to know whether the next part begins with a vowel or a consonant, or whether an odd or even number comes after the navigation command. In Hungarian, we also use four types of "in" suffixes corresponding to the English language, the "ban", the "ben", the ból and the "ből" suffixes, depending on what these suffixes follow.
So simpler use the "" translate string this situation when the navigation command is zoom in.

Attila

Signed-off-by: Attila Hammer <hammera@pickup.hu>
Signed-off-by: Attila Hammer <hammera@pickup.hu>
Signed-off-by: Attila Hammer <hammera@pickup.hu>
@hammera

hammera commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Oh, again a Clippy problem, now I use rustc 1.98.1 (48a229cea 2026-09-01) version.
cargo clippy doed following three fixes with three files, I will be temporary committing this three fixes into my hu branch, because the upstream hu branch doesn't contains latest main branch state:

diff --git a/src/canonicalize.rs b/src/canonicalize.rs
index 380c4de0..d20374d4 100644
--- a/src/canonicalize.rs
+++ b/src/canonicalize.rs
@@ -2412,23 +2412,21 @@ impl CanonicalizeContext {
 			// This is not yet in canonical form, so the fences may be siblings or siblings of the parent 
 			let preceding_siblings = as_element(children[0]).preceding_siblings();
 			let following_siblings = as_element(children[end-1]).following_siblings();
-			let first_child;
-			let last_child;
-			if preceding_siblings.is_empty() && following_siblings.is_empty() {
+			
+			
+			let (first_child, last_child) = if preceding_siblings.is_empty() && following_siblings.is_empty() {
 				// number spans all children, look to parent for fences
 				let preceding_children = mrow.preceding_siblings();
 				let following_children = mrow.following_siblings();
 				if preceding_children.is_empty() || following_children.is_empty() {
 					return true;	// doesn't have left or right fence
 				}
-				first_child = preceding_children[preceding_children.len()-1];
-				last_child = following_children[0];
+				(preceding_children[preceding_children.len()-1], following_children[0])
 			} else if preceding_siblings.is_empty() || following_siblings.is_empty() {
 				return true; // can't be fences around it
 			} else {
-				first_child = preceding_siblings[preceding_siblings.len()-1];
-				last_child = following_siblings[0];
-			}
+				(preceding_siblings[preceding_siblings.len()-1], following_siblings[0])
+			};
 			let first_child = as_element(first_child);
 			let last_child = as_element(last_child);
 			return !(name(first_child) == "mo" && is_fence(first_child) &&
diff --git a/src/chemistry.rs b/src/chemistry.rs
index 1176d8d3..23de8668 100644
--- a/src/chemistry.rs
+++ b/src/chemistry.rs
@@ -1384,17 +1384,14 @@ pub fn likely_adorned_chem_formula(mathml: Element) -> i32 {
         // prescripts are normally positive integers, chem 2.5.1 allows for a superscript for a Lewis dot
         // postscript should be a charge
 
-        let prescripts;
-        let postscripts;
-        if children.len() == 4 && name(as_element(children[1]))=="mprescripts" { // just prescripts
-            prescripts = &children[2..4];
-            postscripts = &children[0..0]; // empty
+        
+        
+        let (prescripts, postscripts) = if children.len() == 4 && name(as_element(children[1]))=="mprescripts" { // just prescripts
+            (&children[2..4], &children[0..0]) // empty
         } else if children.len() == 6 && name(as_element(children[3]))=="mprescripts" {  // pre and postscripts
-            prescripts = &children[4..6];
-            postscripts = &children[1..3]; // empty
+            (&children[4..6], &children[1..3]) // empty
         } else if children.len() == 3 || children.len() == 5 {   // just postscripts (simultaneous or offset)
-            prescripts = &children[0..0]; // empty
-            postscripts = &children[1..];
+            (&children[0..0], &children[1..])
         } else {
             return NOT_CHEMISTRY;
         };
diff --git a/src/tts.rs b/src/tts.rs
index 91b05c0a..c2f951b3 100644
--- a/src/tts.rs
+++ b/src/tts.rs
@@ -669,18 +669,16 @@ impl TTS {
     /// There is a bias towards pausing more _after_ longer strings.
     pub fn compute_auto_pause(&self, prefs: &PreferenceManager, before: &str, after: &str) -> Result<String> {
         static REMOVE_XML: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"<.+?>").unwrap()); // punctuation ending with a '.'
-        let before_len;
-        let after_len;
-        match self {
+        
+        
+        let (before_len, after_len) = match self {
             TTS::SSML | TTS::SAPI5 => {
-                before_len = REMOVE_XML.replace_all(before, "").len();
-                after_len = REMOVE_XML.replace_all(after, "").len();
+                (REMOVE_XML.replace_all(before, "").len(), REMOVE_XML.replace_all(after, "").len())
             },
             _ => {
-                before_len = before.len();
-                after_len = after.len();
+                (before.len(), after.len())
             },
-        }
+        };
 
         // pause values are not cut in stone
         // the calculation bias to 'previous' is based on MathPlayer which used '30 * #-of-descendants-on-left

Attila

@hammera

hammera commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Ok, the Clippy fix is work correct in online workflows too.
If main branch is not applyed the quoted diff patch, need committing too this commit, othervise, need merge only the first three commits.
Neil, have possibility to do a merge to the main branch, and after this the hu branch to upstream hu branch contains latest RC version related changes too?
I not doed now interactive rebase command to the upstream main branch, to prevent lot of not relevant main branch commit to my feature branch with not have yet the upstream hu branch.

Attila

@NSoiffer

NSoiffer commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Most of the tests are completely fine, except for some of the zoom tests where I can't remove the "in" part for some reason.

I see that you found the place in defintions.yaml to make a change. Did that fix the problem for you?

@NSoiffer
NSoiffer merged commit 4d3d73e into daisy:hu Sep 7, 2026
8 checks passed
@github-project-automation github-project-automation Bot moved this from Triage to Done in MathCAT Project Board Sep 7, 2026
@hammera

hammera commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

@NSoiffer, partially.
I only not founded solution to remove unneed „in” texts with some zoom tests.
Few examples:

  • First example, actual code:
fn parts_prefix_logarithm_with_base() -> Result<()> {
    // Intent/general.yaml log-with-base → logarithm-with-base:prefix; parts "base"
    let expr = r#"
      <math>
        <msub id="log">
          <mi>log</mi>
          <mi id="b">b</mi>
        </msub>
      </math>
    "#;
    assert_zoom_in("ZoomIn", expr, "nagyítás; in alap; b")
}

This test, independent hungarian language related rules, partially right.
Full right wanted output:

    assert_zoom_in("ZoomIn", expr, "nagyítás; alap; b")
  • Second example:
fn parts_infix_power() -> Result<()> {
    // power:infix; parts "base; exponent"
    let expr = r#"
      <math>
        <msup id="pow">
          <mi id="alap">x</mi>
          <mn id="kitevő">2</mn>
        </msup>
      </math>
    "#;
    assert_zoom_in("ZoomIn", expr, "nagyítás; in alap; x")
}

The situation this test output is the same, this test output are partially right only in simple speak, because the „in” text part are have the text.
Full right output:

    assert_zoom_in("ZoomIn", expr, "nagyítás; alap; x")
  • Third example:
fn parts_function_fraction() -> Result<()> {
    // fraction (from mfrac); parts "numerator; denominator"
    let expr = r#"
      <math>
        <mfrac id="frac">
          <mn id="számláló">1</mn>
          <mn id="nevező">2</mn>
        </mfrac>
      </math>
    "#;
    assert_zoom_in("ZoomIn", expr, "nagyítás; in számláló; 1")
}

This test are partial right.
Full right wanted text is following:

    assert_zoom_in("ZoomIn", expr, "nagyítás; számláló; 1")

Original english text this test is following:

fn parts_function_fraction() -> Result<()> {
    // fraction (from mfrac); parts "numerator; denominator"
    let expr = r#"
      <math>
        <mfrac id="frac">
          <mn id="num">1</mn>
          <mn id="den">2</mn>
        </mfrac>
      </math>
    "#;
    assert_zoom_in("ZoomIn", expr, "zoom in; in numerator; 1")
}

The english test output too why skipped the denominator part (the 2 number part)?

In Rules/Language/hu/navigate.yaml file, following place I removed the in text translation, not entire rule I quote, possible wrong place I would like fixing the unwanted in output part remove:

        - test:
          # CommandOffset is 1 + length of the English NavCommand stem (Zoom/Move/Read/Describe),
          # not the spoken Prefix length — so translations can use a different Prefix word.
          - if: "starts-with($NavCommand, 'Zoom')"
            then: [set_variables: [Prefix: "'nagyítás'", CommandOffset: "5"]]          # phrase('zoom' in to see more details)     
          - else_if: "starts-with($NavCommand, 'Move')"
            then: [set_variables: [Prefix: "'ugrás'", CommandOffset: "5"]]          # phrase('move' to next entry in table)     
          - else_if: "starts-with($NavCommand, 'Read')"
            then: [set_variables: [Prefix: "'olvasás'", CommandOffset: "5"]]          # phrase('read' to next entry in table)     
          - else_if: "starts-with($NavCommand, 'Describe')"
            then: [set_variables: [Prefix: "'leírás'", CommandOffset: "9"]]      # phrase('describe' to next entry in table)
        - test:
            if: "$Prefix != ''"
            then:
            - x: "$Prefix"
            - test:
              - if: "substring($NavCommand, $CommandOffset) = 'In'"
                then: [T: ""]                                  # phrase(zoom 'in' to see more details)
              - else_if: "substring($NavCommand, $CommandOffset) = 'InAll'"
                # HACK: '\uF8FE' is used internally for the concatenation char by 'ct' -- this gets "ed" concatenated to "zoom"
                then: [T: "Teljesen nagyítsa ki"]                     # phrase(zoom 'out all of the way' to see more details)
              - else_if: "substring($NavCommand, $CommandOffset) = 'Out'"
                then: [T: "ki"]                                 # phrase(zoom 'out' to see more details)
              - else_if: "substring($NavCommand, $CommandOffset) = 'OutAll'"
                # HACK: '\uF8FE' is used internally for the concatenation char by 'ct' -- this gets "ed" concatenated to "zoom"
                then: [T: "teljes táblázat kinagyítása"]                     # phrase(zoom 'out all of the way' to see more details)
              - else_if: "substring($NavCommand, $CommandOffset) = 'Next'"
                then: [T: "jobbra"]                               # phrase(move to the 'right')
              - else_if: "substring($NavCommand, $CommandOffset) = 'Previous'"
                then: [T: "balra"]                                # phrase(move to the 'left')
              - else_if: "substring($NavCommand, $CommandOffset) = 'Current'"
                then: [T: "jelenlegi"]                                 # phrase(who is the 'current' president)
              - else_if: "substring($NavCommand, $CommandOffset) = 'LineStart'"
                then: [T: "a sor elejére"]                                 # phrase(move 'to start of line')
              - else_if: "substring($NavCommand, $CommandOffset) = 'LineEnd'"
                then: [T: "a sor végére"]                                 # phrase(move 'to end of line')
            - pause: "medium"
  - set_variables: [MatchCounter: "$MatchCounter + 1"]

The empty in translation is why not good during mathCat navigation tests to skyp unwanted in texts?

Attila

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants