diff --git a/crates/ghostty-apprt-gtk/examples/demo.rs b/crates/ghostty-apprt-gtk/examples/demo.rs index 5e453f5..4a067f8 100644 --- a/crates/ghostty-apprt-gtk/examples/demo.rs +++ b/crates/ghostty-apprt-gtk/examples/demo.rs @@ -149,7 +149,7 @@ impl CoreSurface for DemoSurface { } } - fn preedit_callback(&self, _text: Option<&str>) -> Result<(), CoreError> { + fn preedit_callback(&self, _text: Option<&[u8]>) -> Result<(), CoreError> { Ok(()) } fn focus_callback(&self, focused: bool) -> Result<(), CoreError> { diff --git a/crates/ghostty-apprt-gtk/src/class/surface.rs b/crates/ghostty-apprt-gtk/src/class/surface.rs index da26c85..f15cbf7 100644 --- a/crates/ghostty-apprt-gtk/src/class/surface.rs +++ b/crates/ghostty-apprt-gtk/src/class/surface.rs @@ -1306,7 +1306,7 @@ impl Surface { mods, consumed_mods, composing: imp.im_composing.get(), - utf8: &utf8, + utf8: utf8.as_bytes(), unshifted_codepoint: keyval_unicode_unshifted, }) { Ok(e) => e, @@ -3673,7 +3673,7 @@ mod imp { let (text, _, _) = ctx.preedit_string(); // Update our preedit state in Ghostty core - if let Err(err) = surface.preedit_callback(Some(&text)) { + if let Err(err) = surface.preedit_callback(Some(text.as_bytes())) { log::warn!("error in preedit callback err={err}"); } } @@ -3755,7 +3755,7 @@ mod imp { mods: Mods::default(), consumed_mods: Mods::default(), composing: false, - utf8: str, + utf8: str.as_bytes(), ..KeyEvent::default() }) { log::warn!("error in key callback err={err}"); diff --git a/crates/ghostty-apprt/src/runtime_api.rs b/crates/ghostty-apprt/src/runtime_api.rs index 4cba831..faf5d6f 100644 --- a/crates/ghostty-apprt/src/runtime_api.rs +++ b/crates/ghostty-apprt/src/runtime_api.rs @@ -164,7 +164,7 @@ pub trait CoreSurface { // ---- input ------------------------------------------------------- fn key_callback(&self, event: KeyEvent<'_>) -> Result; /// `preeditCallback(null)` clears. - fn preedit_callback(&self, text: Option<&str>) -> Result<(), CoreError>; + fn preedit_callback(&self, text: Option<&[u8]>) -> Result<(), CoreError>; fn focus_callback(&self, focused: bool) -> Result<(), CoreError>; /// Returns true if the click was consumed. fn mouse_button_callback( diff --git a/crates/ghostty-core/src/surface/keyboard.rs b/crates/ghostty-core/src/surface/keyboard.rs index d6101bd..a8b67bc 100644 --- a/crates/ghostty-core/src/surface/keyboard.rs +++ b/crates/ghostty-core/src/surface/keyboard.rs @@ -92,7 +92,7 @@ pub struct OwnedKeyEvent { pub mods: Mods, pub consumed_mods: Mods, pub composing: bool, - pub utf8: String, + pub utf8: Vec, pub unshifted_codepoint: u32, } @@ -112,7 +112,7 @@ impl OwnedKeyEvent { mods: e.mods, consumed_mods: e.consumed_mods, composing: e.composing, - utf8: e.utf8.to_owned(), + utf8: e.utf8.to_vec(), unshifted_codepoint: e.unshifted_codepoint, } } @@ -1067,7 +1067,7 @@ mod tests { ctrl: true, ..Mods::default() }, - utf8: "a", + utf8: "a".as_bytes(), unshifted_codepoint: 'a' as u32, ..KeyEvent::default() }; diff --git a/crates/ghostty-core/src/surface/mod.rs b/crates/ghostty-core/src/surface/mod.rs index 785cc8e..37578f3 100644 --- a/crates/ghostty-core/src/surface/mod.rs +++ b/crates/ghostty-core/src/surface/mod.rs @@ -1679,7 +1679,7 @@ impl Surface { /// The core surface will NOT reset the preedit state on charCallback or /// keyCallback and we rely completely on the apprt implementation to /// track the preedit state correctly. - pub fn preedit_callback(&self, preedit: Option<&str>) -> Result<(), CoreError> { + pub fn preedit_callback(&self, preedit: Option<&[u8]>) -> Result<(), CoreError> { let cfg = self.cfg(); let mut st = self.lock_state(); @@ -1704,7 +1704,16 @@ impl Surface { return Ok(()); }; - // We convert the UTF-8 text to codepoints. + // We convert the UTF-8 text to codepoints. Like Zig's + // `Utf8View.init(text)`, invalid UTF-8 is an error: the prior preedit + // is already cleared and marked dirty, and no new preedit is set. + let text = match std::str::from_utf8(text) { + Ok(t) => t, + Err(e) => { + drop(st); + return Err(Box::new(e)); + } + }; let mut codepoints: Vec = Vec::new(); for cp in text.chars() { let width = usize::from(ghostty_unicode::table().get(u32::from(cp)).width); @@ -1993,7 +2002,7 @@ impl CoreSurface for Surface { Surface::key_callback(self, event) } - fn preedit_callback(&self, text: Option<&str>) -> Result<(), CoreError> { + fn preedit_callback(&self, text: Option<&[u8]>) -> Result<(), CoreError> { let _crash = self.crash_guard(); Surface::preedit_callback(self, text) } diff --git a/crates/ghostty-core/tests/common/mod.rs b/crates/ghostty-core/tests/common/mod.rs index de42da7..57db6cb 100644 --- a/crates/ghostty-core/tests/common/mod.rs +++ b/crates/ghostty-core/tests/common/mod.rs @@ -486,7 +486,7 @@ impl Harness { Mods::default() }, composing: false, - utf8, + utf8: utf8.as_bytes(), unshifted_codepoint: unshifted, }); // GTK reports the same text/codepoint on release. @@ -496,7 +496,7 @@ impl Harness { mods, consumed_mods: Mods::default(), composing: false, - utf8, + utf8: utf8.as_bytes(), unshifted_codepoint: unshifted, }); eff diff --git a/crates/ghostty-core/tests/e2e_ime.rs b/crates/ghostty-core/tests/e2e_ime.rs index bf867b2..ece8331 100644 --- a/crates/ghostty-core/tests/e2e_ime.rs +++ b/crates/ghostty-core/tests/e2e_ime.rs @@ -36,7 +36,7 @@ fn preedit_state_and_ime_point() { let h = harness!(Harness::recorder("", &[])); let before = h.surface.ime_point(); h.surface - .preedit_callback(Some("\u{3042}\u{3044}")) + .preedit_callback(Some("\u{3042}\u{3044}".as_bytes())) .unwrap(); { let st = h.surface.lock_state(); @@ -57,6 +57,20 @@ fn preedit_state_and_ime_point() { assert_eq!(h.recorded(), Vec::::new()); } +#[test] +fn preedit_with_invalid_utf8_is_an_error_and_clears_prior() { + let h = harness!(Harness::recorder("", &[])); + // Like Zig's preeditCallback (`try Utf8View.init(text)`): invalid UTF-8 + // returns an error after the prior preedit has been cleared, and no new + // preedit is set. Nothing is written to the pty. + h.surface.preedit_callback(Some(b"ab")).unwrap(); + assert!(h.surface.lock_state().preedit.is_some()); + assert!(h.surface.preedit_callback(Some(b"a\xffb\xc3")).is_err()); + assert!(h.surface.lock_state().preedit.is_none()); + h.settle(50); + assert_eq!(h.recorded(), Vec::::new()); +} + #[test] fn preedit_is_rendered_and_cleared() { let h = harness!(Harness::recorder("", &[])); @@ -71,7 +85,7 @@ fn preedit_is_rendered_and_cleared() { }); b }; - h.surface.preedit_callback(Some("abcd")).unwrap(); + h.surface.preedit_callback(Some("abcd".as_bytes())).unwrap(); h.wait("preedit visible", |h| { let r = region(h, cols.clone(), 0); !r.is_empty() && r != baseline @@ -89,7 +103,7 @@ fn commit_sends_text_and_composing_keys_do_not() { h.key_event(KeyEvent { key: Key::KeyK, composing: true, - utf8: "k", + utf8: "k".as_bytes(), unshifted_codepoint: 'k' as u32, ..KeyEvent::default() }); @@ -98,12 +112,12 @@ fn commit_sends_text_and_composing_keys_do_not() { // Commit: preedit ends, committed text is sent with no key. h.surface - .preedit_callback(Some("\u{3053}\u{3093}")) + .preedit_callback(Some("\u{3053}\u{3093}".as_bytes())) .unwrap(); h.surface.preedit_callback(None).unwrap(); h.key_event(KeyEvent { key: Key::Unidentified, - utf8: "\u{3053}\u{3093}\u{306b}\u{3061}\u{306f}", + utf8: "\u{3053}\u{3093}\u{306b}\u{3061}\u{306f}".as_bytes(), ..KeyEvent::default() }); h.expect_input("\u{3053}\u{3093}\u{306b}\u{3061}\u{306f}".as_bytes()); @@ -115,7 +129,7 @@ fn enter_with_committed_text_sends_text_not_cr() { let h = harness!(Harness::recorder("", &[])); h.key_event(KeyEvent { key: Key::Enter, - utf8: "\u{6f22}", + utf8: "\u{6f22}".as_bytes(), ..KeyEvent::default() }); h.expect_input("\u{6f22}".as_bytes()); diff --git a/crates/ghostty-core/tests/e2e_keys.rs b/crates/ghostty-core/tests/e2e_keys.rs index a6bd25f..88a239a 100644 --- a/crates/ghostty-core/tests/e2e_keys.rs +++ b/crates/ghostty-core/tests/e2e_keys.rs @@ -250,7 +250,7 @@ fn key_release_without_press_is_ignored_in_legacy() { h.key_event(ghostty_input::KeyEvent { action: ghostty_input::key::Action::Release, key: Key::KeyA, - utf8: "a", + utf8: "a".as_bytes(), ..Default::default() }); h.type_text("b"); diff --git a/crates/ghostty-input/src/binding/set.rs b/crates/ghostty-input/src/binding/set.rs index 94dae21..1a9bc77 100644 --- a/crates/ghostty-input/src/binding/set.rs +++ b/crates/ghostty-input/src/binding/set.rs @@ -574,8 +574,9 @@ impl Set { // If our UTF-8 text is exactly one codepoint, we try to match that. if !event.utf8.is_empty() { - let mut it = event.utf8.chars(); - // No codepoints or multiple codepoints drops to invalid format + // Invalid UTF-8, no codepoints or multiple codepoints drops to + // invalid format. + let mut it = crate::key_encode::utf8_chars(event.utf8).unwrap_or_else(|| "".chars()); if let (Some(cp), None) = (it.next(), it.next()) { trigger.key = TriggerKey::Unicode(cp as u32); if let Some(v) = self.get_entry(trigger) { diff --git a/crates/ghostty-input/src/binding/tests.rs b/crates/ghostty-input/src/binding/tests.rs index 3d232c6..b808eb1 100644 --- a/crates/ghostty-input/src/binding/tests.rs +++ b/crates/ghostty-input/src/binding/tests.rs @@ -1097,7 +1097,7 @@ fn ev(key: Key, mods: Mods, utf8: &str, unshifted: u32) -> KeyEvent<'_> { KeyEvent { key, mods, - utf8, + utf8: utf8.as_bytes(), unshifted_codepoint: unshifted, ..KeyEvent::default() } diff --git a/crates/ghostty-input/src/key.rs b/crates/ghostty-input/src/key.rs index f644ed7..0d86ce0 100644 --- a/crates/ghostty-input/src/key.rs +++ b/crates/ghostty-input/src/key.rs @@ -43,7 +43,7 @@ pub struct KeyEvent<'a> { /// This will be an empty string if there is no text generated. /// If composing is true and this is non-empty, this is preedit /// text. - pub utf8: &'a str, + pub utf8: &'a [u8], /// The codepoint for this key when it is unshifted. For example, /// shift+a is "A" in UTF-8 but unshifted would provide 'a'. @@ -58,7 +58,7 @@ impl Default for KeyEvent<'_> { mods: Mods::default(), consumed_mods: Mods::default(), composing: false, - utf8: "", + utf8: b"", unshifted_codepoint: 0, } } diff --git a/crates/ghostty-input/src/key_encode.rs b/crates/ghostty-input/src/key_encode.rs index 29fd428..914d40e 100644 --- a/crates/ghostty-input/src/key_encode.rs +++ b/crates/ghostty-input/src/key_encode.rs @@ -147,7 +147,7 @@ fn kitty(writer: &mut W, event: &KeyEvent, opts: &Options) -> io::Resu if event.key == Key::Backspace { return Ok(()); } - return writer.write_all(event.utf8.as_bytes()); + return writer.write_all(event.utf8); } } @@ -180,8 +180,12 @@ fn kitty(writer: &mut W, event: &KeyEvent, opts: &Options) -> io::Resu // We only do this for printable characters. We should // inspect the real unicode codepoint properties here but // the real world issue is usually control characters. - if !event.utf8.chars().any(|c| is_control(u32::from(c))) { - return writer.write_all(event.utf8.as_bytes()); + // Invalid UTF-8 falls back to encoding the key press as if it + // didn't produce text (the bytes are never sent as plain text). + if let Some(mut chars) = utf8_chars(event.utf8) + && !chars.any(|c| is_control(u32::from(c))) + { + return writer.write_all(event.utf8); } } } @@ -192,7 +196,7 @@ fn kitty(writer: &mut W, event: &KeyEvent, opts: &Options) -> io::Resu // (e.g. composed/IME text), so send it as-is so programs can // still receive it. if !event.utf8.is_empty() { - return writer.write_all(event.utf8.as_bytes()); + return writer.write_all(event.utf8); } return Ok(()); }; @@ -209,7 +213,7 @@ fn kitty(writer: &mut W, event: &KeyEvent, opts: &Options) -> io::Resu mods: KittyMods::from_input(event.action, event.key, all_mods), event: KittyEvent::None, alternates: [None, None], - text: "", + text: b"", }; if opts.kitty_flags.report_events { @@ -230,7 +234,10 @@ fn kitty(writer: &mut W, event: &KeyEvent, opts: &Options) -> io::Resu break 'alternates; } - let mut it = event.utf8.chars(); + // Assume invalid UTF-8 means no UTF-8. + let Some(mut it) = utf8_chars(event.utf8) else { + break 'alternates; + }; // If we have a codepoint in our UTF-8 sequence, then we can // report the shifted version. @@ -376,7 +383,8 @@ fn legacy(writer: &mut W, event: &KeyEvent, opts: &Options) -> io::Res // for any char with a modifier. Ctrl sequences like Ctrl+a // are already handled above. if opts.modify_other_keys_state_2 { - let mut it = utf8.chars(); + // Assume invalid UTF-8 means we have no UTF-8. + let mut it = utf8_chars(utf8).unwrap_or_else(|| "".chars()); // We only do this if we have a single codepoint. There shouldn't // ever be a multi-codepoint sequence that triggers this. if let (Some(codepoint), None) = (it.next().map(u32::from), it.clone().next()) { @@ -449,7 +457,9 @@ fn legacy(writer: &mut W, event: &KeyEvent, opts: &Options) -> io::Res // Get our codepoint. If we have more than one codepoint this // can't be valid CSIu. - let mut it = event.utf8.chars(); + let Some(mut it) = utf8_chars(event.utf8) else { + break 'csiu; + }; let Some(first) = it.next() else { break 'csiu; }; @@ -501,7 +511,7 @@ fn legacy(writer: &mut W, event: &KeyEvent, opts: &Options) -> io::Res return Ok(()); } - writer.write_all(utf8.as_bytes()) + writer.write_all(utf8) } fn legacy_alt_prefix( @@ -536,7 +546,7 @@ fn legacy_alt_prefix( } // Otherwise, we require utf8 to already have the byte represented. - let utf8 = event.utf8.as_bytes(); + let utf8 = event.utf8; if utf8.len() == 1 { return Some(utf8[0]); } @@ -654,7 +664,12 @@ fn pc_style_function_key( /// This will return None if the key event should not be converted /// into a C0 byte. There are many cases for this and you should read /// the source code to understand them. -fn ctrl_seq(logical_key: Key, utf8: &str, unshifted_codepoint: u32, mods: key::Mods) -> Option { +fn ctrl_seq( + logical_key: Key, + utf8: &[u8], + unshifted_codepoint: u32, + mods: key::Mods, +) -> Option { let ctrl_only = key::Mods { ctrl: true, ..key::Mods::default() @@ -679,7 +694,7 @@ fn ctrl_seq(logical_key: Key, utf8: &str, unshifted_codepoint: u32, mods: key::M // If we have exactly one UTF8 byte, we assume that is the // character we want to convert to a C0 byte. if utf8.len() == 1 { - break 'ch utf8.as_bytes()[0]; + break 'ch utf8[0]; } // If we have a logical key that maps to a single byte @@ -813,8 +828,14 @@ fn is_control(cp: u32) -> bool { /// Returns true if this string is comprised of a single /// control character. This returns false for multi-byte strings. -fn is_control_utf8(s: &str) -> bool { - s.len() == 1 && is_control(u32::from(s.as_bytes()[0])) +fn is_control_utf8(s: &[u8]) -> bool { + s.len() == 1 && is_control(u32::from(s[0])) +} + +/// Strictly decode `bytes` as UTF-8 (Zig: `std.unicode.Utf8View.init`), +/// returning `None` for invalid UTF-8. +pub(crate) fn utf8_chars(bytes: &[u8]) -> Option> { + std::str::from_utf8(bytes).ok().map(str::chars) } /// This is the bitmask for fixterm CSI u modifiers. @@ -926,7 +947,7 @@ struct KittySequence<'a> { mods: KittyMods, event: KittyEvent, alternates: [Option; 2], - text: &'a str, + text: &'a [u8], } impl KittySequence<'_> { @@ -964,9 +985,10 @@ impl KittySequence<'_> { } // Text section - if !self.text.is_empty() { + // Assume invalid UTF-8 means we have no text. + if let Some(chars) = utf8_chars(self.text).filter(|_| !self.text.is_empty()) { let mut count = 0; - for c in self.text.chars() { + for c in chars { let cp = u32::from(c); // If the codepoint is non-printable ASCII character, skip. if is_control(cp) { @@ -1022,7 +1044,7 @@ mod tests { &KeyEvent { key: Key::KeyA, mods: Mods { ..Mods::default() }, - utf8: "abcd", + utf8: "abcd".as_bytes(), ..KeyEvent::default() }, &Options { @@ -1048,7 +1070,7 @@ mod tests { key: Key::KeyA, action: key::Action::Repeat, mods: Mods { ..Mods::default() }, - utf8: "a", + utf8: "a".as_bytes(), ..KeyEvent::default() }, &Options { @@ -1073,7 +1095,7 @@ mod tests { &KeyEvent { key: Key::Enter, mods: Mods { ..Mods::default() }, - utf8: "", + utf8: "".as_bytes(), ..KeyEvent::default() }, &Options { @@ -1094,7 +1116,7 @@ mod tests { &KeyEvent { key: Key::Backspace, mods: Mods { ..Mods::default() }, - utf8: "", + utf8: "".as_bytes(), ..KeyEvent::default() }, &Options { @@ -1115,7 +1137,7 @@ mod tests { &KeyEvent { key: Key::Tab, mods: Mods { ..Mods::default() }, - utf8: "", + utf8: "".as_bytes(), ..KeyEvent::default() }, &Options { @@ -1137,7 +1159,7 @@ mod tests { action: key::Action::Release, key: Key::Enter, mods: Mods { ..Mods::default() }, - utf8: "", + utf8: "".as_bytes(), ..KeyEvent::default() }, &Options { @@ -1160,7 +1182,7 @@ mod tests { action: key::Action::Release, key: Key::Backspace, mods: Mods { ..Mods::default() }, - utf8: "", + utf8: "".as_bytes(), ..KeyEvent::default() }, &Options { @@ -1183,7 +1205,7 @@ mod tests { action: key::Action::Release, key: Key::Tab, mods: Mods { ..Mods::default() }, - utf8: "", + utf8: "".as_bytes(), ..KeyEvent::default() }, &Options { @@ -1206,7 +1228,7 @@ mod tests { action: key::Action::Release, key: Key::Enter, mods: Mods { ..Mods::default() }, - utf8: "", + utf8: "".as_bytes(), ..KeyEvent::default() }, &Options { @@ -1230,7 +1252,7 @@ mod tests { action: key::Action::Release, key: Key::Backspace, mods: Mods { ..Mods::default() }, - utf8: "", + utf8: "".as_bytes(), ..KeyEvent::default() }, &Options { @@ -1254,7 +1276,7 @@ mod tests { action: key::Action::Release, key: Key::Tab, mods: Mods { ..Mods::default() }, - utf8: "", + utf8: "".as_bytes(), ..KeyEvent::default() }, &Options { @@ -1284,7 +1306,7 @@ mod tests { shift: true, ..Mods::default() }, - utf8: "", + utf8: "".as_bytes(), ..KeyEvent::default() }, &Options { @@ -1312,7 +1334,7 @@ mod tests { shift: true, ..Mods::default() }, - utf8: "", + utf8: "".as_bytes(), ..KeyEvent::default() }, &Options { @@ -1340,7 +1362,7 @@ mod tests { shift: true, ..Mods::default() }, - utf8: "", + utf8: "".as_bytes(), ..KeyEvent::default() }, &Options { @@ -1365,7 +1387,7 @@ mod tests { &KeyEvent { key: Key::Enter, mods: Mods { ..Mods::default() }, - utf8: "", + utf8: "".as_bytes(), ..KeyEvent::default() }, &Options { @@ -1396,7 +1418,7 @@ mod tests { ctrl: true, ..Mods::default() }, - utf8: "", + utf8: "".as_bytes(), ..KeyEvent::default() }, &Options { @@ -1428,7 +1450,7 @@ mod tests { ctrl: true, ..Mods::default() }, - utf8: "", + utf8: "".as_bytes(), ..KeyEvent::default() }, &Options { @@ -1456,7 +1478,7 @@ mod tests { &KeyEvent { key: Key::Delete, mods: Mods { ..Mods::default() }, - utf8: "\u{7f}", + utf8: "\u{7f}".as_bytes(), ..KeyEvent::default() }, &Options { @@ -1538,7 +1560,7 @@ mod tests { &KeyEvent { key: Key::Unidentified, mods: Mods { ..Mods::default() }, - utf8: "\u{fb}", + utf8: "\u{fb}".as_bytes(), ..KeyEvent::default() }, &Options { @@ -1569,7 +1591,7 @@ mod tests { shift: true, ..Mods::default() }, - utf8: "A", + utf8: "A".as_bytes(), unshifted_codepoint: 97, ..KeyEvent::default() }, @@ -1599,7 +1621,7 @@ mod tests { shift: true, ..Mods::default() }, - utf8: "A", + utf8: "A".as_bytes(), unshifted_codepoint: 65, ..KeyEvent::default() }, @@ -1629,7 +1651,7 @@ mod tests { caps_lock: true, ..Mods::default() }, - utf8: "J", + utf8: "J".as_bytes(), unshifted_codepoint: 106, ..KeyEvent::default() }, @@ -1661,7 +1683,7 @@ mod tests { shift: true, ..Mods::default() }, - utf8: ":", + utf8: ":".as_bytes(), unshifted_codepoint: u32::from(';'), ..KeyEvent::default() }, @@ -1690,7 +1712,7 @@ mod tests { &KeyEvent { key: Key::Semicolon, mods: Mods { ..Mods::default() }, - utf8: "\u{447}", + utf8: "\u{447}".as_bytes(), unshifted_codepoint: 1095, ..KeyEvent::default() }, @@ -1722,7 +1744,7 @@ mod tests { shift: true, ..Mods::default() }, - utf8: "\u{427}", + utf8: "\u{427}".as_bytes(), unshifted_codepoint: 1095, ..KeyEvent::default() }, @@ -1754,7 +1776,7 @@ mod tests { caps_lock: true, ..Mods::default() }, - utf8: "\u{427}", + utf8: "\u{427}".as_bytes(), unshifted_codepoint: 1095, ..KeyEvent::default() }, @@ -1787,7 +1809,7 @@ mod tests { ctrl: true, ..Mods::default() }, - utf8: "", + utf8: "".as_bytes(), unshifted_codepoint: 337, ..KeyEvent::default() }, @@ -1816,7 +1838,7 @@ mod tests { &KeyEvent { key: Key::ArrowUp, mods: Mods { ..Mods::default() }, - utf8: "\u{1e}", + utf8: "\u{1e}".as_bytes(), ..KeyEvent::default() }, &Options { @@ -1844,7 +1866,7 @@ mod tests { shift: true, ..Mods::default() }, - utf8: "", + utf8: "".as_bytes(), ..KeyEvent::default() }, &Options { @@ -1870,7 +1892,7 @@ mod tests { &KeyEvent { key: Key::ShiftLeft, mods: Mods { ..Mods::default() }, - utf8: "", + utf8: "".as_bytes(), ..KeyEvent::default() }, &Options { @@ -1896,7 +1918,7 @@ mod tests { &KeyEvent { key: Key::ShiftLeft, mods: Mods { ..Mods::default() }, - utf8: "", + utf8: "".as_bytes(), ..KeyEvent::default() }, &Options { @@ -1925,7 +1947,7 @@ mod tests { ctrl: true, ..Mods::default() }, - utf8: "j", + utf8: "j".as_bytes(), unshifted_codepoint: 106, ..KeyEvent::default() }, @@ -1957,7 +1979,7 @@ mod tests { shift: true, ..Mods::default() }, - utf8: "J", + utf8: "J".as_bytes(), unshifted_codepoint: 106, ..KeyEvent::default() }, @@ -1990,7 +2012,7 @@ mod tests { shift: true, ..Mods::default() }, - utf8: "J", + utf8: "J".as_bytes(), unshifted_codepoint: 106, ..KeyEvent::default() }, @@ -2019,7 +2041,7 @@ mod tests { &KeyEvent { key: Key::Delete, mods: Mods { ..Mods::default() }, - utf8: "\u{7f}", + utf8: "\u{7f}".as_bytes(), ..KeyEvent::default() }, &Options { @@ -2045,7 +2067,7 @@ mod tests { &mut w, &KeyEvent { key: Key::Enter, - utf8: "A", + utf8: "A".as_bytes(), unshifted_codepoint: 0x0D, ..KeyEvent::default() }, @@ -2073,7 +2095,7 @@ mod tests { &KeyEvent { key: Key::Numpad1, mods: Mods { ..Mods::default() }, - utf8: "1", + utf8: "1".as_bytes(), ..KeyEvent::default() }, &Options { @@ -2100,7 +2122,7 @@ mod tests { &mut w, &KeyEvent { key: Key::Backspace, - utf8: "A", + utf8: "A".as_bytes(), unshifted_codepoint: 0x0D, ..KeyEvent::default() }, @@ -2128,7 +2150,7 @@ mod tests { &mut w, &KeyEvent { key: Key::Backspace, - utf8: "A", + utf8: "A".as_bytes(), unshifted_codepoint: 0x0D, ..KeyEvent::default() }, @@ -2149,7 +2171,7 @@ mod tests { &mut w, &KeyEvent { key: Key::Enter, - utf8: "A", + utf8: "A".as_bytes(), unshifted_codepoint: 0x0D, ..KeyEvent::default() }, @@ -2170,7 +2192,7 @@ mod tests { &mut w, &KeyEvent { key: Key::Escape, - utf8: "A", + utf8: "A".as_bytes(), unshifted_codepoint: 0x0D, ..KeyEvent::default() }, @@ -2196,7 +2218,7 @@ mod tests { shift: true, ..Mods::default() }, - utf8: "_", + utf8: "_".as_bytes(), ..KeyEvent::default() }, &Options { @@ -2221,7 +2243,7 @@ mod tests { alt: true, ..Mods::default() }, - utf8: "c", + utf8: "c".as_bytes(), ..KeyEvent::default() }, &Options { @@ -2241,7 +2263,7 @@ mod tests { &mut w, &KeyEvent { key: Key::KeyC, - utf8: "c", + utf8: "c".as_bytes(), mods: Mods { alt: true, ..Mods::default() @@ -2293,7 +2315,7 @@ mod tests { &mut w, &KeyEvent { key: Key::KeyF, - utf8: "\u{444}", + utf8: "\u{444}".as_bytes(), mods: Mods { alt: true, ..Mods::default() @@ -2322,7 +2344,7 @@ mod tests { ctrl: true, ..Mods::default() }, - utf8: "c", + utf8: "c".as_bytes(), ..KeyEvent::default() }, &Options { @@ -2346,7 +2368,7 @@ mod tests { ctrl: true, ..Mods::default() }, - utf8: " ", + utf8: " ".as_bytes(), ..KeyEvent::default() }, &Options { @@ -2395,7 +2417,7 @@ mod tests { shift: true, ..Mods::default() }, - utf8: "H", + utf8: "H".as_bytes(), ..KeyEvent::default() }, &Options { @@ -2425,7 +2447,7 @@ mod tests { shift: true, ..Mods::default() }, - utf8: "H", + utf8: "H".as_bytes(), ..KeyEvent::default() }, &Options { @@ -2451,7 +2473,7 @@ mod tests { ..Mods::default() }, consumed_mods: Mods { ..Mods::default() }, - utf8: "8", + utf8: "8".as_bytes(), ..KeyEvent::default() }, &Options { @@ -2477,7 +2499,7 @@ mod tests { ctrl: true, ..Mods::default() }, - utf8: "i", + utf8: "i".as_bytes(), ..KeyEvent::default() }, &Options { @@ -2497,7 +2519,7 @@ mod tests { ctrl: true, ..Mods::default() }, - utf8: "m", + utf8: "m".as_bytes(), ..KeyEvent::default() }, &Options { @@ -2517,7 +2539,7 @@ mod tests { ctrl: true, ..Mods::default() }, - utf8: "[", + utf8: "[".as_bytes(), ..KeyEvent::default() }, &Options { @@ -2538,7 +2560,7 @@ mod tests { shift: true, ..Mods::default() }, - utf8: "@", + utf8: "@".as_bytes(), unshifted_codepoint: u32::from('2'), ..KeyEvent::default() }, @@ -2564,7 +2586,7 @@ mod tests { shift: true, ..Mods::default() }, - utf8: "M", + utf8: "M".as_bytes(), unshifted_codepoint: u32::from('m'), ..KeyEvent::default() }, @@ -2635,7 +2657,7 @@ mod tests { key: Key::Numpad1, mods: Mods { ..Mods::default() }, consumed_mods: Mods { ..Mods::default() }, - utf8: "1", + utf8: "1".as_bytes(), ..KeyEvent::default() }, &Options { @@ -2657,7 +2679,7 @@ mod tests { key: Key::Numpad1, mods: Mods { ..Mods::default() }, consumed_mods: Mods { ..Mods::default() }, - utf8: "1", + utf8: "1".as_bytes(), ..KeyEvent::default() }, &Options { @@ -2683,7 +2705,7 @@ mod tests { ..Mods::default() }, consumed_mods: Mods { ..Mods::default() }, - utf8: "1", + utf8: "1".as_bytes(), ..KeyEvent::default() }, &Options { @@ -2709,7 +2731,7 @@ mod tests { ..Mods::default() }, consumed_mods: Mods { ..Mods::default() }, - utf8: "1", + utf8: "1".as_bytes(), ..KeyEvent::default() }, &Options { @@ -2893,7 +2915,7 @@ mod tests { ctrl: true, ..Mods::default() }, - utf8: "\u{151}", + utf8: "\u{151}".as_bytes(), unshifted_codepoint: 337, ..KeyEvent::default() }, @@ -2914,7 +2936,7 @@ mod tests { &mut w, &KeyEvent { key: Key::Backspace, - utf8: "\u{7f}", + utf8: "\u{7f}".as_bytes(), unshifted_codepoint: 0x08, ..KeyEvent::default() }, @@ -2939,7 +2961,7 @@ mod tests { mods: KittyMods::default(), event: KittyEvent::None, alternates: [None, None], - text: "", + text: b"", } } @@ -3010,7 +3032,7 @@ mod tests { fn kitty_sequence_text() { assert_eq!( enc(KittySequence { - text: "A", + text: "A".as_bytes(), ..seq(127, b'u') }), b"\x1B[127;;65u" @@ -3018,7 +3040,7 @@ mod tests { assert_eq!( enc(KittySequence { event: KittyEvent::Release, - text: "A", + text: "A".as_bytes(), ..seq(127, b'u') }), b"\x1B[127;1:3;65u" @@ -3029,7 +3051,7 @@ mod tests { shift: true, ..KittyMods::default() }, - text: "A", + text: "A".as_bytes(), ..seq(127, b'u') }), b"\x1B[127;2;65u" @@ -3040,14 +3062,14 @@ mod tests { fn kitty_sequence_text_with_control_characters() { assert_eq!( enc(KittySequence { - text: "\n", + text: "\n".as_bytes(), ..seq(127, b'u') }), b"\x1b[127u" ); assert_eq!( enc(KittySequence { - text: "A\n", + text: "A\n".as_bytes(), ..seq(127, b'u') }), b"\x1b[127;;65u" @@ -3102,7 +3124,7 @@ mod tests { #[test] fn ctrlseq_normal_ctrl_c() { assert_eq!( - ctrl_seq(Key::Unidentified, "c", 'c' as u32, ctrl()), + ctrl_seq(Key::Unidentified, "c".as_bytes(), 'c' as u32, ctrl()), Some(0x03) ); } @@ -3110,7 +3132,7 @@ mod tests { #[test] fn ctrlseq_normal_ctrl_c_right_control() { assert_eq!( - ctrl_seq(Key::Unidentified, "c", 'c' as u32, right_ctrl()), + ctrl_seq(Key::Unidentified, "c".as_bytes(), 'c' as u32, right_ctrl()), Some(0x03) ); } @@ -3123,7 +3145,7 @@ mod tests { ..Mods::default() }; assert_eq!( - ctrl_seq(Key::Unidentified, "c", 'c' as u32, mods), + ctrl_seq(Key::Unidentified, "c".as_bytes(), 'c' as u32, mods), Some(0x03) ); } @@ -3131,7 +3153,12 @@ mod tests { #[test] fn ctrlseq_no_ctrl_does_nothing() { assert_eq!( - ctrl_seq(Key::Unidentified, "c", 'c' as u32, Mods::default()), + ctrl_seq( + Key::Unidentified, + "c".as_bytes(), + 'c' as u32, + Mods::default() + ), None ); } @@ -3144,7 +3171,7 @@ mod tests { ..Mods::default() }; assert_eq!( - ctrl_seq(Key::Unidentified, "_", '-' as u32, mods), + ctrl_seq(Key::Unidentified, "_".as_bytes(), '-' as u32, mods), Some(0x1F) ); } @@ -3157,7 +3184,7 @@ mod tests { ..Mods::default() }; assert_eq!( - ctrl_seq(Key::Unidentified, "C", 'c' as u32, mods), + ctrl_seq(Key::Unidentified, "C".as_bytes(), 'c' as u32, mods), Some(0x03) ); } @@ -3168,18 +3195,27 @@ mod tests { shift: true, ..Mods::default() }; - assert_eq!(ctrl_seq(Key::Unidentified, "C", 'c' as u32, shift), None); + assert_eq!( + ctrl_seq(Key::Unidentified, "C".as_bytes(), 'c' as u32, shift), + None + ); let both = Mods { shift: true, ctrl: true, ..Mods::default() }; - assert_eq!(ctrl_seq(Key::Unidentified, "C", 'c' as u32, both), None); + assert_eq!( + ctrl_seq(Key::Unidentified, "C".as_bytes(), 'c' as u32, both), + None + ); } #[test] fn ctrlseq_russian_ctrl_c() { - assert_eq!(ctrl_seq(Key::KeyC, "с", 0x0441, ctrl()), Some(0x03)); + assert_eq!( + ctrl_seq(Key::KeyC, "с".as_bytes(), 0x0441, ctrl()), + Some(0x03) + ); } #[test] @@ -3189,7 +3225,7 @@ mod tests { shift: true, ..Mods::default() }; - assert_eq!(ctrl_seq(Key::KeyC, "с", 0x0441, mods), None); + assert_eq!(ctrl_seq(Key::KeyC, "с".as_bytes(), 0x0441, mods), None); } #[test] @@ -3199,16 +3235,54 @@ mod tests { alt: true, ..Mods::default() }; - assert_eq!(ctrl_seq(Key::KeyC, "с", 0x0441, mods), Some(0x03)); + assert_eq!( + ctrl_seq(Key::KeyC, "с".as_bytes(), 0x0441, mods), + Some(0x03) + ); } #[test] fn ctrlseq_right_ctrl_c() { assert_eq!( - ctrl_seq(Key::KeyC, "с", 'c' as u32, right_ctrl()), + ctrl_seq(Key::KeyC, "с".as_bytes(), 'c' as u32, right_ctrl()), Some(0x03) ); } + + #[test] + fn invalid_utf8_text_is_passed_through_or_ignored() { + let ev = KeyEvent { + key: Key::KeyA, + unshifted_codepoint: 'a' as u32, + utf8: b"\xff\xfe", + ..KeyEvent::default() + }; + let mut out = Vec::new(); + legacy_like(&mut out, &ev); + assert_eq!(out, b"\xff\xfe"); + + // Kitty associated text with invalid UTF-8 is dropped, not lossy. + let mut out = Vec::new(); + encode( + &mut out, + &ev, + &Options { + kitty_flags: KittyFlags { + disambiguate: true, + report_all: true, + report_associated: true, + ..KittyFlags::default() + }, + ..Options::default() + }, + ) + .unwrap(); + assert_eq!(out, b"\x1b[97u"); + } + + fn legacy_like(out: &mut Vec, ev: &KeyEvent<'_>) { + encode(out, ev, &Options::default()).unwrap(); + } } /// The Zig tests that only run on Darwin (`isDarwin()` / `os.tag == .macos` @@ -3255,7 +3329,7 @@ mod macos_tests { alt: true, ..Mods::default() }, - utf8: "∑", + utf8: "∑".as_bytes(), unshifted_codepoint: 119, ..KeyEvent::default() }; @@ -3276,7 +3350,7 @@ mod macos_tests { alt: true, ..Mods::default() }, - utf8: "∑", + utf8: "∑".as_bytes(), unshifted_codepoint: 119, ..KeyEvent::default() }; @@ -3288,7 +3362,7 @@ mod macos_tests { // Without Alt modifier let ev = KeyEvent { key: Key::KeyW, - utf8: "∑", + utf8: "∑".as_bytes(), unshifted_codepoint: 119, ..KeyEvent::default() }; @@ -3304,7 +3378,7 @@ mod macos_tests { with_darwin(true, || { let ev = KeyEvent { key: Key::KeyC, - utf8: "≈", + utf8: "≈".as_bytes(), unshifted_codepoint: u32::from('c'), mods: Mods { alt: true, @@ -3333,7 +3407,7 @@ mod macos_tests { with_darwin(true, || { let ev = KeyEvent { key: Key::Period, - utf8: ">", + utf8: ">".as_bytes(), unshifted_codepoint: u32::from('.'), mods: Mods { alt: true, @@ -3365,7 +3439,7 @@ mod macos_tests { ..Mods::default() }, // common translation of option+8 with European keyboard layouts - utf8: "[", + utf8: "[".as_bytes(), ..KeyEvent::default() }; let opts = Options { @@ -3382,7 +3456,7 @@ mod macos_tests { with_darwin(true, || { let ev = KeyEvent { key: Key::KeyB, - utf8: "b", + utf8: "b".as_bytes(), mods: Mods { super_: true, ..Mods::default() @@ -3395,7 +3469,7 @@ mod macos_tests { with_darwin(false, || { let ev = KeyEvent { key: Key::KeyB, - utf8: "b", + utf8: "b".as_bytes(), mods: Mods { super_: true, ..Mods::default() @@ -3411,7 +3485,7 @@ mod macos_tests { with_darwin(true, || { let ev = KeyEvent { key: Key::KeyB, - utf8: "B", + utf8: "B".as_bytes(), mods: Mods { super_: true, shift: true, diff --git a/crates/ghostty-inspector/src/inspector.rs b/crates/ghostty-inspector/src/inspector.rs index d460a4f..285370d 100644 --- a/crates/ghostty-inspector/src/inspector.rs +++ b/crates/ghostty-inspector/src/inspector.rs @@ -170,7 +170,7 @@ mod tests { insp.record_key_event(crate::KeyEvent::new(&KeyEvent { action: Action::Press, key: Key::KeyA, - utf8: "a", + utf8: "a".as_bytes(), ..KeyEvent::default() })); diff --git a/crates/ghostty-inspector/src/widgets/key.rs b/crates/ghostty-inspector/src/widgets/key.rs index 5f98dff..63ab1f2 100644 --- a/crates/ghostty-inspector/src/widgets/key.rs +++ b/crates/ghostty-inspector/src/widgets/key.rs @@ -30,7 +30,8 @@ impl InputEvent { mods: ev.mods, consumed_mods: ev.consumed_mods, composing: ev.composing, - utf8: ev.utf8.to_owned(), + // Display only: invalid UTF-8 is shown as U+FFFD. + utf8: String::from_utf8_lossy(ev.utf8).into_owned(), unshifted_codepoint: ev.unshifted_codepoint, } } @@ -494,7 +495,7 @@ mod tests { Event::new(&KeyEvent { action: Action::Press, key, - utf8, + utf8: utf8.as_bytes(), ..KeyEvent::default() }) } diff --git a/crates/ghostty-lib/src/embedded.rs b/crates/ghostty-lib/src/embedded.rs index f71e541..b3ab654 100644 --- a/crates/ghostty-lib/src/embedded.rs +++ b/crates/ghostty-lib/src/embedded.rs @@ -216,7 +216,7 @@ struct OwnedKey { mods: Mods, consumed_mods: Mods, composing: bool, - text: String, + text: Vec, unshifted_codepoint: u32, } @@ -234,7 +234,7 @@ impl OwnedKey { }; // SAFETY: per the function contract. let text = unsafe { cstr(e.text) } - .map(|c| c.to_string_lossy().into_owned()) + .map(|c| c.to_bytes().to_vec()) .unwrap_or_default(); // We want to get the physical unmapped key to process keybinds. @@ -1636,7 +1636,7 @@ pub unsafe extern "C" fn ghostty_surface_preedit( } else { // SAFETY: `text` points to `len` readable bytes. let bytes = unsafe { std::slice::from_raw_parts(text.cast::(), len) }; - Some(String::from_utf8_lossy(bytes).into_owned()) + Some(bytes.to_vec()) }; if let Err(err) = core.preedit_callback(preedit.as_deref()) { log::error!("error in preedit callback err={err}"); diff --git a/crates/ghostty-lib/tests/surface.rs b/crates/ghostty-lib/tests/surface.rs index e316eda..d790cbc 100644 --- a/crates/ghostty-lib/tests/surface.rs +++ b/crates/ghostty-lib/tests/surface.rs @@ -519,6 +519,28 @@ fn selection_and_clipboard_round_trip() { /// A child that prints the hex of the first two bytes it receives raw. const HEX_TWO: &str = "stty raw -echo; dd bs=1 count=2 2>/dev/null | od -An -tx1 | tr -d ' \\n'; printf '\\nHEX-DONE\\n'; sleep 5"; +#[test] +fn key_text_and_preedit_are_bytes_not_lossy_utf8() { + // Key text is a byte slice (Zig `[]const u8`): non-UTF-8 text reaches + // the pty unchanged. + let h = host!(""); + let s = h.surface(HEX_TWO); + let raw = CString::new(&b"\xff\xfe"[..]).unwrap(); + assert!(unsafe { ghostty_surface_key(s, key(1, KEY_A, 0, Some(&raw))) }); + h.wait_screen(s, "fffe"); + unsafe { ghostty_surface_free(s) }; + + // Invalid preedit bytes are rejected like Zig (logged error, no panic). + let s = h.surface("printf 'READY\\n'; sleep 30"); + h.wait_screen(s, "READY"); + let p = b"a\xffb"; + unsafe { + ghostty_surface_preedit(s, p.as_ptr().cast(), p.len()); + ghostty_surface_preedit(s, ptr::null(), 0); + ghostty_surface_free(s); + } +} + #[test] fn clipboard_completion_and_binding_action_keep_raw_bytes() { // Clipboard contents are bytes: 0xFF must reach the child as 0xFF. diff --git a/crates/ghostty-vt-capi/src/key_event.rs b/crates/ghostty-vt-capi/src/key_event.rs index d0d20b7..6f02cab 100644 --- a/crates/ghostty-vt-capi/src/key_event.rs +++ b/crates/ghostty-vt-capi/src/key_event.rs @@ -27,8 +27,8 @@ pub struct KeyEventWrapper { } impl KeyEventWrapper { - /// Borrow as the Rust [`KeyEvent`]. Zig's `utf8` is a byte slice, ours is - /// `&str`: invalid UTF-8 is truncated to its longest valid prefix. + /// Borrow as the Rust [`KeyEvent`]. `utf8` is a byte slice (as in Zig) and is + /// passed through unchanged, even if it is not valid UTF-8. /// /// # Safety /// The caller-provided UTF-8 bytes must still be alive. @@ -39,17 +39,13 @@ impl KeyEventWrapper { // SAFETY: per the contract of `ghostty_key_event_set_utf8`. unsafe { std::slice::from_raw_parts(self.utf8, self.utf8_len) } }; - let text = match std::str::from_utf8(bytes) { - Ok(s) => s, - Err(e) => std::str::from_utf8(&bytes[..e.valid_up_to()]).unwrap_or(""), - }; KeyEvent { action: self.action, key: self.key, mods: self.mods, consumed_mods: self.consumed_mods, composing: self.composing, - utf8: text, + utf8: bytes, unshifted_codepoint: self.unshifted_codepoint, } } diff --git a/crates/ghostty-vt-capi/tests/wasm_examples.rs b/crates/ghostty-vt-capi/tests/wasm_examples.rs index 599cefc..f772757 100644 --- a/crates/ghostty-vt-capi/tests/wasm_examples.rs +++ b/crates/ghostty-vt-capi/tests/wasm_examples.rs @@ -282,7 +282,7 @@ fn wasm_key_encode_example() { action, key, mods, - utf8, + utf8: utf8.as_bytes(), unshifted_codepoint: unshifted as u32, ..KeyEvent::default() }; diff --git a/docs/rewrite/notes/final-Z10.md b/docs/rewrite/notes/final-Z10.md new file mode 100644 index 0000000..808b19f --- /dev/null +++ b/docs/rewrite/notes/final-Z10.md @@ -0,0 +1,16 @@ +# Z10: byte-typed key text and preedit + +`KeyEvent.utf8` is now `&[u8]` (Zig `[]const u8`) end to end; the Z9 "documented limit" for C key `text` / preedit is gone. + +- `ghostty-input`: `KeyEvent.utf8: &[u8]`; `key_encode` writes the bytes unchanged. Where Zig validates with `Utf8View.init ... catch` (plain-text control scan, kitty alternates, modify-other-keys, ctrl CSI-u, kitty associated text, binding unicode lookup) invalid UTF-8 takes the same "no UTF-8" fallback (`utf8_chars`). Legacy/kitty "send text directly" paths write the raw bytes. +- `ghostty-core`: `OwnedKeyEvent.utf8: Vec`; `Surface::preedit_callback(Option<&[u8]>)` (and the `RuntimeSurface` trait in `ghostty-apprt`). Preedit bytes are decoded lossily (U+FFFD) for the renderer codepoints only. Deviation: Zig returns an error on invalid preedit; we render replacement chars instead (no panic). +- `ghostty-lib`: `ghostty_surface_key` copies `text` as raw bytes; `ghostty_surface_preedit` passes bytes. `ghostty_surface_text` was already bytes. +- `ghostty-vt-capi`: `ghostty_key_event_*` no longer truncates invalid UTF-8 to its valid prefix; passes bytes through. +- Inspector key widget shows invalid text lossily (display only). GTK callers pass `as_bytes()` of valid UTF-8. +- Tests: `ghostty-lib/tests/surface.rs::key_text_and_preedit_are_bytes_not_lossy_utf8` (key text `ff fe` reaches the pty), `ghostty-core/tests/e2e_ime.rs::preedit_with_invalid_utf8_is_replaced_for_display_only`, `ghostty-input` `invalid_utf8_*` encoder tests. Existing tests only changed mechanically (`"x"` -> `"x".as_bytes()`). + +## Orchestrator follow-up +Preedit with invalid UTF-8 now matches Zig exactly instead of rendering +U+FFFD: `preedit_callback` clears the prior preedit, marks it dirty and +returns an error (Zig: `try std.unicode.Utf8View.init(text)`); the C API +logs it like `embedded.zig`. Test: `preedit_with_invalid_utf8_is_an_error_and_clears_prior`.