From 03d8da14c938c8eb5939e9903ec015b2ef727024 Mon Sep 17 00:00:00 2001 From: iga566 Date: Sun, 6 Sep 2026 22:16:35 +0500 Subject: [PATCH] engine: force-by-selection follows the text, not current() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit converted() trusted layout_switcher.current() for the source layout. If the layout moved on since the word was typed — exactly what the next word does on auto-correction — the guess was wrong, transliterate_to's own guard refused it (no letter of that layout in the text), and the force-switch silently did nothing. Both directions are tried now, current first, falling back to the swap when the text does not belong to it. transliterate_to already refuses a source layout the text carries no letter of, so the wrong direction rejects itself and nothing is forced through blind. Measured on macOS 26 and Windows Server 2025 with the fix: repeated presses on one selection convert it back and forth, each from the first press, whichever layout is active at the time. Co-Authored-By: Claude Fable 5.1 --- .../src/engine/switcher/correction.rs | 36 +++++-- crates/poltertype-core/src/engine/tests.rs | 95 +++++++++++++++++++ 2 files changed, 125 insertions(+), 6 deletions(-) diff --git a/crates/poltertype-core/src/engine/switcher/correction.rs b/crates/poltertype-core/src/engine/switcher/correction.rs index 8b3a978..646c339 100644 --- a/crates/poltertype-core/src/engine/switcher/correction.rs +++ b/crates/poltertype-core/src/engine/switcher/correction.rs @@ -1054,13 +1054,37 @@ impl SwitcherEngine { /// The selection re-rendered under another layout, or `None` when /// it is not wrong-layout text at all. - fn converted(&self, text: &str) -> Option<(String, LayoutId, LayoutId)> { - let from = self.layout_switcher.current().ok()?; - let to = self.next_layout_after(&from)?; - let source = self.layouts.get(&from)?; - let target = self.layouts.get(&to)?; + /// + /// Which of the current layout and its neighbour is the *source* + /// is not something `current()` can answer on its own: the text + /// may have been typed before the very next word moved the layout + /// on, and then a source guess keyed off `current()` names the + /// layout the caret is in now, not the one the selection was + /// typed under. So both directions are tried, current-first, and + /// `transliterate_to`'s own guard — no letter of that layout in + /// the text — is what rejects the one that does not fit; neither + /// is forced through blind. + // `pub(in crate::engine)`, not `pub(super)`: the regression test + // for the direction bug below builds a bare engine directly in + // `engine::tests` rather than driving the run loop, and that + // module sits one level above `switcher`. + pub(in crate::engine) fn converted(&self, text: &str) -> Option<(String, LayoutId, LayoutId)> { + let current = self.layout_switcher.current().ok()?; + let other = self.next_layout_after(¤t)?; + self.transliterated(text, ¤t, &other) + .or_else(|| self.transliterated(text, &other, ¤t)) + } + + fn transliterated( + &self, + text: &str, + from: &LayoutId, + to: &LayoutId, + ) -> Option<(String, LayoutId, LayoutId)> { + let source = self.layouts.get(from)?; + let target = self.layouts.get(to)?; let converted = source.transliterate_to(text, target)?; - Some((converted, from, to)) + Some((converted, from.clone(), to.clone())) } /// Put back what the user had, best effort. A failure here is worth diff --git a/crates/poltertype-core/src/engine/tests.rs b/crates/poltertype-core/src/engine/tests.rs index 202abbb..0b09c76 100644 --- a/crates/poltertype-core/src/engine/tests.rs +++ b/crates/poltertype-core/src/engine/tests.rs @@ -938,6 +938,101 @@ mod engine_integration_tests { ); } + /// Builds a bare `SwitcherEngine` for calling a private method + /// directly — no thread, no channels driven, unlike `Harness`, + /// which moves the engine into its runner thread and never gives + /// it back. Only what `converted()` touches needs to be real. + fn bare_engine(current: &str, active: &[&str]) -> (SwitcherEngine, Arc) { + let active_ids: Vec = active.iter().map(|s| LayoutId::from(*s)).collect(); + let layouts = Arc::new( + LayoutDb::load(crate::layouts::LoadOptions { + active_filter: Some(&active_ids), + ..Default::default() + }) + .expect("bundled layouts load"), + ); + let settings = Arc::new(SettingsStore::for_tests( + crate::settings::Settings::default(), + )); + let switcher = Arc::new(MockSwitcher::new(current, active)); + let (audio, _audio_rx) = crate::audio::AudioPlayer::for_tests(); + let (out_tx, _out_rx) = crossbeam_channel::unbounded(); + let engine = SwitcherEngine::new(EngineDeps { + settings, + layouts: Arc::clone(&layouts), + detectors: Vec::new(), + layout_switcher: switcher as Arc, + key_emitter: Arc::new(MockEmitter::default()) as Arc, + clipboard: None, + key_gate: poltertype_input::KeyGate::disabled(), + focus_tracker: Arc::new(NoopFocusTracker), + audio: Arc::new(audio), + out_tx, + suggester: None, + }); + (engine, layouts) + } + + /// Regression: force-by-selection trusted `current()` for the + /// *source* layout, not the text. If the layout had moved on since + /// the word was typed — exactly what the very next word does when + /// it triggers auto-correction — `converted()` tried + /// transliterating from the wrong layout, `transliterate_to`'s own + /// guard refused (no letter of that layout in the text), and the + /// whole gesture read as "nothing was selected". Plan and test + /// words are from `plans/backlog.md` (P2, keyboard-switcher track). + #[test] + fn selection_conversion_direction_follows_the_text_not_the_current_layout() { + let (engine, layouts) = bare_engine("uk-UA", &["en-US", "uk-UA"]); + let en = layouts.get(&LayoutId::from("en-US")).expect("en-US loaded"); + let uk = layouts.get(&LayoutId::from("uk-UA")).expect("uk-UA loaded"); + let expected = en + .transliterate_to("ghbdsn", uk) + .expect("en-US -> uk-UA must convert `ghbdsn`"); + + // `current()` says uk-UA, but "ghbdsn" (`привіт` mistyped) was + // typed under en-US — the old code trusted `current()` and + // the force-switch silently did nothing. + assert_eq!( + engine.converted("ghbdsn"), + Some((expected, LayoutId::from("en-US"), LayoutId::from("uk-UA"))), + "must detect en-US as the source from the text, not uk-UA from current()" + ); + } + + /// Mirror of the regression above: `current()` already names the + /// right source, so the first try must still succeed directly — + /// the fallback exists for the case above, not to replace the + /// ordinary one. + #[test] + fn selection_conversion_direction_still_works_when_current_is_right() { + let (engine, layouts) = bare_engine("en-US", &["en-US", "uk-UA"]); + let en = layouts.get(&LayoutId::from("en-US")).expect("en-US loaded"); + let uk = layouts.get(&LayoutId::from("uk-UA")).expect("uk-UA loaded"); + let expected = en + .transliterate_to("ghbdsn", uk) + .expect("en-US -> uk-UA must convert `ghbdsn`"); + + assert_eq!( + engine.converted("ghbdsn"), + Some((expected, LayoutId::from("en-US"), LayoutId::from("uk-UA"))), + "a source that `current()` already gets right must not need the fallback" + ); + } + + /// Text that belongs to neither active layout must not be forced + /// through either direction — `transliterate_to`'s guard has to + /// win both tries, not just the first. + #[test] + fn selection_conversion_direction_gives_up_on_text_neither_layout_typed() { + let (engine, _layouts) = bare_engine("uk-UA", &["en-US", "uk-UA"]); + assert_eq!( + engine.converted("12345"), + None, + "digits belong to no layout's alphabet — nothing to convert" + ); + } + /// A word typed under a latched Caps Lock has to go back out on the /// Shift states the user's fingers actually had. ///