fix(compatibility): replace wide characters under cursor properly (#1196)

* fix(compatibility): replace wide characters under cursor properly

* style(fmt): rustfmt
This commit is contained in:
Aram Drevekenin 2022-03-09 11:04:07 +01:00 committed by GitHub
parent ed3316491c
commit 95e512c36d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 212 additions and 22 deletions

5
src/tests/fixtures/ncmpcpp-wide-chars vendored Normal file

File diff suppressed because one or more lines are too long

View file

@ -0,0 +1 @@
👨🔭👨🔭xy

View file

@ -0,0 +1 @@
👨x🔭🔭👨

View file

@ -947,6 +947,7 @@ impl Grid {
row.insert_character_at(terminal_character, self.cursor.x);
} else {
row.add_character_at(terminal_character, self.cursor.x);
// self.move_cursor_forward_until_edge(count_to_move_cursor);
}
self.output_buffer.update_line(self.cursor.y);
}
@ -967,6 +968,9 @@ impl Grid {
pub fn add_character(&mut self, terminal_character: TerminalCharacter) {
// TODO: try to separate adding characters from moving the cursors in this function
let character_width = terminal_character.width;
if character_width == 0 {
return;
}
if self.cursor.x + character_width > self.width {
if self.disable_linewrap {
return;
@ -1060,7 +1064,7 @@ impl Grid {
fn pad_current_line_until(&mut self, position: usize) {
let current_row = self.viewport.get_mut(self.cursor.y).unwrap();
for _ in current_row.len()..position {
for _ in current_row.width()..position {
current_row.push(EMPTY_TERMINAL_CHARACTER);
}
self.output_buffer.update_line(self.cursor.y);
@ -2175,14 +2179,34 @@ impl Row {
}
absolute_index
}
pub fn absolute_character_index_and_position_in_char(&self, x: usize) -> (usize, usize) {
// returns x's width aware index as well as its position inside the wide char (eg. 1 if
// it's in the middle of a 2-char wide character)
let mut accumulated_width = 0;
let mut absolute_index = x;
let mut position_inside_character = 0;
for (i, terminal_character) in self.columns.iter().enumerate() {
accumulated_width += terminal_character.width;
absolute_index = i;
if accumulated_width > x {
let character_start_position = accumulated_width - terminal_character.width;
position_inside_character = x - character_start_position;
break;
}
}
(absolute_index, position_inside_character)
}
pub fn add_character_at(&mut self, terminal_character: TerminalCharacter, x: usize) {
match self.width_cached().cmp(&x) {
Ordering::Equal => {
// adding the character at the end of the current line
self.columns.push_back(terminal_character);
// this is unwrapped because this always happens after self.width_cached()
*self.width.as_mut().unwrap() += terminal_character.width;
}
Ordering::Less => {
// adding the character after the end of the current line
// we pad the line up to the character and then add it
let width_offset = self.excess_width_until(x);
self.columns
.resize(x.saturating_sub(width_offset), EMPTY_TERMINAL_CHARACTER);
@ -2190,25 +2214,38 @@ impl Row {
self.width = None;
}
Ordering::Greater => {
// wide-character-aware index, where each character is counted once
let absolute_x_index = self.absolute_character_index(x);
// adding the character in the middle of the line
// we replace the character at its position
let (absolute_x_index, position_inside_character) =
self.absolute_character_index_and_position_in_char(x);
let character_width = terminal_character.width;
let replaced_character =
std::mem::replace(&mut self.columns[absolute_x_index], terminal_character);
match character_width.cmp(&replaced_character.width) {
Ordering::Greater => {
// this is done in a verbose manner because of performance
let width_difference = character_width - replaced_character.width;
for _ in 0..width_difference {
// the replaced character is narrower than the current character
// (eg. we added a wide emoji in place of an English character)
// we remove the character after it to make room
let position_to_remove = absolute_x_index + 1;
if self.columns.get(position_to_remove).is_some() {
self.columns.remove(position_to_remove);
if let Some(removed) = self.columns.remove(position_to_remove) {
if removed.width > 1 {
// the character we removed is a wide character itself, so we add
// padding
self.columns
.insert(position_to_remove, EMPTY_TERMINAL_CHARACTER);
}
}
}
Ordering::Less => {
let width_difference = replaced_character.width - character_width;
for _ in 0..width_difference {
// the replaced character is wider than the current character
// (eg. we added an English character in place of a wide emoji)
// we must make sure to add padding either before the character we added
// or after it, depending on our position inside said removed wide character
// TODO: support characters wider than 2
if position_inside_character > 0 {
self.columns
.insert(absolute_x_index, EMPTY_TERMINAL_CHARACTER);
} else {
self.columns
.insert(absolute_x_index + 1, EMPTY_TERMINAL_CHARACTER);
}
@ -2235,14 +2272,14 @@ impl Row {
self.width = None;
}
pub fn replace_character_at(&mut self, terminal_character: TerminalCharacter, x: usize) {
// this is much more performant than remove/insert
if x < self.columns.len() {
let absolute_x_index = self.absolute_character_index(x);
if absolute_x_index < self.columns.len() {
self.columns.push_back(terminal_character);
// let character = self.columns.swap_remove_back(x);
let character = self.columns.swap_remove_back(x).unwrap(); // TODO: if let?
// this is much more performant than remove/insert
let character = self.columns.swap_remove_back(absolute_x_index).unwrap();
let excess_width = character.width.saturating_sub(1);
for _ in 0..excess_width {
self.columns.insert(x, terminal_character);
self.columns.insert(absolute_x_index, terminal_character);
}
}
self.width = None;

View file

@ -1560,3 +1560,45 @@ pub fn fzf_fullscreen() {
}
assert_snapshot!(format!("{:?}", grid));
}
#[test]
pub fn replace_multiple_wide_characters_under_cursor() {
// this test makes sure that if we replace a wide character with a non-wide character, it
// properly pads the excess width in the proper place (either before the replaced non-wide
// character if the cursor was "in the middle" of the wide character, or after the character if
// it was "in the beginning" of the wide character)
let mut vte_parser = vte::Parser::new();
let mut grid = Grid::new(
51,
112,
Palette::default(),
Rc::new(RefCell::new(LinkHandler::new())),
);
let fixture_name = "replace_multiple_wide_characters";
let content = read_fixture(fixture_name);
for byte in content {
vte_parser.advance(&mut grid, byte);
}
assert_snapshot!(format!("{:?}", grid));
}
#[test]
pub fn replace_non_wide_characters_with_wide_characters() {
// this test makes sure that if we replace a wide character with a non-wide character, it
// properly pads the excess width in the proper place (either before the replaced non-wide
// character if the cursor was "in the middle" of the wide character, or after the character if
// it was "in the beginning" of the wide character)
let mut vte_parser = vte::Parser::new();
let mut grid = Grid::new(
51,
112,
Palette::default(),
Rc::new(RefCell::new(LinkHandler::new())),
);
let fixture_name = "replace_non_wide_characters_with_wide_characters";
let content = read_fixture(fixture_name);
for byte in content {
vte_parser.advance(&mut grid, byte);
}
assert_snapshot!(format!("{:?}", grid));
}

View file

@ -0,0 +1,8 @@
---
source: zellij-server/src/panes/./unit/grid_tests.rs
expression: "format!(\"{:?}\", grid)"
---
00 (C): 👨y x🔭
01 (C):

View file

@ -0,0 +1,8 @@
---
source: zellij-server/src/panes/./unit/grid_tests.rs
expression: "format!(\"{:?}\", grid)"
---
00 (C): 👨👨 🔭
01 (C):

View file

@ -8,23 +8,23 @@ expression: "format!(\"{:?}\", grid)"
02 (C): Character set B (US ASCII)
03 (C): !"#$%&'()*+,-./0123456789:;<=>? !"#$%&'()*+,-./0123456789:;<=>?
04 (C): @ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_ @ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_
05 (C): `abcdefghijklmnopqrstuvwxyz{|}~ `abcdefghijklmnopqrstuvwxyz{|}~
05 (C): `abcdefghijklmnopqrstuvwxyz{|}~ `abcdefghijklmnopqrstuvwxyz{|}~
06 (C): Character set A (British)
07 (C): !"#$%&'()*+,-./0123456789:;<=>? !"#$%&'()*+,-./0123456789:;<=>?
08 (C): @ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_ @ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_
09 (C): `abcdefghijklmnopqrstuvwxyz{|}~ `abcdefghijklmnopqrstuvwxyz{|}~
09 (C): `abcdefghijklmnopqrstuvwxyz{|}~ `abcdefghijklmnopqrstuvwxyz{|}~
10 (C): Character set 0 (DEC Special graphics and line drawing)
11 (C): !"#$%&'()*+,-./0123456789:;<=>? !"#$%&'()*+,-./0123456789:;<=>?
12 (C): @ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_ @ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_
13 (C): ◆▒␉␌␍␊°±␤␋┘┐┌└┼⎺⎻─⎼⎽├┤┴┬│≤≥π≠£· ◆▒␉␌␍␊°±␤␋┘┐┌└┼⎺⎻─⎼⎽├┤┴┬│≤≥π≠£·
13 (C): ◆▒␉␌␍␊°±␤␋┘┐┌└┼⎺⎻─⎼⎽├┤┴┬│≤≥π≠£· ◆▒␉␌␍␊°±␤␋┘┐┌└┼⎺⎻─⎼⎽├┤┴┬│≤≥π≠£·
14 (C): Character set 1 (DEC Alternate character ROM standard characters)
15 (C): !"#$%&'()*+,-./0123456789:;<=>? !"#$%&'()*+,-./0123456789:;<=>?
16 (C): @ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_ @ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_
17 (C): `abcdefghijklmnopqrstuvwxyz{|}~ `abcdefghijklmnopqrstuvwxyz{|}~
17 (C): `abcdefghijklmnopqrstuvwxyz{|}~ `abcdefghijklmnopqrstuvwxyz{|}~
18 (C): Character set 2 (DEC Alternate character ROM special graphics)
19 (C): !"#$%&'()*+,-./0123456789:;<=>? !"#$%&'()*+,-./0123456789:;<=>?
20 (C): @ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_ @ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_
21 (C): `abcdefghijklmnopqrstuvwxyz{|}~ `abcdefghijklmnopqrstuvwxyz{|}~
21 (C): `abcdefghijklmnopqrstuvwxyz{|}~ `abcdefghijklmnopqrstuvwxyz{|}~
22 (C):
23 (C): These are the installed character sets. Push <RETURN>
24 (C):

View file

@ -0,0 +1,54 @@
---
source: zellij-server/src/tab/./unit/tab_integration_tests.rs
expression: snapshot
---
00 (C): ┌ Remixed by BEMANI Sound Team 'TAG feat. PON' - 新宝島 ──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
01 (C): │0:14/1:59 ──┤ 新宝島 ├── Vol: 39%│
02 (C): │[playing] Remixed by BEMANI Sound Team 'TAG feat. PON' - beatmania IIDX 28 BISTROVER Original Soundtrack (Disc2) (2021) [------]│
03 (C): │────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────│
04 (C): │ Playlist (2035 items, length: 3 days, 4 hours, 4 minutes, 28 seconds) │
05 (C): │────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────│
06 (C): │Artist Track Title Album Time │
07 (C): │────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────│
08 (C): │Expander 18 Honey Trap beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 2:08 │
09 (C): │ARM×狐夢想 feat. 山本椛 19 ディッシュウォッシャー◎彡おいわちゃん beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 1:57 │
10 (C): │Gram 20 GuNGNiR beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 2:11 │
11 (C): │maras k(marasy+kors k) 21 Piano Samurai beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 2:09 │
12 (C): │P*Light 22 HAPPY★RUSH beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 2:05 │
13 (C): │猫叉Master 23 月雪に舞う華のように beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 2:19 │
14 (C): │DJ TOTTO 24 DORNWALD Junge beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 2:14 │
15 (C): │USAO 25 BroGamer beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 2:12 │
16 (C): │SOUND HOLIC feat. Nana Takahashi & 709sec. 26 FUZIN RIZIN beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 2:08 │
17 (C): │青龍 27 AO-∞ beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 2:08 │
18 (C): │Manabu Namiki 28 Shoot'Em All beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 2:21 │
19 (C): │Eagle 29 Caterpillar beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 2:06 │
20 (C): │Dirty Androids 30 Egret and Willow beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 2:14 │
21 (C): │DJ CHUCKY feat. Jonjo 31 Warrior beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 2:07 │
22 (C): │DJ Noriken 32 Summerlights(IIDX Edition) beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 2:05 │
23 (C): │TOMOSUKE 33 Macuilxochitl beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 1:50 │
24 (C): │dj Hellix 34 Quakes beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 1:59 │
25 (C): │神楽 35 Mare Nectaris beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 2:11 │
26 (C): │beatnation Records 01 crew beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 2:23 │
27 (C): │kors k vs L.E.D.-G 02 Venom beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 2:10 │
28 (C): │kors k vs Sota Fujimori 03 Cyber True Color beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 2:06 │
29 (C): │あべにゅうぷろじぇくと feat. 佐倉紗織 produced 04 恋はどうモロ◎波動OK☆方程式!! beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 1:55 │
30 (C): │Remo-con 05 Vermilion beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 1:38 │
31 (C): │猫叉Master+ 06 Lost wing at.0 beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 2:08 │
32 (C): │DJ TOTTO 07 童話回廊 beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 1:55 │
33 (C): │Morrigan feat. リリィ 08 Apocalypse beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 2:28 │
34 (C): │kors k 09 Rave Cannon beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 2:09 │
35 (C): │かめりあ 10 yamabiko (SINOBUZ Edition) beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 2:09 │
36 (C): │HuΣeR 11 ECHIDNA beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 2:03 │
37 (C): │Hommarju 12 Beat Juggling Mix beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 2:02 │
38 (C): │dj TAKA 13 RAIN beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 1:59 │
39 (C): │L.E.D. 14 INVISIBLE STRIX beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 2:11 │
40 (C): │金獅子 vs 麒麟 feat. 獏 15 Snakey Kung-fu beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 2:15 │
41 (C): │猫叉Master vs HuΣeR 16 刃図羅 beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 2:15 │
42 (C): │DJ TOTTO 17 DORNWALD Der Junge im Käfig beatmania IIDX 25 CANNON BALLERS ORIGINAL SOUNDTRACK 5:19 │
43 (C): │===========================>────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────│
44 (C): │ │
45 (C): │ │
46 (C): │ │
47 (C): └────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘

View file

@ -129,6 +129,16 @@ fn create_new_tab(size: Size) -> Tab {
tab
}
fn read_fixture(fixture_name: &str) -> Vec<u8> {
let mut path_to_file = std::path::PathBuf::new();
path_to_file.push("../src");
path_to_file.push("tests");
path_to_file.push("fixtures");
path_to_file.push(fixture_name);
std::fs::read(path_to_file)
.unwrap_or_else(|_| panic!("could not read fixture {:?}", &fixture_name))
}
use crate::panes::grid::Grid;
use crate::panes::link_handler::LinkHandler;
use ::insta::assert_snapshot;
@ -1056,3 +1066,27 @@ fn cannot_float_only_embedded_pane() {
);
assert_snapshot!(snapshot);
}
#[test]
fn replacing_existing_wide_characters() {
// this is a real world use case using ncmpcpp with wide characters and scrolling
// the reason we don't break it down is that it exposes quite a few edge cases with wide
// characters that we should handle properly
let size = Size {
cols: 238,
rows: 48,
};
let client_id = 1;
let mut tab = create_new_tab(size);
let mut output = Output::default();
let pane_content = read_fixture("ncmpcpp-wide-chars");
tab.handle_pty_bytes(1, pane_content);
tab.render(&mut output, None);
let snapshot = take_snapshot(
output.serialize().get(&client_id).unwrap(),
size.rows,
size.cols,
Palette::default(),
);
assert_snapshot!(snapshot);
}