From c2dfba27f309158da91054bf2e91b732a77ebe0c Mon Sep 17 00:00:00 2001 From: Wez Furlong Date: Sun, 24 Jul 2022 08:11:11 -0700 Subject: [PATCH] termwiz: don't claim that visible_cells is double-ended It's not; there's important state that only works in forward iteration. --- termwiz/src/surface/line.rs | 83 +++++++-------------------------- wezterm-gui/src/overlay/copy.rs | 3 +- 2 files changed, 18 insertions(+), 68 deletions(-) diff --git a/termwiz/src/surface/line.rs b/termwiz/src/surface/line.rs index 1358d517d..5ece87670 100644 --- a/termwiz/src/surface/line.rs +++ b/termwiz/src/surface/line.rs @@ -883,7 +883,7 @@ impl Line { /// the characters that follow wide characters, the column index may /// skip some positions. It is returned as a convenience to the consumer /// as using .enumerate() on this iterator wouldn't be as useful. - pub fn visible_cells<'a>(&'a self) -> impl DoubleEndedIterator> { + pub fn visible_cells<'a>(&'a self) -> impl Iterator> { match &self.cells { CellStorage::V(cells) => VisibleCellIter::V(CellSliceIter { cells: cells.iter(), @@ -1151,40 +1151,20 @@ struct CellSliceIter<'a> { skip_width: usize, } -impl<'a> CellSliceIter<'a> { - fn advance(&mut self, forwards: bool) -> Option> { - while self.skip_width > 0 { - self.skip_width -= 1; - let _ = if forwards { - self.cells.next() - } else { - self.cells.next_back() - }?; - self.idx += 1; - } - let cell = if forwards { - self.cells.next() - } else { - self.cells.next_back() - }?; - let cell_index = self.idx; - self.idx += 1; - self.skip_width = cell.width().saturating_sub(1); - Some(CellRef::CellRef { cell_index, cell }) - } -} - impl<'a> Iterator for CellSliceIter<'a> { type Item = CellRef<'a>; fn next(&mut self) -> Option> { - self.advance(true) - } -} - -impl<'a> DoubleEndedIterator for CellSliceIter<'a> { - fn next_back(&mut self) -> Option> { - self.advance(false) + while self.skip_width > 0 { + self.skip_width -= 1; + let _ = self.cells.next()?; + self.idx += 1; + } + let cell = self.cells.next()?; + let cell_index = self.idx; + self.idx += 1; + self.skip_width = cell.width().saturating_sub(1); + Some(CellRef::CellRef { cell_index, cell }) } } @@ -1204,15 +1184,6 @@ impl<'a> Iterator for VisibleCellIter<'a> { } } -impl<'a> DoubleEndedIterator for VisibleCellIter<'a> { - fn next_back(&mut self) -> Option> { - match self { - Self::V(iter) => iter.next_back(), - Self::C(iter) => iter.next_back(), - } - } -} - #[derive(Debug)] pub enum CellRef<'a> { CellRef { @@ -1529,13 +1500,11 @@ pub struct ClusterLineCellIter<'a> { line: &'a ClusteredLine, } -impl<'a> ClusterLineCellIter<'a> { - fn advance(&mut self, forwards: bool) -> Option> { - let text = if forwards { - self.graphemes.next()? - } else { - self.graphemes.next_back()? - }; +impl<'a> Iterator for ClusterLineCellIter<'a> { + type Item = CellRef<'a>; + + fn next(&mut self) -> Option> { + let text = self.graphemes.next()?; let cell_index = self.idx; let width = if self.line.is_double_wide(cell_index) { @@ -1548,11 +1517,7 @@ impl<'a> ClusterLineCellIter<'a> { let attrs = &self.cluster.as_ref()?.attrs; if self.cluster_total >= self.cluster.as_ref()?.cell_width { - self.cluster = if forwards { - self.clusters.next() - } else { - self.clusters.next_back() - }; + self.cluster = self.clusters.next(); self.cluster_total = 0; } @@ -1565,20 +1530,6 @@ impl<'a> ClusterLineCellIter<'a> { } } -impl<'a> Iterator for ClusterLineCellIter<'a> { - type Item = CellRef<'a>; - - fn next(&mut self) -> Option> { - self.advance(true) - } -} - -impl<'a> DoubleEndedIterator for ClusterLineCellIter<'a> { - fn next_back(&mut self) -> Option> { - self.advance(false) - } -} - #[cfg(test)] mod test { use super::*; diff --git a/wezterm-gui/src/overlay/copy.rs b/wezterm-gui/src/overlay/copy.rs index 14160dbfa..26d391b1b 100644 --- a/wezterm-gui/src/overlay/copy.rs +++ b/wezterm-gui/src/overlay/copy.rs @@ -577,10 +577,9 @@ impl CopyRenderable { if let Some(line) = lines.get(0) { self.cursor.y = top; self.cursor.x = 0; - for cell in line.visible_cells().rev() { + for cell in line.visible_cells() { if cell.str() != " " { self.cursor.x = cell.cell_index(); - break; } } }