Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

Commit

Permalink
fix: Fix prompt for multi-byte characters. (#41)
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
frederictobiasc and joshka authored Apr 4, 2024
1 parent 0b96532 commit 79b32d4
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 10 deletions.
34 changes: 28 additions & 6 deletions src/prompt.rs
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -114,23 +125,23 @@ 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);
}

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;
self.value_mut().remove(position);
}

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);
Expand All @@ -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) {
Expand All @@ -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);
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/text_prompt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}
}
Expand Down Expand Up @@ -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(),
Expand All @@ -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
Expand Down
26 changes: 25 additions & 1 deletion src/text_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(), "ääÖ");
}
}

0 comments on commit 79b32d4

Please sign in to comment.