From 07bed472b44ea4514a29267c9b00c91852ed8777 Mon Sep 17 00:00:00 2001 From: visitorise <91079853+visitorise@users.noreply.github.com> Date: Tue, 4 Aug 2026 07:09:04 +0900 Subject: [PATCH 1/3] fix: correct multibyte character handling in get_selected_text() The get_selected_text() method was using character indices from tui-textarea as byte offsets for string slicing. This caused incorrect text selection and potential panics when selecting Korean, Japanese, or other multibyte UTF-8 text. The fix uses the existing char_col_to_byte_offset() helper to properly convert character indices to byte offsets before slicing. Added tests for both English ASCII and Korean multibyte text selection. --- src/ui/components/input.rs | 39 +++++++++++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/src/ui/components/input.rs b/src/ui/components/input.rs index 61a9e3a..cc3f4d6 100644 --- a/src/ui/components/input.rs +++ b/src/ui/components/input.rs @@ -878,12 +878,12 @@ impl Input { continue; } let start = if i == start_row { - start_col.min(line.len()) + Self::char_col_to_byte_offset(line, start_col) } else { 0 }; let end = if i == end_row { - end_col.min(line.len()) + Self::char_col_to_byte_offset(line, end_col) } else { line.len() }; @@ -891,7 +891,6 @@ impl Input { if start >= end { continue; } - // Byte-based slicing (safe: start/end are guaranteed char boundaries) if !result.is_empty() { result.push('\n'); } @@ -3019,4 +3018,38 @@ mod tests { assert_eq!(after_style.fg, Some(colors.markdown_image_text)); assert_eq!(after_style.bg, before_style.bg); } + + #[test] + fn test_get_selected_text_english_ascii() { + let mut input = Input::new(); + input.insert_str("Hello World"); + input.textarea.move_cursor(CursorMove::Jump(0, 6)); + input.textarea.start_selection(); + for _ in 0..5 { + input.textarea.move_cursor(CursorMove::Forward); + } + + assert!(input.has_selection()); + assert_eq!(input.get_selected_text(), "World"); + } + + #[test] + fn test_get_selected_text_korean_multibyte() { + let mut input = Input::new(); + // "안녕하세요" = 5 Korean chars, each 3 bytes in UTF-8 (total 15 bytes) + input.insert_str("안녕하세요"); + + // Move cursor to char position 2 (after "녕") + input.textarea.move_cursor(CursorMove::Jump(0, 2)); + input.textarea.start_selection(); + + // Move cursor forward 2 chars to select chars 2-3 ("하세") + input.textarea.move_cursor(CursorMove::Forward); + input.textarea.move_cursor(CursorMove::Forward); + + assert!(input.has_selection()); + // Selection is from char 2 to char 4 (chars positions 2 and 3) + // With the bug, this would produce incorrect bytes; with fix, it should be "하세" + assert_eq!(input.get_selected_text(), "하세"); + } } From 14a5893da5b7d16938afd6eeb08c825ce4e35060 Mon Sep 17 00:00:00 2001 From: visitorise <91079853+visitorise@users.noreply.github.com> Date: Tue, 4 Aug 2026 07:52:04 +0900 Subject: [PATCH 2/3] fix: correct terminal cursor positioning for IME candidate window in CJK text Fixed byte-offset to char-index conversion in visual line computation to properly position the terminal cursor for IME candidate windows in CJK environments. The cursor position was incorrectly calculated for multi-byte text, causing the IME window to appear at the wrong location. - Fix byte_offset to char_index conversion in VisualLine computation - Add set_terminal_cursor_position to position IME candidate window correctly for CJK text with proper Unicode width calculation - Add tests for CJK cursor positioning and English fallback --- src/ui/components/input.rs | 134 +++++++++++++++++++++++++++++++++++-- 1 file changed, 130 insertions(+), 4 deletions(-) diff --git a/src/ui/components/input.rs b/src/ui/components/input.rs index cc3f4d6..c976ad3 100644 --- a/src/ui/components/input.rs +++ b/src/ui/components/input.rs @@ -487,6 +487,13 @@ impl Input { self.update_viewport(visible_lines, wrap_width); self.render_wrapped_textarea(frame, v_chunks[1], colors); + // Set the physical terminal cursor position to the textarea's cursor + // location so that the IME candidate window appears at the correct position. + // This is essential for CJK input methods. + if let Some(area) = self.textarea_area { + self.set_terminal_cursor_position(frame, area); + } + let mut info_spans = vec![ ratatui::text::Span::styled(agent.to_string(), Style::default().fg(agent_color)), ratatui::text::Span::raw(" "), @@ -1426,6 +1433,51 @@ impl Input { self.render_paste_hover_tooltip(frame.buffer_mut(), area, colors, &visual_lines); } + fn set_terminal_cursor_position(&self, frame: &mut ratatui::Frame, area: Rect) { + if area.width == 0 || area.height == 0 { + return; + } + + let (cursor_row, cursor_col) = self.textarea.cursor(); + let wrap_width = area.width as usize; + let visual_lines = self.visual_lines(wrap_width); + + // Find the visual line containing the cursor's source row, then compute + // the rendered column accounting for wrapping and character widths. + let cursor_vl = visual_lines + .iter() + .skip(self.viewport_top) + .take(area.height as usize) + .enumerate() + .find(|(_, vl)| { + vl.source_row == cursor_row + && cursor_col >= vl.start_col + && cursor_col <= vl.end_col + }); + + if let Some((screen_row, vl)) = cursor_vl { + let line = self.textarea.lines().get(cursor_row); + let prefix_width = line + .map(|l| { + l.chars() + .take(cursor_col) + .map(|c| UnicodeWidthChar::width(c).unwrap_or(0)) + .sum::() + }) + .unwrap_or(0); + + // Account for horizontal scrolling if present + let render_col = prefix_width.saturating_sub(vl.start_col); + let cursor_x = area.x + render_col.min(area.width.saturating_sub(1) as usize) as u16; + let cursor_y = area.y + screen_row as u16; + + frame.set_cursor_position(ratatui::layout::Position { + x: cursor_x, + y: cursor_y, + }); + } + } + fn render_visual_line( line: &str, visual_line: &VisualLine, @@ -3028,7 +3080,7 @@ mod tests { for _ in 0..5 { input.textarea.move_cursor(CursorMove::Forward); } - + assert!(input.has_selection()); assert_eq!(input.get_selected_text(), "World"); } @@ -3038,18 +3090,92 @@ mod tests { let mut input = Input::new(); // "안녕하세요" = 5 Korean chars, each 3 bytes in UTF-8 (total 15 bytes) input.insert_str("안녕하세요"); - + // Move cursor to char position 2 (after "녕") input.textarea.move_cursor(CursorMove::Jump(0, 2)); input.textarea.start_selection(); - + // Move cursor forward 2 chars to select chars 2-3 ("하세") input.textarea.move_cursor(CursorMove::Forward); input.textarea.move_cursor(CursorMove::Forward); - + assert!(input.has_selection()); // Selection is from char 2 to char 4 (chars positions 2 and 3) // With the bug, this would produce incorrect bytes; with fix, it should be "하세" assert_eq!(input.get_selected_text(), "하세"); } + + #[test] + fn test_cursor_position_for_ime_cjk() { + let mut input = Input::new(); + // "안녕hello" - 2 CJK chars (width 2 each) + 5 ASCII chars + input.insert_str("안녕hello"); + + // Cursor at position 2 (after "안녕", before "h") + input.textarea.move_cursor(CursorMove::Jump(0, 2)); + + let area = Rect::new(0, 0, 80, 5); + let (row, col) = input.textarea.cursor(); + assert_eq!(row, 0); + assert_eq!(col, 2); + + // The visual line should contain the cursor's source row + let visual_lines = input.visual_lines(area.width as usize); + let cursor_vl = visual_lines + .iter() + .skip(input.viewport_top) + .take(area.height as usize) + .enumerate() + .find(|(_, vl)| vl.source_row == row && col >= vl.start_col && col <= vl.end_col); + + assert!( + cursor_vl.is_some(), + "Cursor should be found in a visible visual line" + ); + + let (screen_row, vl) = cursor_vl.unwrap(); + let line = input.textarea.lines().get(row); + let prefix_width: usize = line + .map(|l| { + l.chars() + .take(col) + .map(|c| UnicodeWidthChar::width(c).unwrap_or(0)) + .sum::() + }) + .unwrap_or(0); + + let render_col = prefix_width.saturating_sub(vl.start_col); + + // "안녕" = 2 chars, each width 2 = 4 cells + assert_eq!(prefix_width, 4, "CJK chars '안녕' should be 4 cells wide"); + assert_eq!( + render_col, 4, + "Cursor should be at column 4 (after 2 CJK chars)" + ); + assert_eq!(screen_row, 0, "Cursor should be on the first visible line"); + } + + #[test] + fn test_cursor_position_for_ime_english() { + let mut input = Input::new(); + input.insert_str("Hello World"); + // Cursor at position 6 (after "Hello ") + input.textarea.move_cursor(CursorMove::Jump(0, 6)); + + let area = Rect::new(0, 0, 80, 5); + + let visual_lines = input.visual_lines(area.width as usize); + let (row, col) = input.textarea.cursor(); + assert_eq!(col, 6); + + let cursor_vl = visual_lines + .iter() + .skip(input.viewport_top) + .take(area.height as usize) + .enumerate() + .find(|(_, vl)| vl.source_row == row && col >= vl.start_col && col <= vl.end_col); + + assert!(cursor_vl.is_some()); + let (_, _) = cursor_vl.unwrap(); // Verify it was found + } } From b61e5d52126c67dfbebfeb17df856635157df700 Mon Sep 17 00:00:00 2001 From: visitorise <91079853+visitorise@users.noreply.github.com> Date: Wed, 5 Aug 2026 06:41:38 +0900 Subject: [PATCH 3/3] Fix right-arrow cursor jump at CJK wrapped line boundaries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the cursor sat at the end of a wrapped visual line (cursor_col == vl.end_col) for CJK input, set_terminal_cursor_position matched that cursor to the *current* visual line instead of the next one. The terminal caret was therefore placed at the end of the first visual line while the orange cursor had already advanced to the second, requiring two Right presses to move forward one cell. Reuse the boundary logic from cursor_visual_row, whose condition (cursor_col == vl.end_col && cursor_col == line_len) correctly assigns the boundary cursor to the following visual line — matching the behavior already used for Up/Down navigation and orange cursor rendering. --- src/ui/components/input.rs | 161 +++++++++++++++++++------------------ 1 file changed, 84 insertions(+), 77 deletions(-) diff --git a/src/ui/components/input.rs b/src/ui/components/input.rs index c976ad3..3861d81 100644 --- a/src/ui/components/input.rs +++ b/src/ui/components/input.rs @@ -1438,44 +1438,32 @@ impl Input { return; } - let (cursor_row, cursor_col) = self.textarea.cursor(); - let wrap_width = area.width as usize; - let visual_lines = self.visual_lines(wrap_width); + let visual_lines = self.visual_lines(area.width as usize); - // Find the visual line containing the cursor's source row, then compute - // the rendered column accounting for wrapping and character widths. - let cursor_vl = visual_lines - .iter() - .skip(self.viewport_top) - .take(area.height as usize) - .enumerate() - .find(|(_, vl)| { - vl.source_row == cursor_row - && cursor_col >= vl.start_col - && cursor_col <= vl.end_col - }); + let Some(visual_idx) = self.cursor_visual_row(&visual_lines) else { + return; + }; - if let Some((screen_row, vl)) = cursor_vl { - let line = self.textarea.lines().get(cursor_row); - let prefix_width = line - .map(|l| { - l.chars() - .take(cursor_col) - .map(|c| UnicodeWidthChar::width(c).unwrap_or(0)) - .sum::() - }) - .unwrap_or(0); + if visual_idx < self.viewport_top || visual_idx >= self.viewport_top + area.height as usize + { + return; + } - // Account for horizontal scrolling if present - let render_col = prefix_width.saturating_sub(vl.start_col); - let cursor_x = area.x + render_col.min(area.width.saturating_sub(1) as usize) as u16; - let cursor_y = area.y + screen_row as u16; + let vl = &visual_lines[visual_idx]; + let screen_row = visual_idx - self.viewport_top; - frame.set_cursor_position(ratatui::layout::Position { - x: cursor_x, - y: cursor_y, - }); - } + // Width of chars from this visual line's start to the cursor. The + // caret sits on the wrapped row, so only the suffix of the source + // line matters — prefix_width - start_col mixes cell widths with + // char indices and drifts right by start_col cells on CJK (width 2). + let render_col = self.cursor_display_col(vl); + let cursor_x = area.x + render_col.min(area.width.saturating_sub(1) as usize) as u16; + let cursor_y = area.y + screen_row as u16; + + frame.set_cursor_position(ratatui::layout::Position { + x: cursor_x, + y: cursor_y, + }); } fn render_visual_line( @@ -3106,53 +3094,79 @@ mod tests { } #[test] - fn test_cursor_position_for_ime_cjk() { + fn test_cursor_position_for_ime_cjk_wrapped() { let mut input = Input::new(); - // "안녕hello" - 2 CJK chars (width 2 each) + 5 ASCII chars - input.insert_str("안녕hello"); + // 16 CJK chars (width 2 each) = 32 cells; at wrap width 10 they wrap + // every 5 chars: vl0 = chars 0..5, vl1 = chars 5..10, ... + input.insert_str("你好世界你好世界你好世界你好世界"); - // Cursor at position 2 (after "안녕", before "h") - input.textarea.move_cursor(CursorMove::Jump(0, 2)); + // Cursor at char index 7 → second wrapped row + input.textarea.move_cursor(CursorMove::Jump(0, 7)); - let area = Rect::new(0, 0, 80, 5); + let area = Rect::new(0, 0, 10, 5); // narrow input forces wrapping let (row, col) = input.textarea.cursor(); - assert_eq!(row, 0); - assert_eq!(col, 2); + assert_eq!((row, col), (0, 7)); - // The visual line should contain the cursor's source row let visual_lines = input.visual_lines(area.width as usize); - let cursor_vl = visual_lines - .iter() - .skip(input.viewport_top) - .take(area.height as usize) - .enumerate() - .find(|(_, vl)| vl.source_row == row && col >= vl.start_col && col <= vl.end_col); - + let visual_idx = input.cursor_visual_row(&visual_lines); + assert!( + visual_idx.is_some(), + "Cursor should be found in visual lines" + ); + let visual_idx = visual_idx.unwrap(); assert!( - cursor_vl.is_some(), - "Cursor should be found in a visible visual line" + visual_idx >= input.viewport_top + && visual_idx < input.viewport_top + area.height as usize, + "Cursor should be in the visible viewport" ); + let vl = &visual_lines[visual_idx]; + let screen_row = visual_idx - input.viewport_top; - let (screen_row, vl) = cursor_vl.unwrap(); - let line = input.textarea.lines().get(row); - let prefix_width: usize = line - .map(|l| { - l.chars() - .take(col) - .map(|c| UnicodeWidthChar::width(c).unwrap_or(0)) - .sum::() - }) - .unwrap_or(0); + assert_eq!(vl.start_col, 5, "Second visual line starts at char 5"); + assert_eq!(screen_row, 1, "Cursor should be on the second wrapped row"); + + // Width of chars 5..7 = two CJK chars = 4 cells. The buggy formula + // (prefix_width - start_col = 14 - 5 = 9) would drift 5 cells right. + let render_col = input.cursor_display_col(vl); + assert_eq!(render_col, 4, "Suffix width from wrap start to cursor"); + } + + #[test] + fn test_cursor_position_at_visual_line_boundary() { + let mut input = Input::new(); + // 10 CJK chars (width 2 each) = 20 cells; at wrap width 10 they wrap + // every 5 chars: vl0 = chars 0..5, vl1 = chars 5..10 + input.insert_str("你好世界你好世界你好世界你好世界"); - let render_col = prefix_width.saturating_sub(vl.start_col); + // Cursor at char index 5 — exactly at the vl0/vl1 boundary + input.textarea.move_cursor(CursorMove::Jump(0, 5)); - // "안녕" = 2 chars, each width 2 = 4 cells - assert_eq!(prefix_width, 4, "CJK chars '안녕' should be 4 cells wide"); + let area = Rect::new(0, 0, 10, 5); + + let visual_lines = input.visual_lines(area.width as usize); + let visual_idx = input.cursor_visual_row(&visual_lines); + assert!( + visual_idx.is_some(), + "Cursor should be found in visual lines" + ); + let visual_idx = visual_idx.unwrap(); + + let vl = &visual_lines[visual_idx]; + // The old `cursor_col <= vl.end_col` condition would match vl0 + // (end_col=5, 5 <= 5), placing the caret at cell 10 (clamped to 9) + // on row 0. The correct behavior is vl1 (start_col=5, end_col=10). + assert_eq!(vl.start_col, 5, "Cursor at char 5 belongs to vl1, not vl0"); + + // cursor_display_col should return 0 (width of chars 5..5 = empty) + let render_col = input.cursor_display_col(vl); assert_eq!( - render_col, 4, - "Cursor should be at column 4 (after 2 CJK chars)" + render_col, 0, + "Caret should be at cell 0 of the second visual line" ); - assert_eq!(screen_row, 0, "Cursor should be on the first visible line"); + + // Simulate what set_terminal_cursor_position does: + let screen_row = visual_idx - input.viewport_top; + assert_eq!(screen_row, 1, "Cursor should be on the second wrapped row"); } #[test] @@ -3168,14 +3182,7 @@ mod tests { let (row, col) = input.textarea.cursor(); assert_eq!(col, 6); - let cursor_vl = visual_lines - .iter() - .skip(input.viewport_top) - .take(area.height as usize) - .enumerate() - .find(|(_, vl)| vl.source_row == row && col >= vl.start_col && col <= vl.end_col); - - assert!(cursor_vl.is_some()); - let (_, _) = cursor_vl.unwrap(); // Verify it was found + let visual_idx = input.cursor_visual_row(&visual_lines); + assert!(visual_idx.is_some()); } }