Refine styling of merge conflicts (#31012)

Nate Butler , Marshall Bowers , and Cole Miller created

- Improved colors
- Blank out diff hunk gutter highlights in conflict regions
- Paint conflict marker highlights all the way to the gutter

Release Notes:

- Improved the highlighting of merge conflict markers in editors.

---------

Co-authored-by: Marshall Bowers <git@maxdeviant.com>
Co-authored-by: Cole Miller <cole@zed.dev>

Change summary

assets/themes/one/one.json           |  2 
crates/agent/src/inline_assistant.rs |  4 
crates/editor/src/editor.rs          | 65 ++++++++++++++++++++++++++++-
crates/git_ui/src/conflict_view.rs   | 35 +++++++++------
crates/theme/src/default_colors.rs   | 14 +----
crates/theme/src/fallback_themes.rs  | 19 -------
crates/theme/src/schema.rs           | 48 ++++++++-------------
crates/theme/src/styles/colors.rs    |  7 --
8 files changed, 113 insertions(+), 81 deletions(-)

Detailed changes

assets/themes/one/one.json 🔗

@@ -99,6 +99,8 @@
         "version_control.added": "#27a657ff",
         "version_control.modified": "#d3b020ff",
         "version_control.deleted": "#e06c76ff",
+        "version_control.conflict_marker.ours": "#a1c1811a",
+        "version_control.conflict_marker.theirs": "#74ade81a",
         "conflict": "#dec184ff",
         "conflict.background": "#dec1841a",
         "conflict.border": "#5d4c2fff",

crates/agent/src/inline_assistant.rs 🔗

@@ -1331,7 +1331,7 @@ impl InlineAssistant {
                 editor.clear_gutter_highlights::<GutterPendingRange>(cx);
             } else {
                 editor.highlight_gutter::<GutterPendingRange>(
-                    &gutter_pending_ranges,
+                    gutter_pending_ranges,
                     |cx| cx.theme().status().info_background,
                     cx,
                 )
@@ -1342,7 +1342,7 @@ impl InlineAssistant {
                 editor.clear_gutter_highlights::<GutterTransformedRange>(cx);
             } else {
                 editor.highlight_gutter::<GutterTransformedRange>(
-                    &gutter_transformed_ranges,
+                    gutter_transformed_ranges,
                     |cx| cx.theme().status().info,
                     cx,
                 )

crates/editor/src/editor.rs 🔗

@@ -698,7 +698,7 @@ impl EditorActionId {
 // type OverrideTextStyle = dyn Fn(&EditorStyle) -> Option<HighlightStyle>;
 
 type BackgroundHighlight = (fn(&ThemeColors) -> Hsla, Arc<[Range<Anchor>]>);
-type GutterHighlight = (fn(&App) -> Hsla, Arc<[Range<Anchor>]>);
+type GutterHighlight = (fn(&App) -> Hsla, Vec<Range<Anchor>>);
 
 #[derive(Default)]
 struct ScrollbarMarkerState {
@@ -18377,12 +18377,12 @@ impl Editor {
 
     pub fn highlight_gutter<T: 'static>(
         &mut self,
-        ranges: &[Range<Anchor>],
+        ranges: impl Into<Vec<Range<Anchor>>>,
         color_fetcher: fn(&App) -> Hsla,
         cx: &mut Context<Self>,
     ) {
         self.gutter_highlights
-            .insert(TypeId::of::<T>(), (color_fetcher, Arc::from(ranges)));
+            .insert(TypeId::of::<T>(), (color_fetcher, ranges.into()));
         cx.notify();
     }
 
@@ -18394,6 +18394,65 @@ impl Editor {
         self.gutter_highlights.remove(&TypeId::of::<T>())
     }
 
+    pub fn insert_gutter_highlight<T: 'static>(
+        &mut self,
+        range: Range<Anchor>,
+        color_fetcher: fn(&App) -> Hsla,
+        cx: &mut Context<Self>,
+    ) {
+        let snapshot = self.buffer().read(cx).snapshot(cx);
+        let mut highlights = self
+            .gutter_highlights
+            .remove(&TypeId::of::<T>())
+            .map(|(_, highlights)| highlights)
+            .unwrap_or_default();
+        let ix = highlights.binary_search_by(|highlight| {
+            Ordering::Equal
+                .then_with(|| highlight.start.cmp(&range.start, &snapshot))
+                .then_with(|| highlight.end.cmp(&range.end, &snapshot))
+        });
+        if let Err(ix) = ix {
+            highlights.insert(ix, range);
+        }
+        self.gutter_highlights
+            .insert(TypeId::of::<T>(), (color_fetcher, highlights));
+    }
+
+    pub fn remove_gutter_highlights<T: 'static>(
+        &mut self,
+        ranges_to_remove: Vec<Range<Anchor>>,
+        cx: &mut Context<Self>,
+    ) {
+        let snapshot = self.buffer().read(cx).snapshot(cx);
+        let Some((color_fetcher, mut gutter_highlights)) =
+            self.gutter_highlights.remove(&TypeId::of::<T>())
+        else {
+            return;
+        };
+        let mut ranges_to_remove = ranges_to_remove.iter().peekable();
+        gutter_highlights.retain(|highlight| {
+            while let Some(range_to_remove) = ranges_to_remove.peek() {
+                match range_to_remove.end.cmp(&highlight.start, &snapshot) {
+                    Ordering::Less | Ordering::Equal => {
+                        ranges_to_remove.next();
+                    }
+                    Ordering::Greater => {
+                        match range_to_remove.start.cmp(&highlight.end, &snapshot) {
+                            Ordering::Less | Ordering::Equal => {
+                                return false;
+                            }
+                            Ordering::Greater => break,
+                        }
+                    }
+                }
+            }
+
+            true
+        });
+        self.gutter_highlights
+            .insert(TypeId::of::<T>(), (color_fetcher, gutter_highlights));
+    }
+
     #[cfg(feature = "test-support")]
     pub fn all_text_background_highlights(
         &self,

crates/git_ui/src/conflict_view.rs 🔗

@@ -248,6 +248,8 @@ fn conflicts_updated(
             removed_block_ids.insert(block_id);
         }
 
+        editor.remove_gutter_highlights::<ConflictsOuter>(removed_highlighted_ranges.clone(), cx);
+
         editor.remove_highlighted_rows::<ConflictsOuter>(removed_highlighted_ranges.clone(), cx);
         editor.remove_highlighted_rows::<ConflictsOurs>(removed_highlighted_ranges.clone(), cx);
         editor
@@ -325,8 +327,7 @@ fn update_conflict_highlighting(
     cx: &mut Context<Editor>,
 ) {
     log::debug!("update conflict highlighting for {conflict:?}");
-    let theme = cx.theme().clone();
-    let colors = theme.colors();
+
     let outer_start = buffer
         .anchor_in_excerpt(excerpt_id, conflict.range.start)
         .unwrap();
@@ -346,26 +347,29 @@ fn update_conflict_highlighting(
         .anchor_in_excerpt(excerpt_id, conflict.theirs.end)
         .unwrap();
 
-    let ours_background = colors.version_control_conflict_ours_background;
-    let ours_marker = colors.version_control_conflict_ours_marker_background;
-    let theirs_background = colors.version_control_conflict_theirs_background;
-    let theirs_marker = colors.version_control_conflict_theirs_marker_background;
-    let divider_background = colors.version_control_conflict_divider_background;
+    let ours_background = cx.theme().colors().version_control_conflict_marker_ours;
+    let theirs_background = cx.theme().colors().version_control_conflict_marker_theirs;
 
     let options = RowHighlightOptions {
-        include_gutter: false,
+        include_gutter: true,
         ..Default::default()
     };
 
+    editor.insert_gutter_highlight::<ConflictsOuter>(
+        outer_start..their_end,
+        |cx| cx.theme().colors().editor_background,
+        cx,
+    );
+
     // Prevent diff hunk highlighting within the entire conflict region.
-    editor.highlight_rows::<ConflictsOuter>(
-        outer_start..outer_end,
-        divider_background,
+    editor.highlight_rows::<ConflictsOuter>(outer_start..outer_end, theirs_background, options, cx);
+    editor.highlight_rows::<ConflictsOurs>(our_start..our_end, ours_background, options, cx);
+    editor.highlight_rows::<ConflictsOursMarker>(
+        outer_start..our_start,
+        ours_background,
         options,
         cx,
     );
-    editor.highlight_rows::<ConflictsOurs>(our_start..our_end, ours_background, options, cx);
-    editor.highlight_rows::<ConflictsOursMarker>(outer_start..our_start, ours_marker, options, cx);
     editor.highlight_rows::<ConflictsTheirs>(
         their_start..their_end,
         theirs_background,
@@ -374,7 +378,7 @@ fn update_conflict_highlighting(
     );
     editor.highlight_rows::<ConflictsTheirsMarker>(
         their_end..outer_end,
-        theirs_marker,
+        theirs_background,
         options,
         cx,
     );
@@ -512,6 +516,9 @@ pub(crate) fn resolve_conflict(
                 let end = snapshot
                     .anchor_in_excerpt(excerpt_id, resolved_conflict.range.end)
                     .unwrap();
+
+                editor.remove_gutter_highlights::<ConflictsOuter>(vec![start..end], cx);
+
                 editor.remove_highlighted_rows::<ConflictsOuter>(vec![start..end], cx);
                 editor.remove_highlighted_rows::<ConflictsOurs>(vec![start..end], cx);
                 editor.remove_highlighted_rows::<ConflictsTheirs>(vec![start..end], cx);

crates/theme/src/default_colors.rs 🔗

@@ -148,11 +148,8 @@ impl ThemeColors {
             version_control_renamed: MODIFIED_COLOR,
             version_control_conflict: orange().light().step_12(),
             version_control_ignored: gray().light().step_12(),
-            version_control_conflict_ours_background: green().light().step_10().alpha(0.5),
-            version_control_conflict_theirs_background: blue().light().step_10().alpha(0.5),
-            version_control_conflict_ours_marker_background: green().light().step_10().alpha(0.7),
-            version_control_conflict_theirs_marker_background: blue().light().step_10().alpha(0.7),
-            version_control_conflict_divider_background: Hsla::default(),
+            version_control_conflict_marker_ours: green().light().step_10().alpha(0.5),
+            version_control_conflict_marker_theirs: blue().light().step_10().alpha(0.5),
         }
     }
 
@@ -273,11 +270,8 @@ impl ThemeColors {
             version_control_renamed: MODIFIED_COLOR,
             version_control_conflict: orange().dark().step_12(),
             version_control_ignored: gray().dark().step_12(),
-            version_control_conflict_ours_background: green().dark().step_10().alpha(0.5),
-            version_control_conflict_theirs_background: blue().dark().step_10().alpha(0.5),
-            version_control_conflict_ours_marker_background: green().dark().step_10().alpha(0.7),
-            version_control_conflict_theirs_marker_background: blue().dark().step_10().alpha(0.7),
-            version_control_conflict_divider_background: Hsla::default(),
+            version_control_conflict_marker_ours: green().dark().step_10().alpha(0.5),
+            version_control_conflict_marker_theirs: blue().dark().step_10().alpha(0.5),
         }
     }
 }

crates/theme/src/fallback_themes.rs 🔗

@@ -211,23 +211,8 @@ pub(crate) fn zed_default_dark() -> Theme {
                 version_control_renamed: MODIFIED_COLOR,
                 version_control_conflict: crate::orange().light().step_12(),
                 version_control_ignored: crate::gray().light().step_12(),
-                version_control_conflict_ours_background: crate::green()
-                    .light()
-                    .step_12()
-                    .alpha(0.5),
-                version_control_conflict_theirs_background: crate::blue()
-                    .light()
-                    .step_12()
-                    .alpha(0.5),
-                version_control_conflict_ours_marker_background: crate::green()
-                    .light()
-                    .step_12()
-                    .alpha(0.7),
-                version_control_conflict_theirs_marker_background: crate::blue()
-                    .light()
-                    .step_12()
-                    .alpha(0.7),
-                version_control_conflict_divider_background: Hsla::default(),
+                version_control_conflict_marker_ours: crate::green().light().step_12().alpha(0.5),
+                version_control_conflict_marker_theirs: crate::blue().light().step_12().alpha(0.5),
             },
             status: StatusColors {
                 conflict: yellow,

crates/theme/src/schema.rs 🔗

@@ -620,24 +620,20 @@ pub struct ThemeColorsContent {
     pub version_control_ignored: Option<String>,
 
     /// Background color for row highlights of "ours" regions in merge conflicts.
-    #[serde(rename = "version_control.conflict.ours_background")]
-    pub version_control_conflict_ours_background: Option<String>,
+    #[serde(rename = "version_control.conflict_marker.ours")]
+    pub version_control_conflict_marker_ours: Option<String>,
 
     /// Background color for row highlights of "theirs" regions in merge conflicts.
-    #[serde(rename = "version_control.conflict.theirs_background")]
-    pub version_control_conflict_theirs_background: Option<String>,
-
-    /// Background color for row highlights of "ours" conflict markers in merge conflicts.
-    #[serde(rename = "version_control.conflict.ours_marker_background")]
-    pub version_control_conflict_ours_marker_background: Option<String>,
+    #[serde(rename = "version_control.conflict_marker.theirs")]
+    pub version_control_conflict_marker_theirs: Option<String>,
 
-    /// Background color for row highlights of "theirs" conflict markers in merge conflicts.
-    #[serde(rename = "version_control.conflict.theirs_marker_background")]
-    pub version_control_conflict_theirs_marker_background: Option<String>,
+    /// Deprecated in favor of `version_control_conflict_marker_ours`.
+    #[deprecated]
+    pub version_control_conflict_ours_background: Option<String>,
 
-    /// Background color for row highlights of the "ours"/"theirs" divider in merge conflicts.
-    #[serde(rename = "version_control.conflict.divider_background")]
-    pub version_control_conflict_divider_background: Option<String>,
+    /// Deprecated in favor of `version_control_conflict_marker_theirs`.
+    #[deprecated]
+    pub version_control_conflict_theirs_background: Option<String>,
 }
 
 impl ThemeColorsContent {
@@ -1118,25 +1114,17 @@ impl ThemeColorsContent {
                 .and_then(|color| try_parse_color(color).ok())
                 // Fall back to `conflict`, for backwards compatibility.
                 .or(status_colors.ignored),
-            version_control_conflict_ours_background: self
-                .version_control_conflict_ours_background
-                .as_ref()
-                .and_then(|color| try_parse_color(color).ok()),
-            version_control_conflict_theirs_background: self
-                .version_control_conflict_theirs_background
-                .as_ref()
-                .and_then(|color| try_parse_color(color).ok()),
-            version_control_conflict_ours_marker_background: self
-                .version_control_conflict_ours_marker_background
-                .as_ref()
-                .and_then(|color| try_parse_color(color).ok()),
-            version_control_conflict_theirs_marker_background: self
-                .version_control_conflict_theirs_marker_background
+            #[allow(deprecated)]
+            version_control_conflict_marker_ours: self
+                .version_control_conflict_marker_ours
                 .as_ref()
+                .or(self.version_control_conflict_ours_background.as_ref())
                 .and_then(|color| try_parse_color(color).ok()),
-            version_control_conflict_divider_background: self
-                .version_control_conflict_divider_background
+            #[allow(deprecated)]
+            version_control_conflict_marker_theirs: self
+                .version_control_conflict_marker_theirs
                 .as_ref()
+                .or(self.version_control_conflict_theirs_background.as_ref())
                 .and_then(|color| try_parse_color(color).ok()),
         }
     }

crates/theme/src/styles/colors.rs 🔗

@@ -273,12 +273,9 @@ pub struct ThemeColors {
     pub version_control_ignored: Hsla,
 
     /// Represents the "ours" region of a merge conflict.
-    pub version_control_conflict_ours_background: Hsla,
+    pub version_control_conflict_marker_ours: Hsla,
     /// Represents the "theirs" region of a merge conflict.
-    pub version_control_conflict_theirs_background: Hsla,
-    pub version_control_conflict_ours_marker_background: Hsla,
-    pub version_control_conflict_theirs_marker_background: Hsla,
-    pub version_control_conflict_divider_background: Hsla,
+    pub version_control_conflict_marker_theirs: Hsla,
 }
 
 #[derive(EnumIter, Debug, Clone, Copy, AsRefStr)]