CP6.11: craft-libs#489 review — dialect mapping after OCONV, bounded conversions

- An OCONV-written suggestion (colour’s) is mapped to the run's English
  spelling: the dialect lists are read with ', and the result keeps the
  dictionary's apostrophe (color’s). Straight and curly duplicates (color's,
  color’s) are one suggestion. Test: curly_suggestions_are_mapped_to_the_dialect.
- Conv::at looks up each prefix of the text (at most MAX_CONV_BYTES) in a
  hash map instead of scanning the pairs under its first character: 65,536
  pairs on one character cost 64 lookups per position, not a scan (Korean's
  OCONV has 588 pairs on one character, so a per-character cap would not
  fit). Test: a_huge_conversion_table_on_one_character_stays_fast.
- The reviewers' OCONV repros are a test (oconv_suggestions_match_hunspell):
  OCONV applies after a suggestion's case is set, case-sensitively, as the
  hunspell 1.7.5 CLI answers (KBT → KAT; Kbt → Kat under k → q).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FypgCX8LsYYE8fsj4byBm2
This commit is contained in:
Martin BallandClaude Opus 5.5 committed 2026-11-01 22:34:32 +00:00
1 parent 91d8bd335d
commit de593dff4d
3 files changed
+89 -15

No files matched your search

+15 -13
View File
@@ -334,34 +334,36 @@ pub struct AffixDict {
#[derive(Clone, Debug, Default)]
struct Conv {
pairs: Vec<(String, String)>,
/// First character of `from` → indices into `pairs`, longest `from` first (of equal ones, the
/// later pair first: it is the one that wins).
by_first: FxMap<char, Vec<u32>>,
/// `from` → index into `pairs` (of equal ones, the later pair: it is the one that wins).
by_from: FxMap<String, u32>,
/// Longest `from`, in bytes.
longest: usize,
}
impl Conv {
/// The table of `pairs` (checked by [`Rules::push_conv`]: non-empty `from`, bounded sizes).
fn new(pairs: Vec<(String, String)>) -> Conv {
let mut by_first: FxMap<char, Vec<u32>> = FxMap::default();
let mut by_from: FxMap<String, u32> = FxMap::default();
for (i, (from, _)) in pairs.iter().enumerate() {
if let (Some(c), Ok(i)) = (from.chars().next(), u32::try_from(i)) {
by_first.entry(c).or_default().push(i);
if let Ok(i) = u32::try_from(i) {
by_from.insert(from.clone(), i);
}
}
for list in by_first.values_mut() {
list.sort_by_key(|&i| (std::cmp::Reverse(pairs.get(i as usize).map_or(0, |p| p.0.len())), std::cmp::Reverse(i)));
}
Conv { pairs, by_first }
let longest = pairs.iter().map(|p| p.0.len()).max().unwrap_or(0);
Conv { pairs, by_from, longest }
}
fn is_empty(&self) -> bool {
self.pairs.is_empty()
}
/// The pair to apply at the start of `rest`: the longest `from` that starts it.
/// The pair to apply at the start of `rest`: the longest `from` that starts it. One hash
/// lookup per prefix of `rest` up to the longest `from` ([`MAX_CONV_BYTES`]), however many
/// pairs there are (craft-libs#489 review: 65,536 pairs on one first character).
fn at(&self, rest: &str) -> Option<&(String, String)> {
let c = rest.chars().next()?;
self.by_first.get(&c)?.iter().filter_map(|&i| self.pairs.get(i as usize)).find(|(from, _)| rest.starts_with(from.as_str()))
let ends = rest.char_indices().map(|(i, _)| i).skip(1).chain(std::iter::once(rest.len()));
let ends: Vec<usize> = ends.take_while(|&e| e <= self.longest).collect();
ends.iter().rev().find_map(|&e| self.by_from.get(rest.get(..e)?)).and_then(|&i| self.pairs.get(i as usize))
}
/// `w` converted: at each position the longest matching `from` is replaced by its `to`, and
+44
View File
@@ -519,6 +519,50 @@ fn scan(col: &mut Collector, chars: &[char], map: &[Vec<char>]) {
mod tests {
use super::*;
/// craft-libs#489 review repros: OCONV applies to a suggestion after its case is set, case
/// sensitively, as the hunspell 1.7.5 CLI answers on the same files (`KBT` → `KAT`, not
/// `K’T`), and every suggestion is right.
#[test]
fn oconv_suggestions_match_hunspell() {
type Case = (&'static str, &'static str, &'static [(&'static str, &'static str)]);
let cases: &[Case] = &[
("SET UTF-8\nTRY ab\nOCONV 1\nOCONV a ’\n", "1\nkat\n", &[("kbt", "k’t"), ("Kbt", "K’t"), ("KBT", "KAT"), ("kot", "k’t")]),
("SET UTF-8\nTRY ab\nOCONV 1\nOCONV k q\n", "1\nkat\n", &[("kbt", "qat"), ("Kbt", "Kat"), ("KBT", "KAT"), ("kot", "qat")]),
(
"SET UTF-8\nTRY esianrtolcdugmphbyfvkwz'\nICONV 1\nICONV ’ '\nOCONV 1\nOCONV ' ’\nSFX S Y 1\nSFX S 0 's .\n",
"2\ncolour/S\ndon't\n",
&[("dont", "don’t"), ("DONT", "DON’T"), ("colours", "colour’s")],
),
];
for (aff, dic, words) in cases {
let d = AffixDict::from_hunspell(aff, dic, &[]).unwrap();
for (word, want) in *words {
let got = suggest_in(&d, word, 8, &Tuning::DEFAULT);
assert!(got.iter().any(|g| g == want), "{word}: {got:?}, want {want}");
for g in &got {
assert!(d.check(g) || !aff.contains("ICONV"), "{word} → {g} is not right");
}
}
}
}
/// craft-libs#489 review: 65,536 ICONV pairs on one first character cost one hash lookup per
/// prefix, not a scan of the table, per position of a long word and per suggestion.
#[test]
fn a_huge_conversion_table_on_one_character_stays_fast() {
let mut aff = String::from("SET UTF-8\nTRY abc\nICONV 65536\n");
let a60 = "a".repeat(60);
for c in (0x100u32..).filter_map(char::from_u32).take(65_536) {
aff.push_str(&format!("ICONV {a60}{c} x\n"));
}
let d = AffixDict::from_hunspell(&aff, "1\nabc\n", &[]).unwrap();
let t = std::time::Instant::now();
assert!(!d.check(&"a".repeat(470)));
let _ = suggest_in(&d, "abd", 4, &Tuning::DEFAULT);
let _ = suggest_in(&d, &"a".repeat(40), 4, &Tuning::DEFAULT);
assert!(t.elapsed().as_secs() < 5, "{:?}", t.elapsed());
}
#[test]
fn an_extra_trailing_letter_is_dropped_before_a_short_word_split() {
let d = AffixDict::from_hunspell("", "2\npočítač\nz\n", &[]).unwrap();
+30 -2
View File
@@ -675,8 +675,10 @@ fn suggest_english(word: &str, max: usize, lang: ProofLang) -> Vec<String> {
}
let shared = english::suggest(word, max.saturating_add(2), lang);
for s in shared {
let s = english_preferred(&s.to_lowercase(), lang).map_or(s, |p| match_case(word, p));
if !out.contains(&s) {
let s = in_dialect(&s, word, lang);
// `color's` and `color’s` are one suggestion (a dictionary's OCONV writes the second).
let key = s.replace('’', "'");
if !out.iter().any(|o| o.replace('’', "'") == key) {
out.push(s);
}
}
@@ -684,6 +686,20 @@ fn suggest_english(word: &str, max: usize, lang: ProofLang) -> Vec<String> {
out
}
/// A suggestion `s` for `word` in `lang`'s spelling. Suggestions come as the dictionary's OCONV
/// writes them (`colour’s`); the dialect lists use `'`, so the mapping reads `'` and the result
/// keeps the dictionary's apostrophe (`color’s`).
fn in_dialect(s: &str, word: &str, lang: ProofLang) -> String {
let curly = s.contains('’');
match english_preferred(&s.to_lowercase().replace('’', "'"), lang) {
Some(p) => {
let p = match_case(word, p);
if curly { p.replace('\'', "’") } else { p }
}
None => s.to_string(),
}
}
/// Words in `text`: (byte start, byte end). Combining marks belong to the word they follow, so a
/// word typed decomposed (`tambie\u{301}n`, NFD) is one word, as its precomposed form is.
pub fn words(text: &str) -> Vec<(usize, usize)> {
@@ -1067,6 +1083,18 @@ mod tests {
use super::*;
/// craft-libs#489 review: a suggestion an OCONV table wrote with `’` is still mapped to this
/// dialect's spelling (`colour’s` → `color’s` in American English), and keeps its apostrophe.
#[test]
fn curly_suggestions_are_mapped_to_the_dialect() {
if english_preferred("colour's", ProofLang::En).is_none() {
return; // no dialect lists without CRAFT_I18N_DIR
}
assert_eq!(in_dialect("colour’s", "colours", ProofLang::En), "color’s");
assert_eq!(in_dialect("colour's", "colours", ProofLang::En), "color's");
assert_eq!(in_dialect("colour’s", "colours", ProofLang::EnGb), "colour’s");
}
/// The non-English test dictionaries every dictionaries-folder test puts in its folder, so
/// tests running meanwhile see the same words whichever of those folders is set.
fn write_test_dictionaries(dir: &std::path::Path) {