preserve all text attributes on cleared cells

some terminals require it (alacritty wants to render underline and
inverse state of cleared cells, for instance, so we have to be sure that
the diff algorithm will properly reset the cursor attributes before
clearing cells)

6 files changed, 90 insertions, 71 deletions

diff --git a/src/attrs.rs b/src/attrs.rs
index 08ec857..eab752c 100644
--- a/src/attrs.rs
+++ b/src/attrs.rs
@@ -35,12 +35,6 @@ pub struct Attrs {
 }
 
 impl Attrs {
-    pub fn clear(&mut self) {
-        self.fgcolor = Color::default();
-        self.bgcolor = Color::default();
-        self.mode = enumset::EnumSet::default();
-    }
-
     pub fn bold(&self) -> bool {
         self.mode.contains(TextMode::Bold)
     }
diff --git a/src/cell.rs b/src/cell.rs
index 4f26cb5..05d93be 100644
--- a/src/cell.rs
+++ b/src/cell.rs
@@ -17,16 +17,10 @@ impl PartialEq<Cell> for Cell {
         if self.len != other.len {
             return false;
         }
-        let len = self.len();
-        if len > 0 {
-            if self.attrs != other.attrs {
-                return false;
-            }
-        } else {
-            if self.attrs.bgcolor != other.attrs.bgcolor {
-                return false;
-            }
+        if self.attrs != other.attrs {
+            return false;
         }
+        let len = self.len();
         self.contents[..len] == other.contents[..len]
     }
 }
@@ -83,10 +77,9 @@ impl Cell {
         self.set_wide(new_contents[0].width().unwrap_or(0) > 1);
     }
 
-    pub(crate) fn clear(&mut self, bgcolor: crate::attrs::Color) {
+    pub(crate) fn clear(&mut self, attrs: crate::attrs::Attrs) {
         self.len = 0;
-        self.attrs.clear();
-        self.attrs.bgcolor = bgcolor;
+        self.attrs = attrs;
     }
 
     /// Returns the text contents of the cell.
diff --git a/src/grid.rs b/src/grid.rs
index 242a152..62c25ab 100644
--- a/src/grid.rs
+++ b/src/grid.rs
@@ -41,7 +41,7 @@ impl Grid {
         self.pos = Pos::default();
         self.saved_pos = Pos::default();
         for row in self.drawing_rows_mut() {
-            row.clear(crate::attrs::Color::Default);
+            row.clear(crate::attrs::Attrs::default());
         }
         self.scroll_top = 0;
         self.scroll_bottom = self.size.rows - 1;
@@ -245,48 +245,48 @@ impl Grid {
         prev_attrs
     }
 
-    pub fn erase_all(&mut self, bgcolor: crate::attrs::Color) {
+    pub fn erase_all(&mut self, attrs: crate::attrs::Attrs) {
         for row in self.drawing_rows_mut() {
-            row.clear(bgcolor);
+            row.clear(attrs);
         }
     }
 
-    pub fn erase_all_forward(&mut self, bgcolor: crate::attrs::Color) {
+    pub fn erase_all_forward(&mut self, attrs: crate::attrs::Attrs) {
         let pos = self.pos;
         for row in self.drawing_rows_mut().skip(pos.row as usize + 1) {
-            row.clear(bgcolor);
+            row.clear(attrs);
         }
 
-        self.erase_row_forward(bgcolor);
+        self.erase_row_forward(attrs);
     }
 
-    pub fn erase_all_backward(&mut self, bgcolor: crate::attrs::Color) {
+    pub fn erase_all_backward(&mut self, attrs: crate::attrs::Attrs) {
         let pos = self.pos;
         for row in self.drawing_rows_mut().take(pos.row as usize) {
-            row.clear(bgcolor);
+            row.clear(attrs);
         }
 
-        self.erase_row_backward(bgcolor);
+        self.erase_row_backward(attrs);
     }
 
-    pub fn erase_row(&mut self, bgcolor: crate::attrs::Color) {
-        self.current_row_mut().clear(bgcolor);
+    pub fn erase_row(&mut self, attrs: crate::attrs::Attrs) {
+        self.current_row_mut().clear(attrs);
     }
 
-    pub fn erase_row_forward(&mut self, bgcolor: crate::attrs::Color) {
+    pub fn erase_row_forward(&mut self, attrs: crate::attrs::Attrs) {
         let pos = self.pos;
         let row = self.current_row_mut();
         row.wrap(false);
         for cell in row.cells_mut().skip(pos.col as usize) {
-            cell.clear(bgcolor);
+            cell.clear(attrs);
         }
     }
 
-    pub fn erase_row_backward(&mut self, bgcolor: crate::attrs::Color) {
+    pub fn erase_row_backward(&mut self, attrs: crate::attrs::Attrs) {
         let pos = self.pos;
         let row = self.current_row_mut();
         for cell in row.cells_mut().take(pos.col as usize + 1) {
-            cell.clear(bgcolor);
+            cell.clear(attrs);
         }
     }
 
@@ -310,13 +310,13 @@ impl Grid {
         row.resize(size.cols as usize, crate::cell::Cell::default());
     }
 
-    pub fn erase_cells(&mut self, count: u16, bgcolor: crate::attrs::Color) {
+    pub fn erase_cells(&mut self, count: u16, attrs: crate::attrs::Attrs) {
         let pos = self.pos;
         let row = self.current_row_mut();
         for cell in
             row.cells_mut().skip(pos.col as usize).take(count as usize)
         {
-            cell.clear(bgcolor);
+            cell.clear(attrs);
         }
     }
 
diff --git a/src/row.rs b/src/row.rs
index 26e117b..f521467 100644
--- a/src/row.rs
+++ b/src/row.rs
@@ -19,9 +19,9 @@ impl Row {
         self.cells.len().try_into().unwrap()
     }
 
-    pub fn clear(&mut self, bgcolor: crate::attrs::Color) {
+    pub fn clear(&mut self, attrs: crate::attrs::Attrs) {
         for cell in &mut self.cells {
-            cell.clear(bgcolor);
+            cell.clear(attrs);
         }
         self.wrapped = false;
     }
@@ -147,21 +147,15 @@ impl Row {
                 }
 
                 let attrs = cell.attrs();
+                if &prev_attrs != attrs {
+                    attrs.write_escape_code_diff(contents, &prev_attrs);
+                    prev_attrs = *attrs;
+                }
 
                 if cell.has_contents() {
-                    if &prev_attrs != attrs {
-                        attrs.write_escape_code_diff(contents, &prev_attrs);
-                        prev_attrs = *attrs;
-                    }
-
                     contents.extend(cell.contents().as_bytes());
                     prev_pos.col += if cell.is_wide() { 2 } else { 1 };
                 } else {
-                    if prev_attrs.bgcolor != attrs.bgcolor {
-                        attrs.write_escape_code_diff(contents, &prev_attrs);
-                        prev_attrs = *attrs;
-                    }
-
                     crate::term::EraseChar::default().write_buf(contents);
                 }
             }
@@ -223,21 +217,15 @@ impl Row {
                 }
 
                 let attrs = cell.attrs();
+                if &prev_attrs != attrs {
+                    attrs.write_escape_code_diff(contents, &prev_attrs);
+                    prev_attrs = *attrs;
+                }
 
                 if cell.has_contents() {
-                    if &prev_attrs != attrs {
-                        attrs.write_escape_code_diff(contents, &prev_attrs);
-                        prev_attrs = *attrs;
-                    }
-
                     contents.extend(cell.contents().as_bytes());
                     prev_pos.col += if cell.is_wide() { 2 } else { 1 };
                 } else {
-                    if prev_attrs.bgcolor != attrs.bgcolor {
-                        attrs.write_escape_code_diff(contents, &prev_attrs);
-                        prev_attrs = *attrs;
-                    }
-
                     crate::term::EraseChar::default().write_buf(contents);
                 }
             }
diff --git a/src/screen.rs b/src/screen.rs
index ce86312..41546fc 100644
--- a/src/screen.rs
+++ b/src/screen.rs
@@ -464,7 +464,7 @@ impl Screen {
     fn text(&mut self, c: char) {
         let pos = self.grid().pos();
         if pos.col > 0 {
-            let bgcolor = self.attrs.bgcolor;
+            let attrs = self.attrs;
             let prev_cell = self
                 .drawing_cell_mut(crate::grid::Pos {
                     row: pos.row,
@@ -472,7 +472,7 @@ impl Screen {
                 })
                 .unwrap();
             if prev_cell.is_wide() {
-                prev_cell.clear(bgcolor);
+                prev_cell.clear(attrs);
             }
         }
 
@@ -512,9 +512,9 @@ impl Screen {
             cell.set(c, attrs);
             self.grid_mut().col_inc(1);
             if width > 1 {
-                let bgcolor = self.attrs.bgcolor;
+                let attrs = self.attrs;
                 let next_cell = self.current_cell_mut();
-                next_cell.clear(bgcolor);
+                next_cell.clear(attrs);
                 self.grid_mut().col_inc(1);
             }
         }
@@ -638,11 +638,11 @@ impl Screen {
 
     // CSI J
     fn ed(&mut self, mode: u16) {
-        let bgcolor = self.attrs.bgcolor;
+        let attrs = self.attrs;
         match mode {
-            0 => self.grid_mut().erase_all_forward(bgcolor),
-            1 => self.grid_mut().erase_all_backward(bgcolor),
-            2 => self.grid_mut().erase_all(bgcolor),
+            0 => self.grid_mut().erase_all_forward(attrs),
+            1 => self.grid_mut().erase_all_backward(attrs),
+            2 => self.grid_mut().erase_all(attrs),
             _ => {}
         }
     }
@@ -654,11 +654,11 @@ impl Screen {
 
     // CSI K
     fn el(&mut self, mode: u16) {
-        let bgcolor = self.attrs.bgcolor;
+        let attrs = self.attrs;
         match mode {
-            0 => self.grid_mut().erase_row_forward(bgcolor),
-            1 => self.grid_mut().erase_row_backward(bgcolor),
-            2 => self.grid_mut().erase_row(bgcolor),
+            0 => self.grid_mut().erase_row_forward(attrs),
+            1 => self.grid_mut().erase_row_backward(attrs),
+            2 => self.grid_mut().erase_row(attrs),
             _ => {}
         }
     }
@@ -695,8 +695,8 @@ impl Screen {
 
     // CSI X
     fn ech(&mut self, count: u16) {
-        let bgcolor = self.attrs.bgcolor;
-        self.grid_mut().erase_cells(count, bgcolor);
+        let attrs = self.attrs;
+        self.grid_mut().erase_cells(count, attrs);
     }
 
     // CSI d
diff --git a/tests/attr.rs b/tests/attr.rs
index bd5aac9..a784388 100644
--- a/tests/attr.rs
+++ b/tests/attr.rs
@@ -171,6 +171,30 @@ fn colors() {
         parser.screen().cell(0, 1).unwrap().bgcolor(),
         vt100::Color::Idx(15)
     );
+
+    // make sure bgcolor is properly preserved on cleared cells
+    parser.process(b"\x1bcfoo");
+
+    assert_eq!(
+        parser.screen().cell(0, 1).unwrap().bgcolor(),
+        vt100::Color::Default
+    );
+    parser.process(b"\x1b[1;2H\x1b[41mo\x1b[m");
+    assert_eq!(
+        parser.screen().cell(0, 1).unwrap().bgcolor(),
+        vt100::Color::Idx(1)
+    );
+
+    assert_eq!(
+        parser.screen().cell(0, 0).unwrap().bgcolor(),
+        vt100::Color::Default
+    );
+    parser.process(b"\x1b[1;1H\x1b[41m\x1b[X\x1b[m");
+    assert_eq!(
+        parser.screen().cell(0, 0).unwrap().bgcolor(),
+        vt100::Color::Idx(1)
+    );
+    assert!(!parser.screen().cell(0, 0).unwrap().has_contents());
 }
 
 #[test]
@@ -222,4 +246,24 @@ fn attrs() {
     assert!(parser.screen().cell(0, 3).unwrap().italic());
     assert!(parser.screen().cell(0, 3).unwrap().underline());
     assert!(parser.screen().cell(0, 3).unwrap().inverse());
+
+    // alacritty renders underline and inverse status for empty cells, so make
+    // sure we reflect that here (so that we generate diffs correctly and
+    // such). unclear who is right here - other terminals don't do this, but
+    // terminals do generally render bgcolor for empty cells, which feels
+    // similar.
+    parser.process(b"\x1bcfoo");
+
+    assert!(!parser.screen().cell(0, 1).unwrap().underline());
+    assert!(!parser.screen().cell(0, 1).unwrap().inverse());
+    parser.process(b"\x1b[1;2H\x1b[4;7mo\x1b[m");
+    assert!(parser.screen().cell(0, 1).unwrap().underline());
+    assert!(parser.screen().cell(0, 1).unwrap().inverse());
+
+    assert!(!parser.screen().cell(0, 0).unwrap().underline());
+    assert!(!parser.screen().cell(0, 0).unwrap().inverse());
+    parser.process(b"\x1b[1;1H\x1b[4;7m\x1b[X\x1b[m");
+    assert!(parser.screen().cell(0, 0).unwrap().underline());
+    assert!(parser.screen().cell(0, 0).unwrap().inverse());
+    assert!(!parser.screen().cell(0, 0).unwrap().has_contents());
 }