editor: Use default gutter margin instead of horizontal_padding for horizontal content padding (#30138)
This PR changes the way a horizontal margin is added in editors. It
removes the possibility to set a custom `horizontal_padding` for an
editor and utilizes the default `gutter_dimension` instead.
This change is made to ensure that no issues with soft-wrapping occurs
for any editor that has a `horizontal_margin` set (see #26893 for more
context on the implications here`. Furthermore, it ensures that the text
actually renders properly when scrolling horizontally and is not
cut-off.
### Horizontal padding:
| `main` | This PR |
| --- | --- |
| 
| 
|
### Editor horizontally scrolled:
| `main` | This PR |
| --- | --- |
| 
| 
|
Notice the difference at the horizontal borders.
The margin added for the `edit_file_tool` was 4 pixels. The `descent`,
whilst not exactly, is roughly the same here and also scales with the
font size nicely. Furthermore, it seems that the
`gutter_dimensions.margin` should be present anyway, given the following
comment
0b00256f58/crates/editor/src/element.rs (L6887-L6889)
so ensuring this property is actually set and not 0 seems to be
reasonable given the circumstances.
Please note though that this will apply to all editors in the app.
Again, this seems like it should be the case anyway, just wanted to
mention this again.
Should the fix like this not be wanted, I can change this here so that
the `horizontal_margin` is better accounted for when soft-wrapping in an
editor. Feel free to let me know in this case.
Release Notes:
- N/A
This commit is contained in:
Finn Evers
GitHub
parent
6e19c9b141
commit
358c324e26
3 changed files with 43 additions and 58 deletions
|
|
@ -8,11 +8,11 @@ use assistant_tool::{
|
|||
ActionLog, AnyToolCard, Tool, ToolCard, ToolResult, ToolResultOutput, ToolUseStatus,
|
||||
};
|
||||
use buffer_diff::{BufferDiff, BufferDiffSnapshot};
|
||||
use editor::{Editor, EditorElement, EditorMode, EditorStyle, MultiBuffer, PathKey};
|
||||
use editor::{Editor, EditorMode, MultiBuffer, PathKey};
|
||||
use futures::StreamExt;
|
||||
use gpui::{
|
||||
Animation, AnimationExt, AnyWindowHandle, App, AppContext, AsyncApp, Entity, EntityId, Task,
|
||||
TextStyle, WeakEntity, pulsating_between,
|
||||
TextStyleRefinement, WeakEntity, pulsating_between,
|
||||
};
|
||||
use indoc::formatdoc;
|
||||
use language::{
|
||||
|
|
@ -574,33 +574,16 @@ impl ToolCard for EditFileToolCard {
|
|||
.map(|style| style.text.line_height_in_pixels(window.rem_size()))
|
||||
.unwrap_or_default();
|
||||
|
||||
let settings = ThemeSettings::get_global(cx);
|
||||
let element = EditorElement::new(
|
||||
&cx.entity(),
|
||||
EditorStyle {
|
||||
background: cx.theme().colors().editor_background,
|
||||
horizontal_padding: rems(0.25).to_pixels(window.rem_size()),
|
||||
local_player: cx.theme().players().local(),
|
||||
text: TextStyle {
|
||||
color: cx.theme().colors().editor_foreground,
|
||||
font_family: settings.buffer_font.family.clone(),
|
||||
font_features: settings.buffer_font.features.clone(),
|
||||
font_fallbacks: settings.buffer_font.fallbacks.clone(),
|
||||
font_size: TextSize::Small
|
||||
.rems(cx)
|
||||
.to_pixels(settings.agent_font_size(cx))
|
||||
.into(),
|
||||
font_weight: settings.buffer_font.weight,
|
||||
line_height: relative(settings.buffer_line_height.value()),
|
||||
..Default::default()
|
||||
},
|
||||
scrollbar_width: EditorElement::SCROLLBAR_WIDTH,
|
||||
syntax: cx.theme().syntax().clone(),
|
||||
status: cx.theme().status().clone(),
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
|
||||
editor.set_text_style_refinement(TextStyleRefinement {
|
||||
font_size: Some(
|
||||
TextSize::Small
|
||||
.rems(cx)
|
||||
.to_pixels(ThemeSettings::get_global(cx).agent_font_size(cx))
|
||||
.into(),
|
||||
),
|
||||
..TextStyleRefinement::default()
|
||||
});
|
||||
let element = editor.render(window, cx);
|
||||
(element.into_any_element(), line_height)
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -517,7 +517,6 @@ pub enum SoftWrap {
|
|||
#[derive(Clone)]
|
||||
pub struct EditorStyle {
|
||||
pub background: Hsla,
|
||||
pub horizontal_padding: Pixels,
|
||||
pub local_player: PlayerColor,
|
||||
pub text: TextStyle,
|
||||
pub scrollbar_width: Pixels,
|
||||
|
|
@ -532,7 +531,6 @@ impl Default for EditorStyle {
|
|||
fn default() -> Self {
|
||||
Self {
|
||||
background: Hsla::default(),
|
||||
horizontal_padding: Pixels::default(),
|
||||
local_player: PlayerColor::default(),
|
||||
text: TextStyle::default(),
|
||||
scrollbar_width: Pixels::default(),
|
||||
|
|
@ -1042,6 +1040,16 @@ pub struct GutterDimensions {
|
|||
}
|
||||
|
||||
impl GutterDimensions {
|
||||
fn default_with_margin(font_id: FontId, font_size: Pixels, cx: &App) -> Self {
|
||||
Self {
|
||||
margin: Self::default_gutter_margin(font_id, font_size, cx),
|
||||
..Default::default()
|
||||
}
|
||||
}
|
||||
|
||||
fn default_gutter_margin(font_id: FontId, font_size: Pixels, cx: &App) -> Pixels {
|
||||
-cx.text_system().descent(font_id, font_size)
|
||||
}
|
||||
/// The full width of the space taken up by the gutter.
|
||||
pub fn full_width(&self) -> Pixels {
|
||||
self.margin + self.width
|
||||
|
|
@ -20006,7 +20014,6 @@ impl EditorSnapshot {
|
|||
return None;
|
||||
}
|
||||
|
||||
let descent = cx.text_system().descent(font_id, font_size);
|
||||
let em_width = cx.text_system().em_width(font_id, font_size).log_err()?;
|
||||
let em_advance = cx.text_system().em_advance(font_id, font_size).log_err()?;
|
||||
|
||||
|
|
@ -20079,7 +20086,7 @@ impl EditorSnapshot {
|
|||
left_padding,
|
||||
right_padding,
|
||||
width: line_gutter_width + left_padding + right_padding,
|
||||
margin: -descent,
|
||||
margin: GutterDimensions::default_gutter_margin(font_id, font_size, cx),
|
||||
git_blame_entries_width,
|
||||
})
|
||||
}
|
||||
|
|
@ -20284,7 +20291,6 @@ impl Render for Editor {
|
|||
&cx.entity(),
|
||||
EditorStyle {
|
||||
background,
|
||||
horizontal_padding: Pixels::default(),
|
||||
local_player: cx.theme().players().local(),
|
||||
text: text_style,
|
||||
scrollbar_width: EditorElement::SCROLLBAR_WIDTH,
|
||||
|
|
|
|||
|
|
@ -170,7 +170,7 @@ pub struct EditorElement {
|
|||
type DisplayRowDelta = u32;
|
||||
|
||||
impl EditorElement {
|
||||
pub const SCROLLBAR_WIDTH: Pixels = px(15.);
|
||||
pub(crate) const SCROLLBAR_WIDTH: Pixels = px(15.);
|
||||
|
||||
pub fn new(editor: &Entity<Editor>, style: EditorStyle) -> Self {
|
||||
Self {
|
||||
|
|
@ -6781,28 +6781,13 @@ impl Element for EditorElement {
|
|||
self.max_line_number_width(&snapshot, window, cx),
|
||||
cx,
|
||||
)
|
||||
.unwrap_or_default();
|
||||
let hitbox = window.insert_hitbox(bounds, false);
|
||||
let gutter_hitbox =
|
||||
window.insert_hitbox(gutter_bounds(bounds, gutter_dimensions), false);
|
||||
let text_hitbox = window.insert_hitbox(
|
||||
Bounds {
|
||||
origin: gutter_hitbox.top_right()
|
||||
+ point(style.horizontal_padding, Pixels::default()),
|
||||
size: size(
|
||||
bounds.size.width
|
||||
- gutter_dimensions.width
|
||||
- 2. * style.horizontal_padding,
|
||||
bounds.size.height,
|
||||
),
|
||||
},
|
||||
false,
|
||||
);
|
||||
.unwrap_or_else(|| {
|
||||
GutterDimensions::default_with_margin(font_id, font_size, cx)
|
||||
});
|
||||
let text_width = bounds.size.width - gutter_dimensions.width;
|
||||
|
||||
let editor_width = text_hitbox.size.width
|
||||
- gutter_dimensions.margin
|
||||
- em_width
|
||||
- style.scrollbar_width;
|
||||
let editor_width =
|
||||
text_width - gutter_dimensions.margin - em_width - style.scrollbar_width;
|
||||
|
||||
snapshot = self.editor.update(cx, |editor, cx| {
|
||||
editor.last_bounds = Some(bounds);
|
||||
|
|
@ -6838,13 +6823,24 @@ impl Element for EditorElement {
|
|||
.map(|(guide, active)| (self.column_pixels(*guide, window, cx), *active))
|
||||
.collect::<SmallVec<[_; 2]>>();
|
||||
|
||||
let hitbox = window.insert_hitbox(bounds, false);
|
||||
let gutter_hitbox =
|
||||
window.insert_hitbox(gutter_bounds(bounds, gutter_dimensions), false);
|
||||
let text_hitbox = window.insert_hitbox(
|
||||
Bounds {
|
||||
origin: gutter_hitbox.top_right(),
|
||||
size: size(text_width, bounds.size.height),
|
||||
},
|
||||
false,
|
||||
);
|
||||
|
||||
// Offset the content_bounds from the text_bounds by the gutter margin (which
|
||||
// is roughly half a character wide) to make hit testing work more like how we want.
|
||||
let content_offset = point(gutter_dimensions.margin, Pixels::ZERO);
|
||||
let content_origin = text_hitbox.origin + content_offset;
|
||||
|
||||
let editor_text_bounds =
|
||||
Bounds::from_corners(content_origin, text_hitbox.bounds.bottom_right());
|
||||
Bounds::from_corners(content_origin, bounds.bottom_right());
|
||||
|
||||
let height_in_lines = editor_text_bounds.size.height / line_height;
|
||||
|
||||
|
|
@ -8646,7 +8642,7 @@ fn compute_auto_height_layout(
|
|||
let mut snapshot = editor.snapshot(window, cx);
|
||||
let gutter_dimensions = snapshot
|
||||
.gutter_dimensions(font_id, font_size, max_line_number_width, cx)
|
||||
.unwrap_or_default();
|
||||
.unwrap_or_else(|| GutterDimensions::default_with_margin(font_id, font_size, cx));
|
||||
|
||||
editor.gutter_dimensions = gutter_dimensions;
|
||||
let text_width = width - gutter_dimensions.width;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue