From dff37bf08d3da2aeeb4edbd0d457189f6b9d725f Mon Sep 17 00:00:00 2001 From: mgros Date: Wed, 12 Aug 2026 00:20:06 +0200 Subject: [PATCH 1/7] Add `HasVisibleColumnLine` XPath function to handle table separators in speech output - Replace `count_table_dims` return type with `usize` values for clarity. - Update function registration to include `HasVisibleColumnLine`. - Modify speech tests to reflect separator usage in matrix descriptions. --- Rules/Languages/en/SharedRules/default.yaml | 3 + src/speech.rs | 2 +- src/xpath_functions.rs | 77 +++++++++++++++++++-- tests/Languages/en/mtable.rs | 31 ++++++--- 4 files changed, 99 insertions(+), 14 deletions(-) diff --git a/Rules/Languages/en/SharedRules/default.yaml b/Rules/Languages/en/SharedRules/default.yaml index 21cfc256..db35793c 100644 --- a/Rules/Languages/en/SharedRules/default.yaml +++ b/Rules/Languages/en/SharedRules/default.yaml @@ -493,6 +493,9 @@ - x: "count(preceding-sibling::*)+IfThenElse(parent::m:mlabeledtr, 0, 1)" - pause: medium - x: "*" + - test: + if: "HasVisibleColumnLine(../.., count(preceding-sibling::*) + 1)" + then: [t: "separator"] - test: # short pause after each element; medium pause if last element in a row; long pause for last element in matrix - if: count(following-sibling::*) > 0 diff --git a/src/speech.rs b/src/speech.rs index 0c956fe9..e2663f6a 100644 --- a/src/speech.rs +++ b/src/speech.rs @@ -1758,7 +1758,7 @@ impl<'c, 'r> ContextStack<'c> { fn base_context(var_defs: PreferenceHashMap) -> sxd_xpath_no_unsafe::Context<'c> { let mut context = sxd_xpath_no_unsafe::Context::new(); context.set_namespace("m", "http://www.w3.org/1998/Math/MathML"); - crate::xpath_functions::add_builtin_functions(&mut context); + crate::xpath_functions::register_mathcat_xpath_functions(&mut context); for (key, value) in var_defs { context.set_variable(key.as_str(), yaml_to_value(&value)); // if let Some(str_value) = value.as_str() { diff --git a/src/xpath_functions.rs b/src/xpath_functions.rs index 47b9d6ab..6a4e1311 100644 --- a/src/xpath_functions.rs +++ b/src/xpath_functions.rs @@ -1480,7 +1480,7 @@ impl CountTableDims { /// This function is relatively permissive. Non-`mtr` rows are /// ignored. The number of columns is determined only from the first /// row, if it exists. Within that row, non-`mtd` elements are ignored. - fn count_table_dims<'d>(mut self, e: Element<'_>) -> Result<(Value<'d>, Value<'d>), Error> { + fn count_table_dims(mut self, e: Element<'_>) -> (usize, usize) { for child in e.children() { let ChildOfElement::Element(row) = child else { continue @@ -1529,7 +1529,7 @@ impl CountTableDims { // columns, so we will not use them. let _extra_rows = self.extended_cells.keys().max().map(|k| k-1).unwrap_or(0); - Ok((Value::Number(self.num_rows as f64), Value::Number(self.num_cols as f64))) + (self.num_rows, self.num_cols) } fn evaluate<'d>(self, fn_name: &str, @@ -1540,7 +1540,8 @@ impl CountTableDims { let node = validate_one_node(element, fn_name)?; if let Node::Element(e) = node { if is_tag(e, "mtable") { - return self.count_table_dims(e); + let (rows, columns) = self.count_table_dims(e); + return Ok((Value::Number(rows as f64), Value::Number(columns as f64))); } else { return Err(Error::Other { what: format!("Input element was a <{}>, not an ", as_qname!(e.name()).local_part()) }); @@ -1569,9 +1570,49 @@ impl Function for CountTableColumns { } } +/// Return whether a one-based mtable boundary has a visible column line. +/// +/// MathML repeats the final `columnlines` value for remaining boundaries. +/// Boundaries after the final column, and values other than `solid` and +/// `dashed`, do not describe a visible separator. +fn has_visible_column_line(table: Element, boundary: usize) -> bool { + if boundary == 0 || !is_tag(table, "mtable") { + return false; + } + + let (_, column_count) = CountTableDims::new().count_table_dims(table); + if boundary >= column_count { + return false; + } + + let line_style = table + .attribute_value("columnlines") + .and_then(|values| values.split_whitespace().take(boundary).last()); + return matches!(line_style, Some("solid" | "dashed")); +} + +struct HasVisibleColumnLine; +impl Function for HasVisibleColumnLine { + fn evaluate<'c, 'd>(&self, + _context: &context::Evaluation<'c, 'd>, + args: Vec>) -> Result, Error> { + let mut args = Args(args); + args.exactly(2)?; + let boundary = args.pop_number()?; + let table = validate_one_node(args.pop_nodeset()?, "HasVisibleColumnLine")?; + let Node::Element(table) = table else { + return Err(Error::Other { what: "HasVisibleColumnLine requires an mtable element".to_string() }); + }; + if !boundary.is_finite() || boundary < 1.0 || boundary.fract() != 0.0 { + return Ok(Value::Boolean(false)); + } + return Ok(Value::Boolean(has_visible_column_line(table, boundary as usize))); + } +} + /// Add all the functions defined in this module to `context`. -pub fn add_builtin_functions(context: &mut Context) { +pub fn register_mathcat_xpath_functions(context: &mut Context) { context.set_function("NestingChars", crate::braille::NemethNestingChars); context.set_function("BrailleChars", crate::braille::BrailleChars); context.set_function("NeedsToBeGrouped", crate::braille::NeedsToBeGrouped); @@ -1591,6 +1632,7 @@ pub fn add_builtin_functions(context: &mut Context) { context.set_function("GetNavigationPartName", GetNavigationPartName); context.set_function("CountTableRows", CountTableRows); context.set_function("CountTableColumns", CountTableColumns); + context.set_function("HasVisibleColumnLine", HasVisibleColumnLine); context.set_function("DEBUG", Debug); // Not used: remove?? @@ -1796,7 +1838,7 @@ mod tests { let package = parser::parse(mathml).map_err(|e| anyhow::anyhow!("failed to parse XML: {e}"))?; let math_elem = get_element(&package); let child = as_element(math_elem.children()[0]); - assert!(CountTableDims::new().count_table_dims(child) == Ok((Value::Number(dims.0 as f64), Value::Number(dims.1 as f64)))); + assert_eq!(CountTableDims::new().count_table_dims(child), dims); return Ok( () ); } @@ -1816,6 +1858,31 @@ mod tests { }); } + fn check_column_line(mathml: &str, boundary: usize, expected: bool) -> Result<()> { + let package = parser::parse(mathml).map_err(|e| anyhow::anyhow!("failed to parse XML: {e}"))?; + let math = get_element(&package); + let table = as_element(math.children()[0]); + assert_eq!(has_visible_column_line(table, boundary), expected); + return Ok(()); + } + + #[test] + fn visible_column_lines() -> Result<()> { + return xpath_test(|| { + check_column_line("", 1, false)?; + check_column_line("", 2, true)?; + check_column_line("", 3, true)?; + + check_column_line("", 3, true)?; + + check_column_line("", 1, false)?; + check_column_line("", 1, false)?; + check_column_line("", 1, false)?; + check_column_line("", 2, false)?; + return Ok(()); + }); + } + #[test] fn at_left_edge() -> Result<()> { return xpath_test(|| { diff --git a/tests/Languages/en/mtable.rs b/tests/Languages/en/mtable.rs index cbd2a527..be197013 100644 --- a/tests/Languages/en/mtable.rs +++ b/tests/Languages/en/mtable.rs @@ -271,8 +271,23 @@ fn augmented_matrix_2x3() -> Result<()> { ] "; - test("en", "ClearSpeak", expr, "the 2 by 3 augmented matrix; row 1; 3, 1, 4; row 2; 0, 2, 6")?; - test("en", "SimpleSpeak", expr, "the 2 by 3 augmented matrix; row 1; 3, 1, 4; row 2; 0, 2, 6")?; + test("en", "ClearSpeak", expr, "the 2 by 3 augmented matrix; row 1; 3, 1 separator, 4; row 2; 0, 2 separator, 6")?; + test("en", "SimpleSpeak", expr, "the 2 by 3 augmented matrix; row 1; 3, 1 separator, 4; row 2; 0, 2 separator, 6")?; + Ok(()) +} + +#[test] +fn dashed_augmented_matrix_separator() -> Result<()> { + let expr = " + + [ + + 123 + + ] + "; + test("en", "ClearSpeak", expr, "the 1 by 3 row matrix; 1 separator, 2 separator, 3")?; + test("en", "SimpleSpeak", expr, "the 1 by 3 row matrix; 1 separator, 2 separator, 3")?; Ok(()) } @@ -926,13 +941,13 @@ let expr = " "; test_ClearSpeak("en", "ClearSpeak_Matrix", "EndMatrix", - expr, "the 3 by 4 augmented matrix; row 1; column 1; 1, column 2; 2, column 3; negative 1, column 4; 3; \ - row 2; column 1; negative 3, column 2; 3, column 3; negative 1, column 4; 2; \ - row 3; column 1; 2, column 2; 3, column 3; 2, column 4; negative 1; end matrix")?; + expr, "the 3 by 4 augmented matrix; row 1; column 1; 1, column 2; 2, column 3; negative 1 separator, column 4; 3; \ + row 2; column 1; negative 3, column 2; 3, column 3; negative 1 separator, column 4; 2; \ + row 3; column 1; 2, column 2; 3, column 3; 2 separator, column 4; negative 1; end matrix")?; test("en", "SimpleSpeak", - expr, "the 3 by 4 augmented matrix; row 1; column 1; 1, column 2; 2, column 3; negative 1, column 4; 3; \ - row 2; column 1; negative 3, column 2; 3, column 3; negative 1, column 4; 2; \ - row 3; column 1; 2, column 2; 3, column 3; 2, column 4; negative 1; end matrix")?; + expr, "the 3 by 4 augmented matrix; row 1; column 1; 1, column 2; 2, column 3; negative 1 separator, column 4; 3; \ + row 2; column 1; negative 3, column 2; 3, column 3; negative 1 separator, column 4; 2; \ + row 3; column 1; 2, column 2; 3, column 3; 2 separator, column 4; negative 1; end matrix")?; Ok(()) } From 9fb3060b62cbe74c87b4976bb7e139b8b047a514 Mon Sep 17 00:00:00 2001 From: mgros Date: Fri, 14 Aug 2026 17:04:06 +0200 Subject: [PATCH 2/7] Refactor `count_table_dims` to return `Result` type and update usage sites --- src/xpath_functions.rs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/xpath_functions.rs b/src/xpath_functions.rs index 6a4e1311..a17a93c1 100644 --- a/src/xpath_functions.rs +++ b/src/xpath_functions.rs @@ -1480,7 +1480,7 @@ impl CountTableDims { /// This function is relatively permissive. Non-`mtr` rows are /// ignored. The number of columns is determined only from the first /// row, if it exists. Within that row, non-`mtd` elements are ignored. - fn count_table_dims(mut self, e: Element<'_>) -> (usize, usize) { + fn count_table_dims<'d>(mut self, e: Element<'_>) -> Result<(Value<'d>, Value<'d>), Error> { for child in e.children() { let ChildOfElement::Element(row) = child else { continue @@ -1529,7 +1529,7 @@ impl CountTableDims { // columns, so we will not use them. let _extra_rows = self.extended_cells.keys().max().map(|k| k-1).unwrap_or(0); - (self.num_rows, self.num_cols) + Ok((Value::Number(self.num_rows as f64), Value::Number(self.num_cols as f64))) } fn evaluate<'d>(self, fn_name: &str, @@ -1540,8 +1540,7 @@ impl CountTableDims { let node = validate_one_node(element, fn_name)?; if let Node::Element(e) = node { if is_tag(e, "mtable") { - let (rows, columns) = self.count_table_dims(e); - return Ok((Value::Number(rows as f64), Value::Number(columns as f64))); + return self.count_table_dims(e); } else { return Err(Error::Other { what: format!("Input element was a <{}>, not an ", as_qname!(e.name()).local_part()) }); @@ -1580,8 +1579,10 @@ fn has_visible_column_line(table: Element, boundary: usize) -> bool { return false; } - let (_, column_count) = CountTableDims::new().count_table_dims(table); - if boundary >= column_count { + let Ok((_, Value::Number(column_count))) = CountTableDims::new().count_table_dims(table) else { + return false; + }; + if boundary as f64 >= column_count { return false; } @@ -1838,7 +1839,10 @@ mod tests { let package = parser::parse(mathml).map_err(|e| anyhow::anyhow!("failed to parse XML: {e}"))?; let math_elem = get_element(&package); let child = as_element(math_elem.children()[0]); - assert_eq!(CountTableDims::new().count_table_dims(child), dims); + assert_eq!( + CountTableDims::new().count_table_dims(child), + Ok((Value::Number(dims.0 as f64), Value::Number(dims.1 as f64))) + ); return Ok( () ); } From 5a8c76cb5bdd5c3d3573437af77db922032a66e8 Mon Sep 17 00:00:00 2001 From: mgros Date: Fri, 14 Aug 2026 17:04:52 +0200 Subject: [PATCH 3/7] undo whitespace change --- src/xpath_functions.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/xpath_functions.rs b/src/xpath_functions.rs index a17a93c1..63318936 100644 --- a/src/xpath_functions.rs +++ b/src/xpath_functions.rs @@ -1529,7 +1529,7 @@ impl CountTableDims { // columns, so we will not use them. let _extra_rows = self.extended_cells.keys().max().map(|k| k-1).unwrap_or(0); - Ok((Value::Number(self.num_rows as f64), Value::Number(self.num_cols as f64))) + Ok((Value::Number(self.num_rows as f64), Value::Number(self.num_cols as f64))) } fn evaluate<'d>(self, fn_name: &str, From a43117b1c66dd949bcb18c4ef69ffea172d2d6ad Mon Sep 17 00:00:00 2001 From: mgros Date: Fri, 14 Aug 2026 17:04:52 +0200 Subject: [PATCH 4/7] . --- src/xpath_functions.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/xpath_functions.rs b/src/xpath_functions.rs index a17a93c1..63318936 100644 --- a/src/xpath_functions.rs +++ b/src/xpath_functions.rs @@ -1529,7 +1529,7 @@ impl CountTableDims { // columns, so we will not use them. let _extra_rows = self.extended_cells.keys().max().map(|k| k-1).unwrap_or(0); - Ok((Value::Number(self.num_rows as f64), Value::Number(self.num_cols as f64))) + Ok((Value::Number(self.num_rows as f64), Value::Number(self.num_cols as f64))) } fn evaluate<'d>(self, fn_name: &str, From 15f1d5bafeef5f41ab7b6ba9fcdef8ff8dee47ee Mon Sep 17 00:00:00 2001 From: mgros Date: Fri, 14 Aug 2026 17:06:45 +0200 Subject: [PATCH 5/7] . --- src/xpath_functions.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/xpath_functions.rs b/src/xpath_functions.rs index 63318936..2412777b 100644 --- a/src/xpath_functions.rs +++ b/src/xpath_functions.rs @@ -1529,7 +1529,7 @@ impl CountTableDims { // columns, so we will not use them. let _extra_rows = self.extended_cells.keys().max().map(|k| k-1).unwrap_or(0); - Ok((Value::Number(self.num_rows as f64), Value::Number(self.num_cols as f64))) + Ok((Value::Number(self.num_rows as f64), Value::Number(self.num_cols as f64))) } fn evaluate<'d>(self, fn_name: &str, From 4a453e737416a4796c1fac15f96091cd3c755774 Mon Sep 17 00:00:00 2001 From: mgros Date: Fri, 14 Aug 2026 17:18:03 +0200 Subject: [PATCH 6/7] explain tests better in comments --- src/xpath_functions.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/xpath_functions.rs b/src/xpath_functions.rs index 2412777b..907117ad 100644 --- a/src/xpath_functions.rs +++ b/src/xpath_functions.rs @@ -1870,6 +1870,7 @@ mod tests { return Ok(()); } + /// Verifies visible column-line styles, repeated styles, and boundaries outside the table. #[test] fn visible_column_lines() -> Result<()> { return xpath_test(|| { @@ -1879,9 +1880,13 @@ mod tests { check_column_line("", 3, true)?; + // No column-line style is specified. check_column_line("", 1, false)?; + // The boundary is explicitly invisible. check_column_line("", 1, false)?; + // Only `solid` and `dashed` describe visible column lines. check_column_line("", 1, false)?; + // Boundary 2 is after the final column, not between two columns. check_column_line("", 2, false)?; return Ok(()); }); From 463b47000d536ee340b8b6eca64a165ae9385f68 Mon Sep 17 00:00:00 2001 From: mgros Date: Fri, 14 Aug 2026 17:24:24 +0200 Subject: [PATCH 7/7] explain tests better in comments --- src/xpath_functions.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/xpath_functions.rs b/src/xpath_functions.rs index 907117ad..cf0caaf7 100644 --- a/src/xpath_functions.rs +++ b/src/xpath_functions.rs @@ -1586,10 +1586,15 @@ fn has_visible_column_line(table: Element, boundary: usize) -> bool { return false; } - let line_style = table + return table .attribute_value("columnlines") - .and_then(|values| values.split_whitespace().take(boundary).last()); - return matches!(line_style, Some("solid" | "dashed")); + .map(|values| { + matches!( + values.split_whitespace().take(boundary).last(), + Some("solid" | "dashed") + ) + }) + .unwrap_or(false); } struct HasVisibleColumnLine;