mirror of
https://github.com/zellij-org/zellij.git
synced 2026-10-12 02:27:24 +00:00
fix: position host cursor for CJK IME even when app hides cursor (#4951)
* fix: position host cursor for CJK IME even when app hides cursor When apps like Claude Code (ink/React library) hide the cursor during rendering, Zellij's render_cursor() was not sending a CUP sequence to the host terminal. This caused IME composition candidates to appear at the wrong position, breaking CJK input in these apps. Change cursor_coordinates() to return the cursor position unconditionally (as long as it is within bounds) along with a bool indicating visibility. render_cursor() now positions the host cursor even when the app has hidden it, so the IME subsystem can read the correct cursor position. * add test * rustfmt --------- Co-authored-by: Aram Drevekenin <[email protected]>
This commit is contained in:
co-authored by
Aram Drevekenin
parent
cbe0237b1e
commit
27a825ebdc
@@ -337,8 +337,10 @@ fn read_from_channel(
|
||||
Ok(0) => {
|
||||
let current_snapshot = take_snapshot(&mut terminal_output);
|
||||
let mut last_snapshot = last_snapshot.lock().unwrap();
|
||||
*cursor_coordinates.lock().unwrap() =
|
||||
terminal_output.cursor_coordinates().unwrap_or((0, 0));
|
||||
*cursor_coordinates.lock().unwrap() = terminal_output
|
||||
.cursor_coordinates()
|
||||
.map(|(x, y, _)| (x, y))
|
||||
.unwrap_or((0, 0));
|
||||
*last_snapshot = current_snapshot;
|
||||
should_sleep = true;
|
||||
},
|
||||
@@ -348,8 +350,11 @@ fn read_from_channel(
|
||||
}
|
||||
let current_snapshot = take_snapshot(&mut terminal_output);
|
||||
let mut last_snapshot = last_snapshot.lock().unwrap();
|
||||
*cursor_coordinates.lock().unwrap() =
|
||||
terminal_output.grid.cursor_coordinates().unwrap_or((0, 0));
|
||||
*cursor_coordinates.lock().unwrap() = terminal_output
|
||||
.grid
|
||||
.cursor_coordinates()
|
||||
.map(|(x, y, _)| (x, y))
|
||||
.unwrap_or((0, 0));
|
||||
*last_snapshot = current_snapshot;
|
||||
should_sleep = true;
|
||||
},
|
||||
@@ -357,8 +362,10 @@ fn read_from_channel(
|
||||
if e.kind() == std::io::ErrorKind::WouldBlock {
|
||||
let current_snapshot = take_snapshot(&mut terminal_output);
|
||||
let mut last_snapshot = last_snapshot.lock().unwrap();
|
||||
*cursor_coordinates.lock().unwrap() =
|
||||
terminal_output.cursor_coordinates().unwrap_or((0, 0));
|
||||
*cursor_coordinates.lock().unwrap() = terminal_output
|
||||
.cursor_coordinates()
|
||||
.map(|(x, y, _)| (x, y))
|
||||
.unwrap_or((0, 0));
|
||||
*last_snapshot = current_snapshot;
|
||||
should_sleep = true;
|
||||
} else if retries_left > 0 {
|
||||
@@ -377,7 +384,9 @@ fn read_from_channel(
|
||||
|
||||
pub fn take_snapshot(terminal_output: &mut TerminalPane) -> String {
|
||||
let output_lines = terminal_output.read_buffer_as_lines();
|
||||
let cursor_coordinates = terminal_output.cursor_coordinates();
|
||||
let cursor_coordinates = terminal_output
|
||||
.cursor_coordinates()
|
||||
.and_then(|(x, y, visible)| if visible { Some((x, y)) } else { None });
|
||||
let mut snapshot = String::new();
|
||||
for (line_index, line) in output_lines.iter().enumerate() {
|
||||
for (character_index, terminal_character) in line.iter().enumerate() {
|
||||
|
||||
@@ -1574,11 +1574,15 @@ impl Grid {
|
||||
sixel_image_chunks,
|
||||
)));
|
||||
}
|
||||
pub fn cursor_coordinates(&self) -> Option<(usize, usize)> {
|
||||
if self.cursor_is_hidden || self.cursor.x >= self.width || self.cursor.y >= self.height {
|
||||
/// Returns the cursor position and whether it is visible.
|
||||
/// The position is returned unconditionally (as long as the cursor is within
|
||||
/// bounds) so that the host terminal can position the cursor for IME even
|
||||
/// when the app has hidden it. The bool is true when the cursor is visible.
|
||||
pub fn cursor_coordinates(&self) -> Option<(usize, usize, bool)> {
|
||||
if self.cursor.x >= self.width || self.cursor.y >= self.height {
|
||||
None
|
||||
} else {
|
||||
Some((self.cursor.x, self.cursor.y))
|
||||
Some((self.cursor.x, self.cursor.y, !self.cursor_is_hidden))
|
||||
}
|
||||
}
|
||||
pub fn is_mid_frame(&self) -> bool {
|
||||
|
||||
@@ -252,7 +252,7 @@ impl Pane for PluginPane {
|
||||
|
||||
self.should_render.insert(client_id, true);
|
||||
}
|
||||
fn cursor_coordinates(&self, client_id: Option<ClientId>) -> Option<(usize, usize)> {
|
||||
fn cursor_coordinates(&self, client_id: Option<ClientId>) -> Option<(usize, usize, bool)> {
|
||||
let own_content_columns = self.get_content_columns();
|
||||
let own_content_rows = self.get_content_rows();
|
||||
let Offset { top, left, .. } = self.content_offset;
|
||||
@@ -265,7 +265,7 @@ impl Pane for PluginPane {
|
||||
if x >= own_content_columns || y >= own_content_rows {
|
||||
None
|
||||
} else {
|
||||
Some((x, y)) // these are 0 indexed
|
||||
Some((x, y, true)) // plugins always show cursor when position is set
|
||||
}
|
||||
})
|
||||
} else {
|
||||
|
||||
@@ -207,8 +207,8 @@ impl Pane for TerminalPane {
|
||||
self.vte_parser.advance(&mut self.grid, byte);
|
||||
}
|
||||
}
|
||||
fn cursor_coordinates(&self, _client_id: Option<ClientId>) -> Option<(usize, usize)> {
|
||||
// (x, y)
|
||||
fn cursor_coordinates(&self, _client_id: Option<ClientId>) -> Option<(usize, usize, bool)> {
|
||||
// (x, y, is_visible)
|
||||
if self.get_content_rows() < 1 || self.get_content_columns() < 1 {
|
||||
// do not render cursor if there's no room for it
|
||||
return None;
|
||||
@@ -216,7 +216,7 @@ impl Pane for TerminalPane {
|
||||
let Offset { top, left, .. } = self.content_offset;
|
||||
self.grid
|
||||
.cursor_coordinates()
|
||||
.map(|(x, y)| (x + left, y + top))
|
||||
.map(|(x, y, is_visible)| (x + left, y + top, is_visible))
|
||||
}
|
||||
fn is_mid_frame(&self) -> bool {
|
||||
self.grid.is_mid_frame()
|
||||
@@ -458,7 +458,7 @@ impl Pane for TerminalPane {
|
||||
text_color: PaletteColor,
|
||||
) -> Option<String> {
|
||||
let mut vte_output = None;
|
||||
if let Some((cursor_x, cursor_y)) = self.cursor_coordinates() {
|
||||
if let Some((cursor_x, cursor_y, true)) = self.cursor_coordinates() {
|
||||
let mut character_under_cursor = self
|
||||
.grid
|
||||
.get_character_under_cursor()
|
||||
@@ -1117,8 +1117,8 @@ impl TerminalPane {
|
||||
pub fn read_buffer_as_lines(&self) -> Vec<Vec<TerminalCharacter>> {
|
||||
self.grid.as_character_lines()
|
||||
}
|
||||
pub fn cursor_coordinates(&self) -> Option<(usize, usize)> {
|
||||
// (x, y)
|
||||
pub fn cursor_coordinates(&self) -> Option<(usize, usize, bool)> {
|
||||
// (x, y, is_visible)
|
||||
if self.get_content_rows() < 1 || self.get_content_columns() < 1 {
|
||||
// do not render cursor if there's no room for it
|
||||
return None;
|
||||
|
||||
@@ -1085,7 +1085,7 @@ impl Pane for MockPane {
|
||||
fn handle_plugin_bytes(&mut self, _client_id: ClientId, _bytes: VteBytes) {
|
||||
unimplemented!()
|
||||
}
|
||||
fn cursor_coordinates(&self, _client_id: Option<ClientId>) -> Option<(usize, usize)> {
|
||||
fn cursor_coordinates(&self, _client_id: Option<ClientId>) -> Option<(usize, usize, bool)> {
|
||||
unimplemented!()
|
||||
}
|
||||
|
||||
|
||||
@@ -3797,15 +3797,17 @@ pub fn cursor_hide_persists_through_alternate_screen() {
|
||||
for byte in hide_cursor.as_bytes() {
|
||||
vte_parser.advance(&mut grid, *byte);
|
||||
}
|
||||
assert_eq!(grid.cursor_coordinates(), None, "Cursor hidden properly");
|
||||
assert!(
|
||||
matches!(grid.cursor_coordinates(), Some((_, _, false))),
|
||||
"Cursor hidden properly"
|
||||
);
|
||||
|
||||
let move_to_alternate_screen = "\u{1b}[?1049h";
|
||||
for byte in move_to_alternate_screen.as_bytes() {
|
||||
vte_parser.advance(&mut grid, *byte);
|
||||
}
|
||||
assert_eq!(
|
||||
grid.cursor_coordinates(),
|
||||
None,
|
||||
assert!(
|
||||
matches!(grid.cursor_coordinates(), Some((_, _, false))),
|
||||
"Cursor still hidden in alternate screen"
|
||||
);
|
||||
|
||||
@@ -3813,14 +3815,17 @@ pub fn cursor_hide_persists_through_alternate_screen() {
|
||||
for byte in show_cursor.as_bytes() {
|
||||
vte_parser.advance(&mut grid, *byte);
|
||||
}
|
||||
assert!(grid.cursor_coordinates().is_some(), "Cursor shown");
|
||||
assert!(
|
||||
matches!(grid.cursor_coordinates(), Some((_, _, true))),
|
||||
"Cursor shown"
|
||||
);
|
||||
|
||||
let move_away_from_alternate_screen = "\u{1b}[?1049l";
|
||||
for byte in move_away_from_alternate_screen.as_bytes() {
|
||||
vte_parser.advance(&mut grid, *byte);
|
||||
}
|
||||
assert!(
|
||||
grid.cursor_coordinates().is_some(),
|
||||
matches!(grid.cursor_coordinates(), Some((_, _, true))),
|
||||
"Cursor still shown away from alternate screen"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -240,7 +240,11 @@ pub trait Pane {
|
||||
fn handle_pty_bytes(&mut self, _bytes: VteBytes) {}
|
||||
fn handle_plugin_bytes(&mut self, _client_id: ClientId, _bytes: VteBytes) {}
|
||||
fn show_cursor(&mut self, _client_id: ClientId, _cursor_position: Option<(usize, usize)>) {}
|
||||
fn cursor_coordinates(&self, _client_id: Option<ClientId>) -> Option<(usize, usize)>;
|
||||
/// Returns the cursor position and whether it is visible.
|
||||
/// The position is returned unconditionally (as long as the cursor is within
|
||||
/// bounds) so that the host terminal can position the cursor for IME even
|
||||
/// when the app has hidden it. The bool is true when the cursor is visible.
|
||||
fn cursor_coordinates(&self, _client_id: Option<ClientId>) -> Option<(usize, usize, bool)>;
|
||||
fn is_mid_frame(&self) -> bool {
|
||||
false
|
||||
}
|
||||
@@ -3037,8 +3041,8 @@ impl Tab {
|
||||
pub fn get_active_terminal_cursor_position(
|
||||
&self,
|
||||
client_id: ClientId,
|
||||
) -> Option<(usize, usize)> {
|
||||
// (x, y)
|
||||
) -> Option<(usize, usize, bool)> {
|
||||
// (x, y, is_cursor_visible)
|
||||
let active_pane_id = if self.floating_panes.panes_are_visible() {
|
||||
self.floating_panes
|
||||
.get_active_pane_id(client_id)
|
||||
@@ -3050,13 +3054,13 @@ impl Tab {
|
||||
.floating_panes
|
||||
.get(&active_pane_id)
|
||||
.or_else(|| self.tiled_panes.get_pane(active_pane_id))?;
|
||||
active_terminal
|
||||
.cursor_coordinates(Some(client_id))
|
||||
.map(|(x_in_terminal, y_in_terminal)| {
|
||||
active_terminal.cursor_coordinates(Some(client_id)).map(
|
||||
|(x_in_terminal, y_in_terminal, is_visible)| {
|
||||
let x = active_terminal.x() + x_in_terminal;
|
||||
let y = active_terminal.y() + y_in_terminal;
|
||||
(x, y)
|
||||
})
|
||||
(x, y, is_visible)
|
||||
},
|
||||
)
|
||||
}
|
||||
pub fn toggle_active_pane_fullscreen(&mut self, client_id: ClientId) {
|
||||
if self.floating_panes.panes_are_visible() {
|
||||
@@ -3236,36 +3240,16 @@ impl Tab {
|
||||
let connected_clients: Vec<ClientId> =
|
||||
{ self.connected_clients.borrow().iter().copied().collect() };
|
||||
for client_id in connected_clients {
|
||||
match self
|
||||
.get_active_terminal_cursor_position(client_id)
|
||||
.and_then(|(cursor_position_x, cursor_position_y)| {
|
||||
match self.get_active_terminal_cursor_position(client_id) {
|
||||
Some((cursor_position_x, cursor_position_y, is_cursor_visible)) => {
|
||||
let active_pane_z_index = self
|
||||
.get_active_pane_id(client_id)
|
||||
.and_then(|pane_id| self.floating_panes.get_pane_z_index(pane_id));
|
||||
if output.cursor_is_visible(
|
||||
let not_occluded = output.cursor_is_visible(
|
||||
cursor_position_x,
|
||||
cursor_position_y,
|
||||
active_pane_z_index,
|
||||
) {
|
||||
Some((cursor_position_x, cursor_position_y))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}) {
|
||||
Some((cursor_position_x, cursor_position_y)) => {
|
||||
let desired_cursor_shape = self
|
||||
.get_active_pane(client_id)
|
||||
.map(|ap| ap.cursor_shape_csi())
|
||||
.unwrap_or_default();
|
||||
let cursor_changed_position_or_shape = self
|
||||
.cursor_positions_and_shape
|
||||
.get(&client_id)
|
||||
.map(|(previous_x, previous_y, previous_shape)| {
|
||||
previous_x != &cursor_position_x
|
||||
|| previous_y != &cursor_position_y
|
||||
|| previous_shape != &desired_cursor_shape
|
||||
})
|
||||
.unwrap_or(true);
|
||||
);
|
||||
let active_terminal_is_mid_frame = self
|
||||
.active_terminal_is_mid_frame(client_id)
|
||||
.unwrap_or(false);
|
||||
@@ -3274,22 +3258,54 @@ impl Tab {
|
||||
// no-op, this means the active terminal is currently rendering a frame,
|
||||
// which means the cursor can be jumping around and we definitely do not
|
||||
// want to render it
|
||||
//
|
||||
// (I felt this was clearer than expanding the if conditional below)
|
||||
} else if output.is_dirty() || cursor_changed_position_or_shape {
|
||||
let show_cursor = "\u{1b}[?25h";
|
||||
} else if not_occluded && is_cursor_visible {
|
||||
let desired_cursor_shape = self
|
||||
.get_active_pane(client_id)
|
||||
.map(|ap| ap.cursor_shape_csi())
|
||||
.unwrap_or_default();
|
||||
let cursor_changed_position_or_shape = self
|
||||
.cursor_positions_and_shape
|
||||
.get(&client_id)
|
||||
.map(|(previous_x, previous_y, previous_shape)| {
|
||||
previous_x != &cursor_position_x
|
||||
|| previous_y != &cursor_position_y
|
||||
|| previous_shape != &desired_cursor_shape
|
||||
})
|
||||
.unwrap_or(true);
|
||||
if output.is_dirty() || cursor_changed_position_or_shape {
|
||||
let show_cursor = "\u{1b}[?25h";
|
||||
let goto_cursor_position = &format!(
|
||||
"\u{1b}[{};{}H\u{1b}[m{}",
|
||||
cursor_position_y + 1,
|
||||
cursor_position_x + 1,
|
||||
desired_cursor_shape
|
||||
); // goto row/col
|
||||
output.add_post_vte_instruction_to_client(client_id, show_cursor);
|
||||
output.add_post_vte_instruction_to_client(
|
||||
client_id,
|
||||
goto_cursor_position,
|
||||
);
|
||||
self.cursor_positions_and_shape.insert(
|
||||
client_id,
|
||||
(cursor_position_x, cursor_position_y, desired_cursor_shape),
|
||||
);
|
||||
}
|
||||
} else if not_occluded {
|
||||
// Cursor is hidden by the app but not occluded by a floating
|
||||
// pane. Position the host terminal cursor at the correct
|
||||
// location (for IME) then hide it. The IME subsystem reads
|
||||
// the cursor position regardless of visibility.
|
||||
let hide_cursor = "\u{1b}[?25l";
|
||||
let goto_cursor_position = &format!(
|
||||
"\u{1b}[{};{}H\u{1b}[m{}",
|
||||
"\u{1b}[{};{}H",
|
||||
cursor_position_y + 1,
|
||||
cursor_position_x + 1,
|
||||
desired_cursor_shape
|
||||
); // goto row/col
|
||||
output.add_post_vte_instruction_to_client(client_id, show_cursor);
|
||||
output.add_post_vte_instruction_to_client(client_id, goto_cursor_position);
|
||||
self.cursor_positions_and_shape.insert(
|
||||
client_id,
|
||||
(cursor_position_x, cursor_position_y, desired_cursor_shape),
|
||||
);
|
||||
output.add_post_vte_instruction_to_client(client_id, hide_cursor);
|
||||
output.add_post_vte_instruction_to_client(client_id, goto_cursor_position);
|
||||
} else {
|
||||
let hide_cursor = "\u{1b}[?25l";
|
||||
output.add_post_vte_instruction_to_client(client_id, hide_cursor);
|
||||
}
|
||||
},
|
||||
None => {
|
||||
@@ -6004,7 +6020,9 @@ pub fn pane_info_for_pane(
|
||||
pane_info.pane_content_rows = pane.get_content_rows();
|
||||
pane_info.pane_columns = pane.cols();
|
||||
pane_info.pane_content_columns = pane.get_content_columns();
|
||||
pane_info.cursor_coordinates_in_pane = pane.cursor_coordinates(None);
|
||||
pane_info.cursor_coordinates_in_pane = pane
|
||||
.cursor_coordinates(None)
|
||||
.and_then(|(x, y, is_visible)| if is_visible { Some((x, y)) } else { None });
|
||||
pane_info.is_selectable = pane.selectable();
|
||||
pane_info.title = pane.current_title();
|
||||
pane_info.exited = pane.exited();
|
||||
|
||||
@@ -990,7 +990,10 @@ fn take_snapshot_and_cursor_position(
|
||||
for &byte in ansi_instructions.as_bytes() {
|
||||
vte_parser.advance(&mut grid, byte);
|
||||
}
|
||||
(format!("{:?}", grid), grid.cursor_coordinates())
|
||||
let coords = grid
|
||||
.cursor_coordinates()
|
||||
.and_then(|(x, y, visible)| if visible { Some((x, y)) } else { None });
|
||||
(format!("{:?}", grid), coords)
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -14798,3 +14801,47 @@ fn osc99_query_flag_roundtrip() {
|
||||
assert!(!wants_report);
|
||||
assert!(!is_query);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hidden_cursor_still_emits_cup_for_host_terminal_positioning() {
|
||||
// When an app hides the cursor the host
|
||||
// terminal still needs a CUP sequence so that IME composition candidates and
|
||||
// other cursor-position-dependent features appear in the correct location.
|
||||
let size = Size { cols: 80, rows: 24 };
|
||||
let client_id = 1;
|
||||
let mut tab = create_new_tab(size, ModeInfo::default());
|
||||
|
||||
// Move cursor to a known position, then hide it
|
||||
tab.handle_pty_bytes(1, Vec::from("\u{1b}[10;20H\u{1b}[?25l".as_bytes()))
|
||||
.unwrap();
|
||||
|
||||
let mut output = Output::default();
|
||||
tab.render(&mut output, None).unwrap();
|
||||
let serialized = output.serialize().unwrap();
|
||||
let client_output = serialized.get(&client_id).unwrap();
|
||||
|
||||
// The output must contain a hide-cursor instruction
|
||||
assert!(
|
||||
client_output.contains("\u{1b}[?25l"),
|
||||
"Expected hide-cursor sequence in output"
|
||||
);
|
||||
|
||||
// The output must contain a CUP sequence positioning the cursor where the app
|
||||
// left it (row 10, col 20 — 1-indexed in the pane, plus the content offset
|
||||
// from the pane frame). The key point is that a CUP is emitted at all, even
|
||||
// though the cursor is hidden.
|
||||
//
|
||||
// With draw_pane_frames=true the content offset is (1, 1), so the absolute
|
||||
// position is row 11, col 21.
|
||||
assert!(
|
||||
client_output.contains("\u{1b}[11;21H"),
|
||||
"Expected CUP sequence for hidden cursor positioning, got: {:?}",
|
||||
client_output
|
||||
);
|
||||
|
||||
// The output must NOT contain a show-cursor instruction
|
||||
assert!(
|
||||
!client_output.contains("\u{1b}[?25h"),
|
||||
"Show-cursor sequence must not be present when app has hidden the cursor"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -163,12 +163,16 @@ impl<'a> PaneContentsAndUi<'a> {
|
||||
let cursor_is_visible = self
|
||||
.pane
|
||||
.cursor_coordinates(Some(*fake_cursor_client_id))
|
||||
.map(|(x, y)| {
|
||||
self.output.cursor_is_visible(
|
||||
self.pane.x() + x,
|
||||
self.pane.y() + y,
|
||||
self.z_index,
|
||||
)
|
||||
.and_then(|(x, y, is_visible)| {
|
||||
if is_visible {
|
||||
Some(self.output.cursor_is_visible(
|
||||
self.pane.x() + x,
|
||||
self.pane.y() + y,
|
||||
self.z_index,
|
||||
))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
})
|
||||
.unwrap_or(false);
|
||||
if cursor_is_visible {
|
||||
|
||||
@@ -63,7 +63,10 @@ fn take_snapshot_and_cursor_coordinates(
|
||||
for &byte in ansi_instructions.as_bytes() {
|
||||
vte_parser.advance(grid, byte);
|
||||
}
|
||||
(grid.cursor_coordinates(), format!("{:?}", grid))
|
||||
let coords = grid
|
||||
.cursor_coordinates()
|
||||
.and_then(|(x, y, visible)| if visible { Some((x, y)) } else { None });
|
||||
(coords, format!("{:?}", grid))
|
||||
}
|
||||
|
||||
fn take_snapshots_and_cursor_coordinates_from_render_events<'a>(
|
||||
|
||||
Reference in New Issue
Block a user