From 634c6bf2b6318aa660b19aa9adffefa253d00fe9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Christ?= Date: Wed, 3 Apr 2024 21:52:23 +0200 Subject: [PATCH] fix: Fix prompt for multi-byte characters. 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 --- 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 d53e208..5ed44ab 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(), "ääÖ"); + } +}