From 79b32d46ac81e7578e21319afcbf6105b8cb8c9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Tobias=20Christ?= Date: Thu, 4 Apr 2024 13:12:42 +0200 Subject: [PATCH] fix: Fix prompt for multi-byte characters. (#41) Before this contribution, entering characters that consists of multiple bytes lead to undesired behaviour such as panics. This results from the position previous position handling inside the prompt. The String::len() returns the size of the string in bytes. Since a single unicode scalar value may consist of multiple bytes, this is not useful to determine the position inside the prompt. The panic occured in the State::push() method and resulted from the attempt to insert at a byte index that does not represent a character boundary. See: https://doc.rust-lang.org/stable/std/primitive.str.html#method.len https://doc.rust-lang.org/stable/std/string/struct.String.html#method.insert Co-authored-by: Josh McKinney --- src/prompt.rs | 34 ++++++++++++++++++++++++++++------ src/text_prompt.rs | 6 +++--- src/text_state.rs | 26 +++++++++++++++++++++++++- 3 files changed, 56 insertions(+), 10 deletions(-) diff --git a/src/prompt.rs b/src/prompt.rs index 5483724..c343061 100644 --- a/src/prompt.rs +++ b/src/prompt.rs @@ -1,5 +1,8 @@ +use std::iter::once; + use crate::Status; use crossterm::event::{KeyCode, KeyEvent, KeyEventKind, KeyModifiers}; +use itertools::chain; use ratatui::{prelude::*, widgets::StatefulWidget}; /// A prompt that can be drawn to a terminal. @@ -81,6 +84,14 @@ pub trait State { /// A mutable reference to the value of the prompt. fn value_mut(&mut self) -> &mut String; + fn len(&self) -> usize { + self.value().chars().count() + } + + fn is_empty(&self) -> bool { + self.value().len() == 0 + } + fn handle_key_event(&mut self, key_event: KeyEvent) { if key_event.kind == KeyEventKind::Release { return; @@ -114,7 +125,7 @@ pub trait State { fn delete(&mut self) { let position = self.position(); - if position == self.value().len() { + if position == self.len() { return; } self.value_mut().remove(position); @@ -122,7 +133,7 @@ pub trait State { fn backspace(&mut self) { let position = self.position().saturating_sub(1); - if position == self.value().len() { + if position == self.len() { return; } *self.position_mut() = position; @@ -130,7 +141,7 @@ pub trait State { } fn move_right(&mut self) { - if self.position() == self.value().len() { + if self.position() == self.len() { return; } *self.position_mut() = self.position().saturating_add(1); @@ -141,7 +152,7 @@ pub trait State { } fn move_end(&mut self) { - *self.position_mut() = self.value().len(); + *self.position_mut() = self.len(); } fn move_start(&mut self) { @@ -159,8 +170,19 @@ pub trait State { } fn push(&mut self, c: char) { - let position = self.position(); - self.value_mut().insert(position, c); + if self.position() == self.len() { + self.value_mut().push(c); + } else { + // We cannot use String::insert() as it operates on bytes, which can lead to incorrect modifications with + // multibyte characters. Instead, we handle text manipulation at the character level using Rust's char type + // for Unicode correctness. Check docs of String::insert() and String::chars() for futher info. + *self.value_mut() = chain![ + self.value().chars().take(self.position()), + once(c), + self.value().chars().skip(self.position()) + ] + .collect(); + } *self.position_mut() = self.position().saturating_add(1); } } diff --git a/src/text_prompt.rs b/src/text_prompt.rs index fd7c8cf..77a2ddc 100644 --- a/src/text_prompt.rs +++ b/src/text_prompt.rs @@ -38,7 +38,7 @@ impl TextRenderStyle { pub fn render(&self, state: &TextState) -> String { match self { Self::Default => state.value().to_string(), - Self::Password => "*".repeat(state.value().len()), + Self::Password => "*".repeat(state.len()), Self::Invisible => String::new(), } } @@ -91,7 +91,7 @@ impl<'a> StatefulWidget for TextPrompt<'a> { let width = area.width as usize; let height = area.height as usize; let value = self.render_style.render(state); - let value_length = value.len(); + let value_length = value.chars().count(); let line = Line::from(vec![ state.status().symbol(), @@ -100,7 +100,7 @@ impl<'a> StatefulWidget for TextPrompt<'a> { " › ".cyan().dim(), Span::raw(value), ]); - let prompt_length = line.width() - value_length; + let prompt_length = line.to_string().chars().count() - value_length; let lines = wrap(line, width).take(height).collect_vec(); // constrain the position to the area diff --git a/src/text_state.rs b/src/text_state.rs index 4835137..08d0abf 100644 --- a/src/text_state.rs +++ b/src/text_state.rs @@ -90,4 +90,28 @@ impl State for TextState<'_> { } #[cfg(test)] -mod tests {} +mod tests { + use crate::{State, TextState}; + + #[test] + fn insert_multibyte_start() { + let mut test = TextState::new().with_value("ää"); + test.move_start(); + test.push('Ö'); + assert_eq!(test.value(), "Öää"); + } + #[test] + fn insert_multibyte_middle() { + let mut test = TextState::new().with_value("ää"); + test.move_right(); + test.push('Ö'); + assert_eq!(test.value(), "äÖä"); + } + #[test] + fn insert_multibyte_end() { + let mut test = TextState::new().with_value("ää"); + test.move_end(); + test.push('Ö'); + assert_eq!(test.value(), "ääÖ"); + } +}