fix a couple more issues with end of line behavior

7 files changed, 221 insertions, 22 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 09ead23..ea84420 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+### Fixed
+
+* Fix a couple more end-of-line/wrapping bugs.
+
 ## [0.7.0] - 2019-11-23
 
 ### Added
diff --git a/src/grid.rs b/src/grid.rs
index 8076a6c..9619443 100644
--- a/src/grid.rs
+++ b/src/grid.rs
@@ -228,9 +228,55 @@ impl Grid {
                     col: self.size.cols - 1,
                 };
             }
-            crate::term::MoveFromTo::new(prev_pos, pos).write_buf(contents);
             let cell = self.visible_cell(pos).unwrap();
-            contents.extend(cell.contents().as_bytes());
+            if cell.has_contents() {
+                crate::term::MoveFromTo::new(prev_pos, pos)
+                    .write_buf(contents);
+                contents.extend(cell.contents().as_bytes());
+            } else {
+                // if the cell doesn't have contents, we can't have gotten
+                // here by drawing a character in the last column. this means
+                // that as far as i'm aware, we have to have reached here from
+                // a newline when we were already after the end of an earlier
+                // row. in the case where we are already after the end of an
+                // earlier row, we can just write a few newlines, otherwise we
+                // also need to do the same as above to get ourselves to after
+                // the end of a row.
+                let orig_row = pos.row;
+                let mut found = false;
+                for i in (0..orig_row).rev() {
+                    pos.row = i;
+                    let cell = self.visible_cell(pos).unwrap();
+                    if cell.has_contents() {
+                        if prev_pos.row != i || prev_pos.col < self.size.cols
+                        {
+                            crate::term::MoveFromTo::new(prev_pos, pos)
+                                .write_buf(contents);
+                            contents.extend(cell.contents().as_bytes());
+                        }
+                        contents.extend(
+                            "\n".repeat((orig_row - i) as usize).as_bytes(),
+                        );
+                        found = true;
+                        break;
+                    }
+                }
+
+                // this can happen if you get the cursor off the end of a row,
+                // and then do something to clear the current row without
+                // moving the cursor (IL, ED, EL, etc). i can't see any way
+                // for this to happen without the entire current row being
+                // cleared (not just the last cell), so this seems
+                // prooooobably fine?
+                if !found {
+                    pos.row = orig_row;
+                    crate::term::MoveFromTo::new(prev_pos, pos)
+                        .write_buf(contents);
+                    contents.push(b' ');
+                    crate::term::ClearRowBackward::default()
+                        .write_buf(contents);
+                }
+            }
         } else {
             crate::term::MoveFromTo::new(prev_pos, self.pos)
                 .write_buf(contents);
@@ -282,9 +328,55 @@ impl Grid {
                     col: self.size.cols - 1,
                 };
             }
-            crate::term::MoveFromTo::new(prev_pos, pos).write_buf(contents);
             let cell = self.visible_cell(pos).unwrap();
-            contents.extend(cell.contents().as_bytes());
+            if cell.has_contents() {
+                crate::term::MoveFromTo::new(prev_pos, pos)
+                    .write_buf(contents);
+                contents.extend(cell.contents().as_bytes());
+            } else {
+                // if the cell doesn't have contents, we can't have gotten
+                // here by drawing a character in the last column. this means
+                // that as far as i'm aware, we have to have reached here from
+                // a newline when we were already after the end of an earlier
+                // row. in the case where we are already after the end of an
+                // earlier row, we can just write a few newlines, otherwise we
+                // also need to do the same as above to get ourselves to after
+                // the end of a row.
+                let orig_row = pos.row;
+                let mut found = false;
+                for i in (0..orig_row).rev() {
+                    pos.row = i;
+                    let cell = self.visible_cell(pos).unwrap();
+                    if cell.has_contents() {
+                        if prev_pos.row != i || prev_pos.col < self.size.cols
+                        {
+                            crate::term::MoveFromTo::new(prev_pos, pos)
+                                .write_buf(contents);
+                            contents.extend(cell.contents().as_bytes());
+                        }
+                        contents.extend(
+                            "\n".repeat((orig_row - i) as usize).as_bytes(),
+                        );
+                        found = true;
+                        break;
+                    }
+                }
+
+                // this can happen if you get the cursor off the end of a row,
+                // and then do something to clear the current row without
+                // moving the cursor (IL, ED, EL, etc). i can't see any way
+                // for this to happen without the entire current row being
+                // cleared (not just the last cell), so this seems
+                // prooooobably fine?
+                if !found {
+                    pos.row = orig_row;
+                    crate::term::MoveFromTo::new(prev_pos, pos)
+                        .write_buf(contents);
+                    contents.push(b' ');
+                    crate::term::ClearRowBackward::default()
+                        .write_buf(contents);
+                }
+            }
         } else {
             crate::term::MoveFromTo::new(prev_pos, self.pos)
                 .write_buf(contents);
@@ -495,9 +587,9 @@ impl Grid {
         self.col_clamp();
     }
 
-    pub fn col_wrap(&mut self, width: u16) {
+    pub fn col_wrap(&mut self, width: u16, wrap: bool) {
         if self.pos.col > self.size.cols - width {
-            self.current_row_mut().wrap(true);
+            self.current_row_mut().wrap(wrap);
             self.pos.col = 0;
             self.row_inc_scroll(1);
         }
diff --git a/src/screen.rs b/src/screen.rs
index ec4d204..07363b1 100644
--- a/src/screen.rs
+++ b/src/screen.rs
@@ -588,19 +588,45 @@ impl Screen {
             }
         }
 
+        let mut wrap = true;
+        // it doesn't make any sense to wrap if the last column in a row
+        // didn't already have contents
+        if pos.col > 1 {
+            let mut prev_cell = self
+                .drawing_cell_mut(crate::grid::Pos {
+                    row: pos.row,
+                    col: pos.col - 2,
+                })
+                .unwrap();
+            if !prev_cell.is_wide() {
+                prev_cell = self
+                    .drawing_cell_mut(crate::grid::Pos {
+                        row: pos.row,
+                        col: pos.col - 1,
+                    })
+                    .unwrap();
+            }
+            if !prev_cell.has_contents() {
+                wrap = false;
+            }
+        }
+
         let width = c.width().unwrap_or(0).try_into().unwrap();
         let attrs = self.attrs;
 
-        // zero width characters still cause the cursor to wrap - this doesn't
-        // affect which cell they go into (the "previous cell" for both (row,
-        // max_col + 1) and (row + 1, 0) is (row, max_col)), but does affect
-        // further movement afterwards - writing an `a` at (row, max_col)
-        // followed by a crlf puts the cursor at (row + 1, 0), but writing a
-        // `à` (specifically `a` followed by a combining grave accent - the
-        // normalized U+00E0 "latin small letter a with grave" behaves the
-        // same as `a`) at (row, max_col) followed by a crlf puts the cursor
-        // at (row + 2, 0)
-        self.grid_mut().col_wrap(if width == 0 { 1 } else { width });
+        self.grid_mut().col_wrap(
+            // zero width characters still cause the cursor to wrap - this
+            // doesn't affect which cell they go into (the "previous cell" for
+            // both (row, max_col + 1) and (row + 1, 0) is (row, max_col)),
+            // but does affect further movement afterwards - writing an `a` at
+            // (row, max_col) followed by a crlf puts the cursor at (row + 1,
+            // 0), but writing a `à` (specifically `a` followed by a combining
+            // grave accent - the normalized U+00E0 "latin small letter a with
+            // grave" behaves the same as `a`) at (row, max_col) followed by a
+            // crlf puts the cursor at (row + 2, 0)
+            if width == 0 { 1 } else { width },
+            wrap,
+        );
 
         if width == 0 {
             if pos.col > 0 {
diff --git a/src/term.rs b/src/term.rs
index d2f7c9b..5a9fbd6 100644
--- a/src/term.rs
+++ b/src/term.rs
@@ -26,6 +26,16 @@ impl BufWrite for ClearRowForward {
 
 #[derive(Default, Debug)]
 #[must_use = "this struct does nothing unless you call write_buf"]
+pub struct ClearRowBackward;
+
+impl BufWrite for ClearRowBackward {
+    fn write_buf(&self, buf: &mut Vec<u8>) {
+        buf.extend_from_slice(b"\x1b[1K");
+    }
+}
+
+#[derive(Default, Debug)]
+#[must_use = "this struct does nothing unless you call write_buf"]
 pub struct CRLF;
 
 impl BufWrite for CRLF {
diff --git a/tests/control.rs b/tests/control.rs
index e0dc276..74e8d9c 100644
--- a/tests/control.rs
+++ b/tests/control.rs
@@ -97,6 +97,21 @@ fn lf_with(b: u8) {
     assert_eq!(parser.screen().cell(1, 5).unwrap().contents(), "r");
     assert_eq!(parser.screen().cell(1, 6).unwrap().contents(), "");
     assert_eq!(parser.screen().contents(), "foo\n   bar");
+
+    parser.process(b"\x1b[H\x1b[J\x1b[4;80H");
+    assert_eq!(parser.screen().cursor_position(), (3, 79));
+    parser.process(b"a");
+    assert_eq!(parser.screen().cursor_position(), (3, 80));
+
+    // note: this is a behavior that terminals disagree on - xterm and urxvt
+    // would leave the cursor at (4, 79) here, but alacritty, tmux, and screen
+    // put it at (4, 80). in general, i'm aiming for roughly tmux/screen
+    // compat where possible, so that's what i'm going with here.
+    parser.process(&[b]);
+    assert_eq!(parser.screen().cursor_position(), (4, 80));
+
+    parser.process(b"b");
+    assert_eq!(parser.screen().cursor_position(), (5, 1));
 }
 
 #[test]
diff --git a/tests/text.rs b/tests/text.rs
index 1ad16ae..4206925 100644
--- a/tests/text.rs
+++ b/tests/text.rs
@@ -221,6 +221,17 @@ fn wrap() {
     assert_eq!(parser.screen().cell(1, 1).unwrap().contents(), "");
     assert_eq!(parser.screen().cell(1, 2).unwrap().contents(), "a");
     assert_eq!(parser.screen().cell(1, 3).unwrap().contents(), "");
+
+    parser.process(b"\x1b[H\x1b[J");
+    assert_eq!(parser.screen().contents(), "");
+    parser.process(b"                                                                                ");
+    assert_eq!(parser.screen().contents(), "                                                                                ");
+    parser.process(b"\n");
+    assert_eq!(parser.screen().contents(), "                                                                                ");
+    parser.process(b"\n");
+    assert_eq!(parser.screen().contents(), "                                                                                ");
+    parser.process(b" ");
+    assert_eq!(parser.screen().contents(), "                                                                                \n\n\n ");
 }
 
 #[test]
diff --git a/tests/window_contents.rs b/tests/window_contents.rs
index 34f5a66..352ffd5 100644
--- a/tests/window_contents.rs
+++ b/tests/window_contents.rs
@@ -80,33 +80,72 @@ fn empty_cells() {
 #[test]
 fn cursor_positioning() {
     let mut parser = vt100::Parser::default();
-    let screen1 = parser.screen().clone();
 
+    let screen = parser.screen().clone();
     parser.process(b":\x1b[K");
-    let screen2 = parser.screen().clone();
     assert_eq!(parser.screen().cursor_position(), (0, 1));
     assert_eq!(
         parser.screen().contents_formatted(),
         b"\x1b[?25h\x1b[m\x1b[H\x1b[J:"
     );
-    assert_eq!(parser.screen().contents_diff(&screen1), b":");
+    assert_eq!(parser.screen().contents_diff(&screen), b":");
 
+    let screen = parser.screen().clone();
     parser.process(b"a");
-    let screen3 = parser.screen().clone();
     assert_eq!(parser.screen().cursor_position(), (0, 2));
     assert_eq!(
         parser.screen().contents_formatted(),
         b"\x1b[?25h\x1b[m\x1b[H\x1b[J:a"
     );
-    assert_eq!(parser.screen().contents_diff(&screen2), b"a");
+    assert_eq!(parser.screen().contents_diff(&screen), b"a");
 
+    let screen = parser.screen().clone();
     parser.process(b"\x1b[1;2H\x1b[K");
     assert_eq!(parser.screen().cursor_position(), (0, 1));
     assert_eq!(
         parser.screen().contents_formatted(),
         b"\x1b[?25h\x1b[m\x1b[H\x1b[J:"
     );
-    assert_eq!(parser.screen().contents_diff(&screen3), b"\x1b[1;2H\x1b[K");
+    assert_eq!(parser.screen().contents_diff(&screen), b"\x1b[1;2H\x1b[K");
+
+    let screen = parser.screen().clone();
+    parser.process(b"\x1b[H\x1b[J\x1b[4;80H");
+    assert_eq!(parser.screen().cursor_position(), (3, 79));
+    assert_eq!(
+        parser.screen().contents_formatted(),
+        b"\x1b[?25h\x1b[m\x1b[H\x1b[J\x1b[4;80H"
+    );
+    assert_eq!(
+        parser.screen().contents_diff(&screen),
+        b"\x1b[H\x1b[K\x1b[4;80H"
+    );
+
+    let screen = parser.screen().clone();
+    parser.process(b"a");
+    assert_eq!(parser.screen().cursor_position(), (3, 80));
+    assert_eq!(
+        parser.screen().contents_formatted(),
+        b"\x1b[?25h\x1b[m\x1b[H\x1b[J\x1b[4;80Ha"
+    );
+    assert_eq!(parser.screen().contents_diff(&screen), b"a");
+
+    let screen = parser.screen().clone();
+    parser.process(b"\n");
+    assert_eq!(parser.screen().cursor_position(), (4, 80));
+    assert_eq!(
+        parser.screen().contents_formatted(),
+        b"\x1b[?25h\x1b[m\x1b[H\x1b[J\x1b[4;80Ha\n"
+    );
+    assert_eq!(parser.screen().contents_diff(&screen), b"\n");
+
+    let screen = parser.screen().clone();
+    parser.process(b"b");
+    assert_eq!(parser.screen().cursor_position(), (5, 1));
+    assert_eq!(
+        parser.screen().contents_formatted(),
+        b"\x1b[?25h\x1b[m\x1b[H\x1b[J\x1b[4;80Ha\x1b[6;1Hb"
+    );
+    assert_eq!(parser.screen().contents_diff(&screen), b"\r\nb");
 }
 
 #[test]