Fix bugs in diff hunk highlighting (#18454)

Fixes https://github.com/zed-industries/zed/issues/18405

In https://github.com/zed-industries/zed/pull/18313, we introduced a
problem where git addition highlights might spuriously return when
undoing certain changes. It turned out, there were already some cases
where git hunk highlighting was incorrect when editing at the boundaries
of expanded diff hunks.

In this PR, I've introduced a test helper method for more rigorously
(and readably) testing the editor's git state. You can assert about the
entire state of an editor's diff decorations using a formatted diff:

```rust
    cx.assert_diff_hunks(
        r#"
        - use some::mod1;
          use some::mod2;
          const A: u32 = 42;
        - const B: u32 = 42;
          const C: u32 = 42;
          fn main() {
        -     println!("hello");
        +     //println!("hello");
              println!("world");
        +     //
        +     //
          }
          fn another() {
              println!("another");
        +     println!("another");
          }
        - fn another2() {
              println!("another2");
          }
        "#
        .unindent(),
    );
```

This will assert about the editor's actual row highlights, not just the
editor's internal hunk-tracking state.

I rewrote all of our editor diff tests to use these more high-level
assertions, and it caught the new bug, as well as some pre-existing bugs
in the highlighting of added content.

The problem was how we *remove* highlighted rows. Previously, it relied
on supplying exactly the same range as one that we had previously
highlighted. I've added a `remove_highlighted_rows(ranges)` APIs which
is much simpler - it clears out any row ranges that intersect the given
ranges (which is all that we need for the Git diff use case).

Release Notes:

- N/A

Cargo.lock

@@ -3729,6 +3729,7 @@ dependencies = [
  "multi_buffer",
  "ordered-float 2.10.1",
  "parking_lot",
+ "pretty_assertions",
  "project",
  "rand 0.8.5",
  "release_channel",

crates/assistant/src/inline_assistant.rs

@@ -1142,7 +1142,7 @@ impl InlineAssistant {
             for row_range in inserted_row_ranges {
                 editor.highlight_rows::<InlineAssist>(
                     row_range,
-                    Some(cx.theme().status().info_background),
+                    cx.theme().status().info_background,
                     false,
                     cx,
                 );
@@ -1209,7 +1209,7 @@ impl InlineAssistant {
                     editor.set_show_inline_completions(Some(false), cx);
                     editor.highlight_rows::<DeletedLines>(
                         Anchor::min()..=Anchor::max(),
-                        Some(cx.theme().status().deleted_background),
+                        cx.theme().status().deleted_background,
                         false,
                         cx,
                     );

crates/editor/Cargo.toml

@@ -24,7 +24,8 @@ test-support = [
     "workspace/test-support",
     "tree-sitter-rust",
     "tree-sitter-typescript",
-    "tree-sitter-html"
+    "tree-sitter-html",
+    "unindent",
 ]
 
 [dependencies]
@@ -54,6 +55,7 @@ markdown.workspace = true
 multi_buffer.workspace = true
 ordered-float.workspace = true
 parking_lot.workspace = true
+pretty_assertions.workspace = true
 project.workspace = true
 rand.workspace = true
 rpc.workspace = true
@@ -74,6 +76,7 @@ theme.workspace = true
 tree-sitter-html = { workspace = true, optional = true }
 tree-sitter-rust = { workspace = true, optional = true }
 tree-sitter-typescript = { workspace = true, optional = true }
+unindent = { workspace = true, optional = true }
 ui.workspace = true
 url.workspace = true
 util.workspace = true

crates/editor/src/editor.rs

@@ -822,7 +822,7 @@ impl SelectionHistory {
 struct RowHighlight {
     index: usize,
     range: RangeInclusive<Anchor>,
-    color: Option<Hsla>,
+    color: Hsla,
     should_autoscroll: bool,
 }
 
@@ -11500,41 +11500,125 @@ impl Editor {
         }
     }
 
-    /// Adds or removes (on `None` color) a highlight for the rows corresponding to the anchor range given.
+    /// Adds a row highlight for the given range. If a row has multiple highlights, the
-    /// On matching anchor range, replaces the old highlight; does not clear the other existing highlights.
+    /// last highlight added will be used.
-    /// If multiple anchor ranges will produce highlights for the same row, the last range added will be used.
     pub fn highlight_rows<T: 'static>(
         &mut self,
-        rows: RangeInclusive<Anchor>,
+        range: RangeInclusive<Anchor>,
-        color: Option<Hsla>,
+        color: Hsla,
         should_autoscroll: bool,
         cx: &mut ViewContext<Self>,
     ) {
         let snapshot = self.buffer().read(cx).snapshot(cx);
         let row_highlights = self.highlighted_rows.entry(TypeId::of::<T>()).or_default();
-        let existing_highlight_index = row_highlights.binary_search_by(|highlight| {
+        let ix = row_highlights.binary_search_by(|highlight| {
-            highlight
+            Ordering::Equal
-                .range
+                .then_with(|| highlight.range.start().cmp(&range.start(), &snapshot))
-                .start()
+                .then_with(|| highlight.range.end().cmp(&range.end(), &snapshot))
-                .cmp(rows.start(), &snapshot)
-                .then(highlight.range.end().cmp(rows.end(), &snapshot))
         });
-        match (color, existing_highlight_index) {
+
-            (Some(_), Ok(ix)) | (_, Err(ix)) => row_highlights.insert(
+        if let Err(mut ix) = ix {
-                ix,
+            let index = post_inc(&mut self.highlight_order);
-                RowHighlight {
+
-                    index: post_inc(&mut self.highlight_order),
+            // If this range intersects with the preceding highlight, then merge it with
-                    range: rows,
+            // the preceding highlight. Otherwise insert a new highlight.
-                    should_autoscroll,
+            let mut merged = false;
-                    color,
+            if ix > 0 {
-                },
+                let prev_highlight = &mut row_highlights[ix - 1];
-            ),
+                if prev_highlight
-            (None, Ok(i)) => {
+                    .range
-                row_highlights.remove(i);
+                    .end()
+                    .cmp(&range.start(), &snapshot)
+                    .is_ge()
+                {
+                    ix -= 1;
+                    if prev_highlight
+                        .range
+                        .end()
+                        .cmp(&range.end(), &snapshot)
+                        .is_lt()
+                    {
+                        prev_highlight.range = *prev_highlight.range.start()..=*range.end();
+                    }
+                    merged = true;
+                    prev_highlight.index = index;
+                    prev_highlight.color = color;
+                    prev_highlight.should_autoscroll = should_autoscroll;
+                }
+            }
+
+            if !merged {
+                row_highlights.insert(
+                    ix,
+                    RowHighlight {
+                        range: range.clone(),
+                        index,
+                        color,
+                        should_autoscroll,
+                    },
+                );
+            }
+
+            // If any of the following highlights intersect with this one, merge them.
+            while let Some(next_highlight) = row_highlights.get(ix + 1) {
+                let highlight = &row_highlights[ix];
+                if next_highlight
+                    .range
+                    .start()
+                    .cmp(&highlight.range.end(), &snapshot)
+                    .is_le()
+                {
+                    if next_highlight
+                        .range
+                        .end()
+                        .cmp(&highlight.range.end(), &snapshot)
+                        .is_gt()
+                    {
+                        row_highlights[ix].range =
+                            *highlight.range.start()..=*next_highlight.range.end();
+                    }
+                    row_highlights.remove(ix + 1);
+                } else {
+                    break;
+                }
             }
         }
     }
 
+    /// Remove any highlighted row ranges of the given type that intersect the
+    /// given ranges.
+    pub fn remove_highlighted_rows<T: 'static>(
+        &mut self,
+        ranges_to_remove: Vec<Range<Anchor>>,
+        cx: &mut ViewContext<Self>,
+    ) {
+        let snapshot = self.buffer().read(cx).snapshot(cx);
+        let row_highlights = self.highlighted_rows.entry(TypeId::of::<T>()).or_default();
+        let mut ranges_to_remove = ranges_to_remove.iter().peekable();
+        row_highlights.retain(|highlight| {
+            while let Some(range_to_remove) = ranges_to_remove.peek() {
+                match range_to_remove.end.cmp(&highlight.range.start(), &snapshot) {
+                    Ordering::Less => {
+                        ranges_to_remove.next();
+                    }
+                    Ordering::Equal => {
+                        return false;
+                    }
+                    Ordering::Greater => {
+                        match range_to_remove.start.cmp(&highlight.range.end(), &snapshot) {
+                            Ordering::Less | Ordering::Equal => {
+                                return false;
+                            }
+                            Ordering::Greater => break,
+                        }
+                    }
+                }
+            }
+
+            true
+        })
+    }
+
     /// Clear all anchor ranges for a certain highlight context type, so no corresponding rows will be highlighted.
     pub fn clear_row_highlights<T: 'static>(&mut self) {
         self.highlighted_rows.remove(&TypeId::of::<T>());
@@ -11543,13 +11627,12 @@ impl Editor {
     /// For a highlight given context type, gets all anchor ranges that will be used for row highlighting.
     pub fn highlighted_rows<T: 'static>(
         &self,
-    ) -> Option<impl Iterator<Item = (&RangeInclusive<Anchor>, Option<&Hsla>)>> {
+    ) -> impl '_ + Iterator<Item = (RangeInclusive<Anchor>, Hsla)> {
-        Some(
+        self.highlighted_rows
-            self.highlighted_rows
+            .get(&TypeId::of::<T>())
-                .get(&TypeId::of::<T>())?
+            .map_or(&[] as &[_], |vec| vec.as_slice())
-                .iter()
+            .iter()
-                .map(|highlight| (&highlight.range, highlight.color.as_ref())),
+            .map(|highlight| (highlight.range.clone(), highlight.color))
-        )
     }
 
     /// Merges all anchor ranges for all context types ever set, picking the last highlight added in case of a row conflict.
@@ -11574,10 +11657,7 @@ impl Editor {
                             used_highlight_orders.entry(row).or_insert(highlight.index);
                         if highlight.index >= *used_index {
                             *used_index = highlight.index;
-                            match highlight.color {
+                            unique_rows.insert(DisplayRow(row), highlight.color);
-                                Some(hsla) => unique_rows.insert(DisplayRow(row), hsla),
-                                None => unique_rows.remove(&DisplayRow(row)),
-                            };
                         }
                     }
                     unique_rows
@@ -11593,10 +11673,11 @@ impl Editor {
             .values()
             .flat_map(|highlighted_rows| highlighted_rows.iter())
             .filter_map(|highlight| {
-                if highlight.color.is_none() || !highlight.should_autoscroll {
+                if highlight.should_autoscroll {
-                    return None;
+                    Some(highlight.range.start().to_display_point(snapshot).row())
+                } else {
+                    None
                 }
-                Some(highlight.range.start().to_display_point(snapshot).row())
             })
             .min()
     }

crates/editor/src/hunk_diff.rs

@@ -19,8 +19,8 @@ use util::RangeExt;
 use crate::{
     editor_settings::CurrentLineHighlight, hunk_status, hunks_for_selections, BlockDisposition,
     BlockProperties, BlockStyle, CustomBlockId, DiffRowHighlight, DisplayRow, DisplaySnapshot,
-    Editor, EditorElement, EditorSnapshot, ExpandAllHunkDiffs, GoToHunk, GoToPrevHunk,
+    Editor, EditorElement, EditorSnapshot, ExpandAllHunkDiffs, GoToHunk, GoToPrevHunk, RevertFile,
-    RangeToAnchorExt, RevertFile, RevertSelectedHunks, ToDisplayPoint, ToggleHunkDiff,
+    RevertSelectedHunks, ToDisplayPoint, ToggleHunkDiff,
 };
 
 #[derive(Debug, Clone)]
@@ -219,14 +219,7 @@ impl Editor {
                         });
                     }
 
-                    for removed_rows in highlights_to_remove {
+                    editor.remove_highlighted_rows::<DiffRowHighlight>(highlights_to_remove, cx);
-                        editor.highlight_rows::<DiffRowHighlight>(
-                            to_inclusive_row_range(removed_rows, &snapshot),
-                            None,
-                            false,
-                            cx,
-                        );
-                    }
                     editor.remove_blocks(blocks_to_remove, None, cx);
                     for hunk in hunks_to_expand {
                         editor.expand_diff_hunk(None, &hunk, cx);
@@ -306,7 +299,7 @@ impl Editor {
             DiffHunkStatus::Added => {
                 self.highlight_rows::<DiffRowHighlight>(
                     to_inclusive_row_range(hunk_start..hunk_end, &snapshot),
-                    Some(added_hunk_color(cx)),
+                    added_hunk_color(cx),
                     false,
                     cx,
                 );
@@ -315,7 +308,7 @@ impl Editor {
             DiffHunkStatus::Modified => {
                 self.highlight_rows::<DiffRowHighlight>(
                     to_inclusive_row_range(hunk_start..hunk_end, &snapshot),
-                    Some(added_hunk_color(cx)),
+                    added_hunk_color(cx),
                     false,
                     cx,
                 );
@@ -850,14 +843,7 @@ impl Editor {
                         retain
                     });
 
-                    for removed_rows in highlights_to_remove {
+                    editor.remove_highlighted_rows::<DiffRowHighlight>(highlights_to_remove, cx);
-                        editor.highlight_rows::<DiffRowHighlight>(
-                            to_inclusive_row_range(removed_rows, &snapshot),
-                            None,
-                            false,
-                            cx,
-                        );
-                    }
                     editor.remove_blocks(blocks_to_remove, None, cx);
 
                     if let Some(diff_base_buffer) = &diff_base_buffer {
@@ -978,7 +964,7 @@ fn editor_with_deleted_text(
         editor.set_show_inline_completions(Some(false), cx);
         editor.highlight_rows::<DiffRowHighlight>(
             Anchor::min()..=Anchor::max(),
-            Some(deleted_color),
+            deleted_color,
             false,
             cx,
         );
@@ -1060,15 +1046,16 @@ fn to_inclusive_row_range(
     row_range: Range<Anchor>,
     snapshot: &EditorSnapshot,
 ) -> RangeInclusive<Anchor> {
-    let mut display_row_range =
+    let mut end = row_range.end.to_point(&snapshot.buffer_snapshot);
-        row_range.start.to_display_point(snapshot)..row_range.end.to_display_point(snapshot);
+    if end.column == 0 && end.row > 0 {
-    if display_row_range.end.row() > display_row_range.start.row() {
+        end = Point::new(
-        *display_row_range.end.row_mut() -= 1;
+            end.row - 1,
+            snapshot
+                .buffer_snapshot
+                .line_len(MultiBufferRow(end.row - 1)),
+        );
     }
-    let point_range = display_row_range.start.to_point(&snapshot.display_snapshot)
+    row_range.start..=snapshot.buffer_snapshot.anchor_after(end)
-        ..display_row_range.end.to_point(&snapshot.display_snapshot);
-    let new_range = point_range.to_anchors(&snapshot.buffer_snapshot);
-    new_range.start..=new_range.end
 }
 
 impl DisplayDiffHunk {

crates/editor/src/test.rs

@@ -88,116 +88,3 @@ pub(crate) fn build_editor_with_project(
 ) -> Editor {
     Editor::new(EditorMode::Full, buffer, Some(project), true, cx)
 }
-
-#[cfg(any(test, feature = "test-support"))]
-pub fn editor_hunks(
-    editor: &Editor,
-    snapshot: &DisplaySnapshot,
-    cx: &mut ViewContext<'_, Editor>,
-) -> Vec<(
-    String,
-    git::diff::DiffHunkStatus,
-    std::ops::Range<crate::DisplayRow>,
-)> {
-    use multi_buffer::MultiBufferRow;
-    use text::Point;
-
-    use crate::hunk_status;
-
-    snapshot
-        .buffer_snapshot
-        .git_diff_hunks_in_range(MultiBufferRow::MIN..MultiBufferRow::MAX)
-        .map(|hunk| {
-            let display_range = Point::new(hunk.row_range.start.0, 0)
-                .to_display_point(snapshot)
-                .row()
-                ..Point::new(hunk.row_range.end.0, 0)
-                    .to_display_point(snapshot)
-                    .row();
-            let (_, buffer, _) = editor
-                .buffer()
-                .read(cx)
-                .excerpt_containing(Point::new(hunk.row_range.start.0, 0), cx)
-                .expect("no excerpt for expanded buffer's hunk start");
-            let diff_base = buffer
-                .read(cx)
-                .diff_base()
-                .expect("should have a diff base for expanded hunk")
-                .slice(hunk.diff_base_byte_range.clone())
-                .to_string();
-            (diff_base, hunk_status(&hunk), display_range)
-        })
-        .collect()
-}
-
-#[cfg(any(test, feature = "test-support"))]
-pub fn expanded_hunks(
-    editor: &Editor,
-    snapshot: &DisplaySnapshot,
-    cx: &mut ViewContext<'_, Editor>,
-) -> Vec<(
-    String,
-    git::diff::DiffHunkStatus,
-    std::ops::Range<crate::DisplayRow>,
-)> {
-    editor
-        .expanded_hunks
-        .hunks(false)
-        .map(|expanded_hunk| {
-            let hunk_display_range = expanded_hunk
-                .hunk_range
-                .start
-                .to_display_point(snapshot)
-                .row()
-                ..expanded_hunk
-                    .hunk_range
-                    .end
-                    .to_display_point(snapshot)
-                    .row();
-            let (_, buffer, _) = editor
-                .buffer()
-                .read(cx)
-                .excerpt_containing(expanded_hunk.hunk_range.start, cx)
-                .expect("no excerpt for expanded buffer's hunk start");
-            let diff_base = buffer
-                .read(cx)
-                .diff_base()
-                .expect("should have a diff base for expanded hunk")
-                .slice(expanded_hunk.diff_base_byte_range.clone())
-                .to_string();
-            (diff_base, expanded_hunk.status, hunk_display_range)
-        })
-        .collect()
-}
-
-#[cfg(any(test, feature = "test-support"))]
-pub fn expanded_hunks_background_highlights(
-    editor: &mut Editor,
-    cx: &mut gpui::WindowContext,
-) -> Vec<std::ops::RangeInclusive<crate::DisplayRow>> {
-    use crate::DisplayRow;
-
-    let mut highlights = Vec::new();
-
-    let mut range_start = 0;
-    let mut previous_highlighted_row = None;
-    for (highlighted_row, _) in editor.highlighted_display_rows(cx) {
-        match previous_highlighted_row {
-            Some(previous_row) => {
-                if previous_row + 1 != highlighted_row.0 {
-                    highlights.push(DisplayRow(range_start)..=DisplayRow(previous_row));
-                    range_start = highlighted_row.0;
-                }
-            }
-            None => {
-                range_start = highlighted_row.0;
-            }
-        }
-        previous_highlighted_row = Some(highlighted_row.0);
-    }
-    if let Some(previous_row) = previous_highlighted_row {
-        highlights.push(DisplayRow(range_start)..=DisplayRow(previous_row));
-    }
-
-    highlights
-}

crates/editor/src/test/editor_test_context.rs

@@ -1,17 +1,18 @@
 use crate::{
-    display_map::ToDisplayPoint, AnchorRangeExt, Autoscroll, DisplayPoint, Editor, MultiBuffer,
+    display_map::ToDisplayPoint, AnchorRangeExt, Autoscroll, DiffRowHighlight, DisplayPoint,
-    RowExt,
+    Editor, MultiBuffer, RowExt,
 };
 use collections::BTreeMap;
 use futures::Future;
+use git::diff::DiffHunkStatus;
 use gpui::{
     AnyWindowHandle, AppContext, Keystroke, ModelContext, Pixels, Point, View, ViewContext,
-    VisualTestContext,
+    VisualTestContext, WindowHandle,
 };
 use indoc::indoc;
 use itertools::Itertools;
 use language::{Buffer, BufferSnapshot, LanguageRegistry};
-use multi_buffer::ExcerptRange;
+use multi_buffer::{ExcerptRange, ToPoint};
 use parking_lot::RwLock;
 use project::{FakeFs, Project};
 use std::{
@@ -71,6 +72,16 @@ impl EditorTestContext {
         }
     }
 
+    pub async fn for_editor(editor: WindowHandle<Editor>, cx: &mut gpui::TestAppContext) -> Self {
+        let editor_view = editor.root_view(cx).unwrap();
+        Self {
+            cx: VisualTestContext::from_window(*editor.deref(), cx),
+            window: editor.into(),
+            editor: editor_view,
+            assertion_cx: AssertionContextManager::new(),
+        }
+    }
+
     pub fn new_multibuffer<const COUNT: usize>(
         cx: &mut gpui::TestAppContext,
         excerpts: [&str; COUNT],
@@ -297,6 +308,76 @@ impl EditorTestContext {
         state_context
     }
 
+    #[track_caller]
+    pub fn assert_diff_hunks(&mut self, expected_diff: String) {
+        // Normalize the expected diff. If it has no diff markers, then insert blank markers
+        // before each line. Strip any whitespace-only lines.
+        let has_diff_markers = expected_diff
+            .lines()
+            .any(|line| line.starts_with("+") || line.starts_with("-"));
+        let expected_diff_text = expected_diff
+            .split('\n')
+            .map(|line| {
+                let trimmed = line.trim();
+                if trimmed.is_empty() {
+                    String::new()
+                } else if has_diff_markers {
+                    line.to_string()
+                } else {
+                    format!("  {line}")
+                }
+            })
+            .join("\n");
+
+        // Read the actual diff from the editor's row highlights and block
+        // decorations.
+        let actual_diff = self.editor.update(&mut self.cx, |editor, cx| {
+            let snapshot = editor.snapshot(cx);
+            let text = editor.text(cx);
+            let insertions = editor
+                .highlighted_rows::<DiffRowHighlight>()
+                .map(|(range, _)| {
+                    range.start().to_point(&snapshot.buffer_snapshot).row
+                        ..range.end().to_point(&snapshot.buffer_snapshot).row + 1
+                })
+                .collect::<Vec<_>>();
+            let deletions = editor
+                .expanded_hunks
+                .hunks
+                .iter()
+                .filter_map(|hunk| {
+                    if hunk.blocks.is_empty() {
+                        return None;
+                    }
+                    let row = hunk
+                        .hunk_range
+                        .start
+                        .to_point(&snapshot.buffer_snapshot)
+                        .row;
+                    let (_, buffer, _) = editor
+                        .buffer()
+                        .read(cx)
+                        .excerpt_containing(hunk.hunk_range.start, cx)
+                        .expect("no excerpt for expanded buffer's hunk start");
+                    let deleted_text = buffer
+                        .read(cx)
+                        .diff_base()
+                        .expect("should have a diff base for expanded hunk")
+                        .slice(hunk.diff_base_byte_range.clone())
+                        .to_string();
+                    if let DiffHunkStatus::Modified | DiffHunkStatus::Removed = hunk.status {
+                        Some((row, deleted_text))
+                    } else {
+                        None
+                    }
+                })
+                .collect::<Vec<_>>();
+            format_diff(text, deletions, insertions)
+        });
+
+        pretty_assertions::assert_eq!(actual_diff, expected_diff_text, "unexpected diff state");
+    }
+
     /// Make an assertion about the editor's text and the ranges and directions
     /// of its selections using a string containing embedded range markers.
     ///
@@ -401,6 +482,46 @@ impl EditorTestContext {
     }
 }
 
+fn format_diff(
+    text: String,
+    actual_deletions: Vec<(u32, String)>,
+    actual_insertions: Vec<Range<u32>>,
+) -> String {
+    let mut diff = String::new();
+    for (row, line) in text.split('\n').enumerate() {
+        let row = row as u32;
+        if row > 0 {
+            diff.push('\n');
+        }
+        if let Some(text) = actual_deletions
+            .iter()
+            .find_map(|(deletion_row, deleted_text)| {
+                if *deletion_row == row {
+                    Some(deleted_text)
+                } else {
+                    None
+                }
+            })
+        {
+            for line in text.lines() {
+                diff.push('-');
+                if !line.is_empty() {
+                    diff.push(' ');
+                    diff.push_str(line);
+                }
+                diff.push('\n');
+            }
+        }
+        let marker = if actual_insertions.iter().any(|range| range.contains(&row)) {
+            "+ "
+        } else {
+            "  "
+        };
+        diff.push_str(format!("{marker}{line}").trim_end());
+    }
+    diff
+}
+
 impl Deref for EditorTestContext {
     type Target = gpui::VisualTestContext;
 

crates/go_to_line/src/go_to_line.rs

@@ -121,7 +121,7 @@ impl GoToLine {
                 active_editor.clear_row_highlights::<GoToLineRowHighlights>();
                 active_editor.highlight_rows::<GoToLineRowHighlights>(
                     anchor..=anchor,
-                    Some(cx.theme().colors().editor_highlighted_line_background),
+                    cx.theme().colors().editor_highlighted_line_background,
                     true,
                     cx,
                 );

crates/outline/src/outline.rs

@@ -144,7 +144,7 @@ impl OutlineViewDelegate {
                 active_editor.clear_row_highlights::<OutlineRowHighlights>();
                 active_editor.highlight_rows::<OutlineRowHighlights>(
                     outline_item.range.start..=outline_item.range.end,
-                    Some(cx.theme().colors().editor_highlighted_line_background),
+                    cx.theme().colors().editor_highlighted_line_background,
                     true,
                     cx,
                 );
@@ -240,10 +240,10 @@ impl PickerDelegate for OutlineViewDelegate {
         self.prev_scroll_position.take();
 
         self.active_editor.update(cx, |active_editor, cx| {
-            if let Some(rows) = active_editor
+            let highlight = active_editor
                 .highlighted_rows::<OutlineRowHighlights>()
-                .and_then(|highlights| highlights.into_iter().next().map(|(rows, _)| rows.clone()))
+                .next();
-            {
+            if let Some((rows, _)) = highlight {
                 active_editor.change_selections(Some(Autoscroll::center()), cx, |s| {
                     s.select_ranges([*rows.start()..*rows.start()])
                 });