From ac9c0274f44a12bd329da4f528323f22d5b6f2df Mon Sep 17 00:00:00 2001 From: Aram Drevekenin Date: Mon, 14 Dec 2020 16:25:46 +0100 Subject: [PATCH] fix(compatibility): fix vim overwrite message and buffer clearing on exit (#98) * fix(compatibility): fix vim overwrite message and buffer clearing on exit * style(formatting): make rustfmt happy --- src/terminal_pane/scroll.rs | 95 +++++++++++++----- src/terminal_pane/terminal_pane.rs | 9 ++ src/tests/fixtures/clear_scroll_region | Bin 0 -> 5240 bytes src/tests/fixtures/vim_overwrite | Bin 0 -> 4154 bytes src/tests/integration/compatibility.rs | 57 +++++++++++ ..._compatibility__clear_scroll_region-2.snap | 32 ++++++ ...n__compatibility__clear_scroll_region.snap | 32 ++++++ ...ation__compatibility__vim_overwrite-2.snap | 32 ++++++ ...gration__compatibility__vim_overwrite.snap | 32 ++++++ 9 files changed, 265 insertions(+), 24 deletions(-) create mode 100644 src/tests/fixtures/clear_scroll_region create mode 100644 src/tests/fixtures/vim_overwrite create mode 100644 src/tests/integration/snapshots/mosaic__tests__integration__compatibility__clear_scroll_region-2.snap create mode 100644 src/tests/integration/snapshots/mosaic__tests__integration__compatibility__clear_scroll_region.snap create mode 100644 src/tests/integration/snapshots/mosaic__tests__integration__compatibility__vim_overwrite-2.snap create mode 100644 src/tests/integration/snapshots/mosaic__tests__integration__compatibility__vim_overwrite.snap diff --git a/src/terminal_pane/scroll.rs b/src/terminal_pane/scroll.rs index e9801ef0..b6f66be9 100644 --- a/src/terminal_pane/scroll.rs +++ b/src/terminal_pane/scroll.rs @@ -248,7 +248,7 @@ impl Debug for WrappedFragment { } } -#[derive(Debug)] +#[derive(Debug, Clone, Copy)] pub struct CursorPosition { line_index: (usize, usize), // (canonical line index, fragment index in line) column_index: usize, // 0 is the first character from the pane edge @@ -312,6 +312,8 @@ pub struct Scroll { viewport_bottom_offset: Option, scroll_region: Option<(usize, usize)>, // start line, end line (if set, this is the area the will scroll) show_cursor: bool, + lines_outside_of_scroll_region: Option>, + cursor_position_outside_of_scroll_region: Option, } impl Scroll { @@ -327,6 +329,8 @@ impl Scroll { viewport_bottom_offset: None, scroll_region: None, show_cursor: true, + lines_outside_of_scroll_region: None, + cursor_position_outside_of_scroll_region: None, } } pub fn as_character_lines(&self) -> Vec> { @@ -393,7 +397,9 @@ impl Scroll { } pub fn add_canonical_line(&mut self) { let current_canonical_line_index = self.cursor_position.line_index.0; - if let Some((scroll_region_top, scroll_region_bottom)) = self.scroll_region { + if let Some((scroll_region_top, scroll_region_bottom)) = + self.scroll_region_absolute_indices() + { if current_canonical_line_index == scroll_region_bottom { // end of scroll region // when we have a scroll region set and we're at its bottom @@ -500,17 +506,15 @@ impl Scroll { self.cursor_position.move_up_by_canonical_lines(count); } pub fn change_size(&mut self, columns: usize, lines: usize) { - if self.scroll_region.is_none() { - for canonical_line in self.canonical_lines.iter_mut() { - canonical_line.change_width(columns); - } - let cursor_line = self - .canonical_lines - .get(self.cursor_position.line_index.0) - .expect("cursor out of bounds"); - if cursor_line.wrapped_fragments.len() <= self.cursor_position.line_index.1 { - self.cursor_position.line_index.1 = cursor_line.wrapped_fragments.len() - 1; - } + for canonical_line in self.canonical_lines.iter_mut() { + canonical_line.change_width(columns); + } + let cursor_line = self + .canonical_lines + .get(self.cursor_position.line_index.0) + .expect("cursor out of bounds"); + if cursor_line.wrapped_fragments.len() <= self.cursor_position.line_index.1 { + self.cursor_position.line_index.1 = cursor_line.wrapped_fragments.len() - 1; } self.lines_in_view = lines; self.total_columns = columns; @@ -597,13 +601,18 @@ impl Scroll { self.cursor_position.reset(); } pub fn move_cursor_to(&mut self, line: usize, col: usize) { - if self.canonical_lines.len() > line { - self.cursor_position.move_to_canonical_line(line); + let line_on_screen = if self.canonical_lines.len() > self.lines_in_view { + line + (self.canonical_lines.len() - self.lines_in_view) } else { - for _ in self.canonical_lines.len()..=line { + line + }; + if self.canonical_lines.len() > line_on_screen { + self.cursor_position.move_to_canonical_line(line_on_screen); + } else { + for _ in self.canonical_lines.len()..=line_on_screen { self.canonical_lines.push(CanonicalLine::new()); } - self.cursor_position.move_to_canonical_line(line); + self.cursor_position.move_to_canonical_line(line_on_screen); } let (current_canonical_line_index, current_line_wrap_position) = self.cursor_position.line_index; @@ -630,14 +639,48 @@ impl Scroll { self.move_cursor_to(line, current_column); } pub fn set_scroll_region(&mut self, top_line: usize, bottom_line: usize) { + if self.scroll_region.is_none() { + self.lines_outside_of_scroll_region = Some(self.canonical_lines.drain(..).collect()); + self.cursor_position_outside_of_scroll_region = Some(self.cursor_position); + } self.scroll_region = Some((top_line, bottom_line)); - // TODO: clear linewraps in scroll region? } pub fn clear_scroll_region(&mut self) { - self.scroll_region = None; + if let Some(scroll_region) = self.scroll_region_absolute_indices() { + self.canonical_lines.drain(scroll_region.0..scroll_region.1); + self.cursor_position.reset(); + self.scroll_region = None; + } + if let Some(lines_outside_of_scroll_region) = self.lines_outside_of_scroll_region.as_mut() { + self.canonical_lines = lines_outside_of_scroll_region.drain(..).collect(); + } + if let Some(cursor_position_outside_of_scroll_region) = + self.cursor_position_outside_of_scroll_region + { + self.cursor_position = cursor_position_outside_of_scroll_region; + } + self.lines_outside_of_scroll_region = None; + self.cursor_position_outside_of_scroll_region = None; + } + pub fn set_scroll_region_to_screen_size(&mut self) { + self.scroll_region = Some((0, self.lines_in_view - 1)); // these are indices + } + fn scroll_region_absolute_indices(&mut self) -> Option<(usize, usize)> { + if self.scroll_region.is_none() { + return None; + }; + if self.canonical_lines.len() > self.lines_in_view { + let absolute_top = self.canonical_lines.len() - 1 - self.lines_in_view; + let absolute_bottom = self.canonical_lines.len() - 1; + Some((absolute_top, absolute_bottom)) + } else { + Some((self.scroll_region.unwrap().0, self.scroll_region.unwrap().1)) + } } pub fn delete_lines_in_scroll_region(&mut self, count: usize) { - if let Some((scroll_region_top, scroll_region_bottom)) = self.scroll_region { + if let Some((scroll_region_top, scroll_region_bottom)) = + self.scroll_region_absolute_indices() + { let current_canonical_line_index = self.cursor_position.line_index.0; if current_canonical_line_index >= scroll_region_top && current_canonical_line_index <= scroll_region_bottom @@ -655,7 +698,9 @@ impl Scroll { } } pub fn add_empty_lines_in_scroll_region(&mut self, count: usize) { - if let Some((scroll_region_top, scroll_region_bottom)) = self.scroll_region { + if let Some((scroll_region_top, scroll_region_bottom)) = + self.scroll_region_absolute_indices() + { let current_canonical_line_index = self.cursor_position.line_index.0; if current_canonical_line_index >= scroll_region_top && current_canonical_line_index <= scroll_region_bottom @@ -673,7 +718,9 @@ impl Scroll { } } pub fn move_cursor_up_in_scroll_region(&mut self, count: usize) { - if let Some((scroll_region_top, scroll_region_bottom)) = self.scroll_region { + if let Some((scroll_region_top, scroll_region_bottom)) = + self.scroll_region_absolute_indices() + { // the scroll region indices start at 1, so we need to adjust them for _ in 0..count { let current_canonical_line_index = self.cursor_position.line_index.0; @@ -694,7 +741,7 @@ impl Scroll { /// [scroll_up](https://github.com/alacritty/alacritty/blob/ec42b42ce601808070462111c0c28edb0e89babb/alacritty_terminal/src/grid/mod.rs#L261) /// This function takes the first line of the scroll region and moves it to the bottom (count times) pub fn rotate_scroll_region_up(&mut self, count: usize) { - if let Some((_, scroll_region_bottom)) = self.scroll_region { + if let Some((_, scroll_region_bottom)) = self.scroll_region_absolute_indices() { if self.show_cursor { let scroll_region_bottom_index = scroll_region_bottom - 1; self.cursor_position @@ -714,7 +761,7 @@ impl Scroll { /// [scroll_down](https://github.com/alacritty/alacritty/blob/ec42b42ce601808070462111c0c28edb0e89babb/alacritty_terminal/src/grid/mod.rs#L221) /// This function takes the last line of the scroll region and moves it to the top (count times) pub fn rotate_scroll_region_down(&mut self, count: usize) { - if let Some((scroll_region_top, _)) = self.scroll_region { + if let Some((scroll_region_top, _)) = self.scroll_region_absolute_indices() { if self.show_cursor { let scroll_region_top_index = scroll_region_top - 1; self.cursor_position diff --git a/src/terminal_pane/terminal_pane.rs b/src/terminal_pane/terminal_pane.rs index a6b4b346..6754974c 100644 --- a/src/terminal_pane/terminal_pane.rs +++ b/src/terminal_pane/terminal_pane.rs @@ -443,6 +443,15 @@ impl vte::Perform for TerminalPane { (params[0] as usize - 1, params[1] as usize - 1) } }; + if params.len() >= 1 && params[0] == 0 { + // this is a hack + // + // the logic should *probably* be: + // if we get an instruction to move outside the scroll region + // (which is 1 indexed, so if we get 0 it's always(?) outside) + // we need to set it to screen size + self.scroll.set_scroll_region_to_screen_size(); + } self.scroll.move_cursor_to(row, col); } else if c == 'A' { // move cursor up until edge of screen diff --git a/src/tests/fixtures/clear_scroll_region b/src/tests/fixtures/clear_scroll_region new file mode 100644 index 0000000000000000000000000000000000000000..d47413645f04189165abbdef1112f8c86945b704 GIT binary patch literal 5240 zcmd5=&1xe@5cbWBK~FH3W+AYFHJa+4AIU>jB+6lvT_=!3au7ZUtsD)gC1Pt$0wE8O zKyu47lJAc`)K5&XJ6Dmt;_xWUw>A8=J;0!lbcR7diBF}K7rk4wKTkn z=VmpVn(Lc#x_C4H#gvQH^kzI+m3LEfJDbkuowJi@G_Y};%ub^BS~;a)B4J80fB(nt zFKP;EUY!4}JE^IwpDY(<$8_KphNyQcvIatqG*Z_ZgQCL2HN;3s~vREqro*UCyV`D5tf_?>A!Kms?|h zfT1%c$F7ZHgLjgi4WED=@^>&{JNog?2QdV!kus8)YL|H~$08{Havfh8N2Y%)^%xaq zHD23C5U`!I+B=PRLw?OUz3vbShv`ZOa}Fp@`m<>CA}K7EGN4)X**aUX(8Bg^xYq)L z`y5GO3%d#pag*aE>-g5k4S!WchOU?B{h9|$`Mq92FQ0CRdTNsR^>gEhc;yz)?*INT z8f8dT{$ao`p@m_S=!)WEaEgYwsr$n`M4ljoa^fHr)}?-4E~bVNN4lgTyp%O1N$L%3 zX+wg6cH>VQKvn}e0dg8B5uii^r2>>{piF==4U`K|u7P?2)YCwH0qScYirXlPRTK%7 z$~g(rNhO^G>!h+yf_74AC&4?ZyptfFRN_f6Pb%{ys3(E6O+*`rDRk^o_+2u zz>}nbXV8SlTyGcU&*oS2c6EK;L%YC^Y_)6JUNbGMG{%0i1KF(ICc!|ufQCj_wrH02 z412C-$NH%`H{XAC$r{~Yf>fu_KNJA${OUS@l9HjMv@1m=_2e!$B?qvUa(2#OFyE;C z?k3l{6yy(<0w9ibwatu1saOy)rCt%@+!%zhF^E!Q5QN4czl}lq8iVXKy3koE0(!DZ z5zv!Oih!DIPKbambWVtXE_6(ZuFZ9L}0*BbBtWHhCwiHe2d;(ozmZDh^9Nn_-v2c~tNAMCx(fGF;-! zj_QZ0TtS?!I9XvE+1Q?XF0+O>*133B(OkuKi0}~WA?ic?hfH`V%U%3XmNVi=m-Gka z?e&|-r+E@1JoW`eo=Pj4(``E{9EPIo-LQ~i+;SoNL;DX-WykLOS9b}u8S9V%XKp)Y zSn=91!X1YPNb+ESFycs;bWQMN(k!=?-W0<@ZD;IqC6oYpyGiiVy1sVKIFYx9xTx#z zPbZyq39iqIeCG39)hFYdAD27Nj40{*0XY9vM?$DTbu!#OAhw_N!hr}n5srgw5Izgx ubU2`SgwHy>QwV2KtN@#Ya5%-&N(w)g&tJ9GANR*B)D-@r0iSPu%=#ZvagYW8 literal 0 HcmV?d00001 diff --git a/src/tests/fixtures/vim_overwrite b/src/tests/fixtures/vim_overwrite new file mode 100644 index 0000000000000000000000000000000000000000..3a804d64abb1557b67b2549cb5ec10b6f474082a GIT binary patch literal 4154 zcmchaUvAq*5XSwMAb?K5mvw*w4iZ6}At_NN8Y5MV+KuBDuF*UQeo$pPBA}%LiMDGX z=mA=wed`(e))VwHK1sjfACX+IQ^O7-7+QJtX=e7fv$I3z#Es69)lBtf3mx|+%Zcty z7pHMgFIJOe`p6l3VH8BJcE;M-`lkGsy@9^|^N(Uqefd`h&Q-@5zj+tWr-)l+3ni;~ zp|V-5&X!4>p3bjSl4kL8GR=~USgmI9eBODo;f#0Kb^Y0fbFYFev?e|uRX zD7kq0*WR>5SAMfdRRvA!Ns^Y+*MI+m`8Uou^heeAzN?A0z5cfE7AmZe+;D_}{n5q+ z#NM660aSOEJ}%u4b<2Udy)*BSS5 zROKH9zJw8hO`Q{Rqn$?>NQQDg_d}!!h0t`ImkRa*Jx|hDiQ)^r%s^x*6%s$t3bC{$ zd4Rm$PtHYb6M2Tnvx$5|W$qJqf@?<4X270ohCxbm%*|Wi(t?b!g&sO$q zuxBfKHrTV3Jsa%V%AO7OY-P_4_Fi7trR5-ZsMX>3M{i#p80l7>>_F;YS#zLW*SK5c zzF*7xs0j~wou|n!>Z3Z(&blGS6dhT1o7=Efo7fX=VH2ijLeek-7)H)p$DwSV~I?$NR8cH7mmxJ5je zL=Q)i*j|_Vg5nK5wr`KHQ8F^*GJ4Kfwsp_PeIsLlt5?Q8dK4K4&Uh$e2q&(57#1I- zmWyLtOtgn?kx<@AtP{+Wm?zjLu}^S7;(*|R!~?+vi30-Og#LJCNp=xv^9ib>$C}*>1E&2of?$y!Z z{^7UV>R29d&{?b5WTk$N<5W#&lk|Ojs#Zxljny)qoF?gem1LWnn{D8^8XS=U8Gr(p5qy!Jhc@M&Re>oC-V*p zfIwDn&j0md4=Ho^`E8pWsMMhrZI<znWle5V0^WcJ^jTzu_IHKHrjuRnh+d Dq%_t` literal 0 HcmV?d00001 diff --git a/src/tests/integration/compatibility.rs b/src/tests/integration/compatibility.rs index 114c10d2..1df46583 100644 --- a/src/tests/integration/compatibility.rs +++ b/src/tests/integration/compatibility.rs @@ -260,3 +260,60 @@ pub fn htop_right_scrolling() { assert_snapshot!(snapshot); } } + +#[test] +pub fn vim_overwrite() { + // this tests the vim overwrite message + // to recreate: + // * open a file in vim + // * open the same file in another window + // * change the file in the other window and save + // * change the file in the original vim window and save + // * confirm you would like to change the file by pressing 'y' and then ENTER + // * if everything looks fine, this test passed :) + let fake_win_size = PositionAndSize { + columns: 116, + rows: 28, + x: 0, + y: 0, + }; + let fixture_name = "vim_overwrite"; + let mut fake_input_output = get_fake_os_input(&fake_win_size, fixture_name); + fake_input_output.add_terminal_input(&[&COMMAND_TOGGLE, &COMMAND_TOGGLE, &QUIT]); + start(Box::new(fake_input_output.clone()), Opt::default()); + let output_frames = fake_input_output + .stdout_writer + .output_frames + .lock() + .unwrap(); + let snapshots = get_output_frame_snapshots(&output_frames, &fake_win_size); + for snapshot in snapshots { + assert_snapshot!(snapshot); + } +} + +#[test] +pub fn clear_scroll_region() { + // this tests the scroll region used by eg. vim is cleared properly + // this means that when vim exits, we get back the previous scroll + // buffer + let fake_win_size = PositionAndSize { + columns: 116, + rows: 28, + x: 0, + y: 0, + }; + let fixture_name = "clear_scroll_region"; + let mut fake_input_output = get_fake_os_input(&fake_win_size, fixture_name); + fake_input_output.add_terminal_input(&[&COMMAND_TOGGLE, &COMMAND_TOGGLE, &QUIT]); + start(Box::new(fake_input_output.clone()), Opt::default()); + let output_frames = fake_input_output + .stdout_writer + .output_frames + .lock() + .unwrap(); + let snapshots = get_output_frame_snapshots(&output_frames, &fake_win_size); + for snapshot in snapshots { + assert_snapshot!(snapshot); + } +} diff --git a/src/tests/integration/snapshots/mosaic__tests__integration__compatibility__clear_scroll_region-2.snap b/src/tests/integration/snapshots/mosaic__tests__integration__compatibility__clear_scroll_region-2.snap new file mode 100644 index 00000000..8c309266 --- /dev/null +++ b/src/tests/integration/snapshots/mosaic__tests__integration__compatibility__clear_scroll_region-2.snap @@ -0,0 +1,32 @@ +--- +source: src/tests/integration/compatibility.rs +expression: snapshot +--- +⋊> ~/c/mosaic on main ⨯ vim some-file 15:07:22 +⋊> ~/c/mosaic on main ⨯ 15:07:29 + + + + + + + + + + + + + + + + + + + + + + + + + +Bye from Mosaic!█ diff --git a/src/tests/integration/snapshots/mosaic__tests__integration__compatibility__clear_scroll_region.snap b/src/tests/integration/snapshots/mosaic__tests__integration__compatibility__clear_scroll_region.snap new file mode 100644 index 00000000..3a7b2d5e --- /dev/null +++ b/src/tests/integration/snapshots/mosaic__tests__integration__compatibility__clear_scroll_region.snap @@ -0,0 +1,32 @@ +--- +source: src/tests/integration/compatibility.rs +expression: snapshot +--- +Welcome to fish, the friendly interactive shell +⋊> ~/c/mosaic on main ⨯ vim some-file 15:07:22 +⋊> ~/c/mosaic on main ⨯ █ 15:07:29 + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/tests/integration/snapshots/mosaic__tests__integration__compatibility__vim_overwrite-2.snap b/src/tests/integration/snapshots/mosaic__tests__integration__compatibility__vim_overwrite-2.snap new file mode 100644 index 00000000..7edfc74d --- /dev/null +++ b/src/tests/integration/snapshots/mosaic__tests__integration__compatibility__vim_overwrite-2.snap @@ -0,0 +1,32 @@ +--- +source: src/tests/integration/compatibility.rs +expression: snapshot +--- + 1 line 1 + 2 line 2 + 3 line 3 + 4 line 4 + 5 line 5 + 6 line 6 + 7 line 7 + 8 line 8 + 9 line 9 + 10 line 10 + 11 line 11 + 12 line 12 + 13 line 13 + 14 line 14 + 15 line 15 + 16 line 16 + 17 line 17 + 18 line 18 + 19 line 19 + 20 line 20 + 21 line 21 + 22 line 22 + 23 line 23 + 24 line 24 + 25 line 25 + NORMAL some-file unix | utf-8 | no ft 1% 1:1 + +Bye from Mosaic!█ diff --git a/src/tests/integration/snapshots/mosaic__tests__integration__compatibility__vim_overwrite.snap b/src/tests/integration/snapshots/mosaic__tests__integration__compatibility__vim_overwrite.snap new file mode 100644 index 00000000..406b5b31 --- /dev/null +++ b/src/tests/integration/snapshots/mosaic__tests__integration__compatibility__vim_overwrite.snap @@ -0,0 +1,32 @@ +--- +source: src/tests/integration/compatibility.rs +expression: snapshot +--- +1 █ + 1 line 1 + 2 line 2 + 3 line 3 + 4 line 4 + 5 line 5 + 6 line 6 + 7 line 7 + 8 line 8 + 9 line 9 + 10 line 10 + 11 line 11 + 12 line 12 + 13 line 13 + 14 line 14 + 15 line 15 + 16 line 16 + 17 line 17 + 18 line 18 + 19 line 19 + 20 line 20 + 21 line 21 + 22 line 22 + 23 line 23 + 24 line 24 + 25 line 25 + NORMAL some-file unix | utf-8 | no ft 1% 1:1 +