From 0b7583bae56f0a4f5e492719f0001441500d0459 Mon Sep 17 00:00:00 2001 From: Nate Butler Date: Mon, 9 Jun 2025 15:03:19 -0400 Subject: [PATCH] Refine styling of merge conflicts (#31012) - 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 Co-authored-by: Cole Miller --- 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 | 50 ++++++++------------- crates/theme/src/styles/colors.rs | 7 +-- 8 files changed, 114 insertions(+), 82 deletions(-) diff --git a/assets/themes/one/one.json b/assets/themes/one/one.json index 0163c0958e..bf38d9dccb 100644 --- a/assets/themes/one/one.json +++ b/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", diff --git a/crates/agent/src/inline_assistant.rs b/crates/agent/src/inline_assistant.rs index 622098c6b8..89ff338604 100644 --- a/crates/agent/src/inline_assistant.rs +++ b/crates/agent/src/inline_assistant.rs @@ -1331,7 +1331,7 @@ impl InlineAssistant { editor.clear_gutter_highlights::(cx); } else { editor.highlight_gutter::( - &gutter_pending_ranges, + gutter_pending_ranges, |cx| cx.theme().status().info_background, cx, ) @@ -1342,7 +1342,7 @@ impl InlineAssistant { editor.clear_gutter_highlights::(cx); } else { editor.highlight_gutter::( - &gutter_transformed_ranges, + gutter_transformed_ranges, |cx| cx.theme().status().info, cx, ) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 82695f319e..28f2ef75b0 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -698,7 +698,7 @@ impl EditorActionId { // type OverrideTextStyle = dyn Fn(&EditorStyle) -> Option; type BackgroundHighlight = (fn(&ThemeColors) -> Hsla, Arc<[Range]>); -type GutterHighlight = (fn(&App) -> Hsla, Arc<[Range]>); +type GutterHighlight = (fn(&App) -> Hsla, Vec>); #[derive(Default)] struct ScrollbarMarkerState { @@ -18377,12 +18377,12 @@ impl Editor { pub fn highlight_gutter( &mut self, - ranges: &[Range], + ranges: impl Into>>, color_fetcher: fn(&App) -> Hsla, cx: &mut Context, ) { self.gutter_highlights - .insert(TypeId::of::(), (color_fetcher, Arc::from(ranges))); + .insert(TypeId::of::(), (color_fetcher, ranges.into())); cx.notify(); } @@ -18394,6 +18394,65 @@ impl Editor { self.gutter_highlights.remove(&TypeId::of::()) } + pub fn insert_gutter_highlight( + &mut self, + range: Range, + color_fetcher: fn(&App) -> Hsla, + cx: &mut Context, + ) { + let snapshot = self.buffer().read(cx).snapshot(cx); + let mut highlights = self + .gutter_highlights + .remove(&TypeId::of::()) + .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::(), (color_fetcher, highlights)); + } + + pub fn remove_gutter_highlights( + &mut self, + ranges_to_remove: Vec>, + cx: &mut Context, + ) { + let snapshot = self.buffer().read(cx).snapshot(cx); + let Some((color_fetcher, mut gutter_highlights)) = + self.gutter_highlights.remove(&TypeId::of::()) + 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::(), (color_fetcher, gutter_highlights)); + } + #[cfg(feature = "test-support")] pub fn all_text_background_highlights( &self, diff --git a/crates/git_ui/src/conflict_view.rs b/crates/git_ui/src/conflict_view.rs index a7fbcb9bec..8eadf70830 100644 --- a/crates/git_ui/src/conflict_view.rs +++ b/crates/git_ui/src/conflict_view.rs @@ -248,6 +248,8 @@ fn conflicts_updated( removed_block_ids.insert(block_id); } + editor.remove_gutter_highlights::(removed_highlighted_ranges.clone(), cx); + editor.remove_highlighted_rows::(removed_highlighted_ranges.clone(), cx); editor.remove_highlighted_rows::(removed_highlighted_ranges.clone(), cx); editor @@ -325,8 +327,7 @@ fn update_conflict_highlighting( cx: &mut Context, ) { 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::( + outer_start..their_end, + |cx| cx.theme().colors().editor_background, + cx, + ); + // Prevent diff hunk highlighting within the entire conflict region. - editor.highlight_rows::( - outer_start..outer_end, - divider_background, + editor.highlight_rows::(outer_start..outer_end, theirs_background, options, cx); + editor.highlight_rows::(our_start..our_end, ours_background, options, cx); + editor.highlight_rows::( + outer_start..our_start, + ours_background, options, cx, ); - editor.highlight_rows::(our_start..our_end, ours_background, options, cx); - editor.highlight_rows::(outer_start..our_start, ours_marker, options, cx); editor.highlight_rows::( their_start..their_end, theirs_background, @@ -374,7 +378,7 @@ fn update_conflict_highlighting( ); editor.highlight_rows::( 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::(vec![start..end], cx); + editor.remove_highlighted_rows::(vec![start..end], cx); editor.remove_highlighted_rows::(vec![start..end], cx); editor.remove_highlighted_rows::(vec![start..end], cx); diff --git a/crates/theme/src/default_colors.rs b/crates/theme/src/default_colors.rs index cc4ad01e4d..33d7b86e3d 100644 --- a/crates/theme/src/default_colors.rs +++ b/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), } } } diff --git a/crates/theme/src/fallback_themes.rs b/crates/theme/src/fallback_themes.rs index 941a1901bb..afc977d7fd 100644 --- a/crates/theme/src/fallback_themes.rs +++ b/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, diff --git a/crates/theme/src/schema.rs b/crates/theme/src/schema.rs index 32810c2ae7..a071ca26c8 100644 --- a/crates/theme/src/schema.rs +++ b/crates/theme/src/schema.rs @@ -620,24 +620,20 @@ pub struct ThemeColorsContent { pub version_control_ignored: Option, /// 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, + #[serde(rename = "version_control.conflict_marker.ours")] + pub version_control_conflict_marker_ours: Option, /// Background color for row highlights of "theirs" regions in merge conflicts. - #[serde(rename = "version_control.conflict.theirs_background")] + #[serde(rename = "version_control.conflict_marker.theirs")] + pub version_control_conflict_marker_theirs: Option, + + /// Deprecated in favor of `version_control_conflict_marker_ours`. + #[deprecated] + pub version_control_conflict_ours_background: Option, + + /// Deprecated in favor of `version_control_conflict_marker_theirs`. + #[deprecated] pub version_control_conflict_theirs_background: Option, - - /// 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, - - /// 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, - - /// 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, } 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 + #[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_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 - .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()), } } diff --git a/crates/theme/src/styles/colors.rs b/crates/theme/src/styles/colors.rs index a66f206744..fb821385f5 100644 --- a/crates/theme/src/styles/colors.rs +++ b/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)]