Support relative line number on wrapped lines (rework) (#41805)
## Add relative line numbers on wrapped lines, take 2 This is a rework of https://github.com/zed-industries/zed/pull/39268 that excludes https://github.com/zed-industries/zed/pull/39268/commits/e7096d27a6463f6eb7c2a821637c5773b2460c10. This commit introduced some line number rendering issues as described in https://github.com/zed-industries/zed/issues/41422. While @ConradIrwin suggested we try to pass in the buffer rows from the calling method instead of the snapshot, that appears to have had unintended consequences and I don't think the two calculations were intended to do the same thing. Hence, this PR has removed those changes. This PR also includes the migration fix originally done by @MrSubidubi in https://github.com/zed-industries/zed/pull/41351. ## Original PR description and release notes. **Problem:** Current relative line numbering creates a mismatch with vim-style navigation when soft wrap is enabled. Users must mentally calculate whether target lines are wrapped segments or logical lines, making `<n>j/k` navigation unreliable and cognitively demanding. **How things work today:** - Real line navigation (`j/k` moves by logical lines): Requires determining if visible lines are wrapped segments before jumping. Can't jump to wrapped lines directly. - Display line navigation (`j/k` moves by display rows): Line numbers don't correspond to actual row distances for multi-line jumps. **Proposed solution:** Count and number each display line (including wrapped segments) for relative numbering. This creates direct visual-to-navigational correspondence, where the relative number shown always matches the `<n>j/k` distance needed. **Benefits:** - Eliminates mental overhead of distinguishing wrapped vs. logical lines - Makes relative line numbers consistently actionable regardless of wrap state - Preserves intuitive "what you see is what you navigate" principle - Maintains vim workflow efficiency in narrow window scenarios Also explained and discussed in https://github.com/zed-industries/zed/discussions/25733. Release Notes: - Added support for counting wrapped lines as relative lines and for displaying line numbers for wrapped segments. Changes `relative_line_numbers` from a boolean to an enum: `enabled`, `disabled`, or `wrapped`.
This commit is contained in:
@@ -1025,6 +1025,7 @@ impl Iterator for WrapRows<'_> {
|
||||
multibuffer_row: None,
|
||||
diff_status,
|
||||
expand_info: None,
|
||||
wrapped_buffer_row: buffer_row.buffer_row,
|
||||
}
|
||||
} else {
|
||||
buffer_row
|
||||
|
||||
@@ -163,7 +163,10 @@ use rpc::{ErrorCode, ErrorExt, proto::PeerId};
|
||||
use scroll::{Autoscroll, OngoingScroll, ScrollAnchor, ScrollManager};
|
||||
use selections_collection::{MutableSelectionsCollection, SelectionsCollection};
|
||||
use serde::{Deserialize, Serialize};
|
||||
use settings::{GitGutterSetting, Settings, SettingsLocation, SettingsStore, update_settings_file};
|
||||
use settings::{
|
||||
GitGutterSetting, RelativeLineNumbers, Settings, SettingsLocation, SettingsStore,
|
||||
update_settings_file,
|
||||
};
|
||||
use smallvec::{SmallVec, smallvec};
|
||||
use snippet::Snippet;
|
||||
use std::{
|
||||
@@ -19542,9 +19545,16 @@ impl Editor {
|
||||
EditorSettings::get_global(cx).gutter.line_numbers
|
||||
}
|
||||
|
||||
pub fn should_use_relative_line_numbers(&self, cx: &mut App) -> bool {
|
||||
self.use_relative_line_numbers
|
||||
.unwrap_or(EditorSettings::get_global(cx).relative_line_numbers)
|
||||
pub fn relative_line_numbers(&self, cx: &mut App) -> RelativeLineNumbers {
|
||||
match (
|
||||
self.use_relative_line_numbers,
|
||||
EditorSettings::get_global(cx).relative_line_numbers,
|
||||
) {
|
||||
(None, setting) => setting,
|
||||
(Some(false), _) => RelativeLineNumbers::Disabled,
|
||||
(Some(true), RelativeLineNumbers::Wrapped) => RelativeLineNumbers::Wrapped,
|
||||
(Some(true), _) => RelativeLineNumbers::Enabled,
|
||||
}
|
||||
}
|
||||
|
||||
pub fn toggle_relative_line_numbers(
|
||||
@@ -19553,8 +19563,8 @@ impl Editor {
|
||||
_: &mut Window,
|
||||
cx: &mut Context<Self>,
|
||||
) {
|
||||
let is_relative = self.should_use_relative_line_numbers(cx);
|
||||
self.set_relative_line_number(Some(!is_relative), cx)
|
||||
let is_relative = self.relative_line_numbers(cx);
|
||||
self.set_relative_line_number(Some(!is_relative.enabled()), cx)
|
||||
}
|
||||
|
||||
pub fn set_relative_line_number(&mut self, is_relative: Option<bool>, cx: &mut Context<Self>) {
|
||||
|
||||
@@ -3,12 +3,12 @@ use core::num;
|
||||
use gpui::App;
|
||||
use language::CursorShape;
|
||||
use project::project_settings::DiagnosticSeverity;
|
||||
use settings::Settings;
|
||||
pub use settings::{
|
||||
CurrentLineHighlight, DelayMs, DisplayIn, DocumentColorsRenderMode, DoubleClickInMultibuffer,
|
||||
GoToDefinitionFallback, HideMouseMode, MinimapThumb, MinimapThumbBorder, MultiCursorModifier,
|
||||
ScrollBeyondLastLine, ScrollbarDiagnostics, SeedQuerySetting, ShowMinimap, SnippetSortOrder,
|
||||
};
|
||||
use settings::{RelativeLineNumbers, Settings};
|
||||
use ui::scrollbars::{ScrollbarVisibility, ShowScrollbar};
|
||||
|
||||
/// Imports from the VSCode settings at
|
||||
@@ -33,7 +33,7 @@ pub struct EditorSettings {
|
||||
pub horizontal_scroll_margin: f32,
|
||||
pub scroll_sensitivity: f32,
|
||||
pub fast_scroll_sensitivity: f32,
|
||||
pub relative_line_numbers: bool,
|
||||
pub relative_line_numbers: RelativeLineNumbers,
|
||||
pub seed_search_query_from_cursor: SeedQuerySetting,
|
||||
pub use_smartcase_search: bool,
|
||||
pub multi_cursor_modifier: MultiCursorModifier,
|
||||
|
||||
+216
-96
@@ -766,8 +766,14 @@ impl EditorElement {
|
||||
.row;
|
||||
if line_numbers
|
||||
.get(&MultiBufferRow(multi_buffer_row))
|
||||
.and_then(|line_number| line_number.hitbox.as_ref())
|
||||
.is_some_and(|hitbox| hitbox.contains(&event.position))
|
||||
.is_some_and(|line_layout| {
|
||||
line_layout.segments.iter().any(|segment| {
|
||||
segment
|
||||
.hitbox
|
||||
.as_ref()
|
||||
.is_some_and(|hitbox| hitbox.contains(&event.position))
|
||||
})
|
||||
})
|
||||
{
|
||||
let line_offset_from_top = display_row - position_map.scroll_position.y as u32;
|
||||
|
||||
@@ -3157,6 +3163,7 @@ impl EditorElement {
|
||||
snapshot: &EditorSnapshot,
|
||||
rows: &Range<DisplayRow>,
|
||||
relative_to: Option<DisplayRow>,
|
||||
count_wrapped_lines: bool,
|
||||
) -> HashMap<DisplayRow, DisplayRowDelta> {
|
||||
let mut relative_rows: HashMap<DisplayRow, DisplayRowDelta> = Default::default();
|
||||
let Some(relative_to) = relative_to else {
|
||||
@@ -3174,8 +3181,15 @@ impl EditorElement {
|
||||
let head_idx = relative_to.minus(start);
|
||||
let mut delta = 1;
|
||||
let mut i = head_idx + 1;
|
||||
let should_count_line = |row_info: &RowInfo| {
|
||||
if count_wrapped_lines {
|
||||
row_info.buffer_row.is_some() || row_info.wrapped_buffer_row.is_some()
|
||||
} else {
|
||||
row_info.buffer_row.is_some()
|
||||
}
|
||||
};
|
||||
while i < buffer_rows.len() as u32 {
|
||||
if buffer_rows[i as usize].buffer_row.is_some() {
|
||||
if should_count_line(&buffer_rows[i as usize]) {
|
||||
if rows.contains(&DisplayRow(i + start.0)) {
|
||||
relative_rows.insert(DisplayRow(i + start.0), delta);
|
||||
}
|
||||
@@ -3185,13 +3199,13 @@ impl EditorElement {
|
||||
}
|
||||
delta = 1;
|
||||
i = head_idx.min(buffer_rows.len().saturating_sub(1) as u32);
|
||||
while i > 0 && buffer_rows[i as usize].buffer_row.is_none() {
|
||||
while i > 0 && buffer_rows[i as usize].buffer_row.is_none() && !count_wrapped_lines {
|
||||
i -= 1;
|
||||
}
|
||||
|
||||
while i > 0 {
|
||||
i -= 1;
|
||||
if buffer_rows[i as usize].buffer_row.is_some() {
|
||||
if should_count_line(&buffer_rows[i as usize]) {
|
||||
if rows.contains(&DisplayRow(i + start.0)) {
|
||||
relative_rows.insert(DisplayRow(i + start.0), delta);
|
||||
}
|
||||
@@ -3223,7 +3237,7 @@ impl EditorElement {
|
||||
return Arc::default();
|
||||
}
|
||||
|
||||
let (newest_selection_head, is_relative) = self.editor.update(cx, |editor, cx| {
|
||||
let (newest_selection_head, relative) = self.editor.update(cx, |editor, cx| {
|
||||
let newest_selection_head = newest_selection_head.unwrap_or_else(|| {
|
||||
let newest = editor
|
||||
.selections
|
||||
@@ -3239,79 +3253,93 @@ impl EditorElement {
|
||||
)
|
||||
.head
|
||||
});
|
||||
let is_relative = editor.should_use_relative_line_numbers(cx);
|
||||
(newest_selection_head, is_relative)
|
||||
let relative = editor.relative_line_numbers(cx);
|
||||
(newest_selection_head, relative)
|
||||
});
|
||||
|
||||
let relative_to = if is_relative {
|
||||
let relative_to = if relative.enabled() {
|
||||
Some(newest_selection_head.row())
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let relative_rows = self.calculate_relative_line_numbers(snapshot, &rows, relative_to);
|
||||
let relative_rows =
|
||||
self.calculate_relative_line_numbers(snapshot, &rows, relative_to, relative.wrapped());
|
||||
let mut line_number = String::new();
|
||||
let line_numbers = buffer_rows
|
||||
.iter()
|
||||
.enumerate()
|
||||
.flat_map(|(ix, row_info)| {
|
||||
let display_row = DisplayRow(rows.start.0 + ix as u32);
|
||||
line_number.clear();
|
||||
let non_relative_number = row_info.buffer_row? + 1;
|
||||
let number = relative_rows
|
||||
.get(&display_row)
|
||||
.unwrap_or(&non_relative_number);
|
||||
write!(&mut line_number, "{number}").unwrap();
|
||||
if row_info
|
||||
.diff_status
|
||||
.is_some_and(|status| status.is_deleted())
|
||||
{
|
||||
return None;
|
||||
}
|
||||
let segments = buffer_rows.iter().enumerate().flat_map(|(ix, row_info)| {
|
||||
let display_row = DisplayRow(rows.start.0 + ix as u32);
|
||||
line_number.clear();
|
||||
let non_relative_number = if relative.wrapped() {
|
||||
row_info.buffer_row.or(row_info.wrapped_buffer_row)? + 1
|
||||
} else {
|
||||
row_info.buffer_row? + 1
|
||||
};
|
||||
let number = relative_rows
|
||||
.get(&display_row)
|
||||
.unwrap_or(&non_relative_number);
|
||||
write!(&mut line_number, "{number}").unwrap();
|
||||
if row_info
|
||||
.diff_status
|
||||
.is_some_and(|status| status.is_deleted())
|
||||
{
|
||||
return None;
|
||||
}
|
||||
|
||||
let color = active_rows
|
||||
.get(&display_row)
|
||||
.map(|spec| {
|
||||
if spec.breakpoint {
|
||||
cx.theme().colors().debugger_accent
|
||||
} else {
|
||||
cx.theme().colors().editor_active_line_number
|
||||
}
|
||||
})
|
||||
.unwrap_or_else(|| cx.theme().colors().editor_line_number);
|
||||
let shaped_line =
|
||||
self.shape_line_number(SharedString::from(&line_number), color, window);
|
||||
let scroll_top = scroll_position.y * ScrollPixelOffset::from(line_height);
|
||||
let line_origin = gutter_hitbox.map(|hitbox| {
|
||||
hitbox.origin
|
||||
+ point(
|
||||
hitbox.size.width - shaped_line.width - gutter_dimensions.right_padding,
|
||||
ix as f32 * line_height
|
||||
- Pixels::from(scroll_top % ScrollPixelOffset::from(line_height)),
|
||||
)
|
||||
});
|
||||
|
||||
#[cfg(not(test))]
|
||||
let hitbox = line_origin.map(|line_origin| {
|
||||
window.insert_hitbox(
|
||||
Bounds::new(line_origin, size(shaped_line.width, line_height)),
|
||||
HitboxBehavior::Normal,
|
||||
let color = active_rows
|
||||
.get(&display_row)
|
||||
.map(|spec| {
|
||||
if spec.breakpoint {
|
||||
cx.theme().colors().debugger_accent
|
||||
} else {
|
||||
cx.theme().colors().editor_active_line_number
|
||||
}
|
||||
})
|
||||
.unwrap_or_else(|| cx.theme().colors().editor_line_number);
|
||||
let shaped_line =
|
||||
self.shape_line_number(SharedString::from(&line_number), color, window);
|
||||
let scroll_top = scroll_position.y * ScrollPixelOffset::from(line_height);
|
||||
let line_origin = gutter_hitbox.map(|hitbox| {
|
||||
hitbox.origin
|
||||
+ point(
|
||||
hitbox.size.width - shaped_line.width - gutter_dimensions.right_padding,
|
||||
ix as f32 * line_height
|
||||
- Pixels::from(scroll_top % ScrollPixelOffset::from(line_height)),
|
||||
)
|
||||
});
|
||||
#[cfg(test)]
|
||||
let hitbox = {
|
||||
let _ = line_origin;
|
||||
None
|
||||
};
|
||||
});
|
||||
|
||||
let multi_buffer_row = DisplayPoint::new(display_row, 0).to_point(snapshot).row;
|
||||
let multi_buffer_row = MultiBufferRow(multi_buffer_row);
|
||||
let line_number = LineNumberLayout {
|
||||
shaped_line,
|
||||
hitbox,
|
||||
};
|
||||
Some((multi_buffer_row, line_number))
|
||||
})
|
||||
.collect();
|
||||
#[cfg(not(test))]
|
||||
let hitbox = line_origin.map(|line_origin| {
|
||||
window.insert_hitbox(
|
||||
Bounds::new(line_origin, size(shaped_line.width, line_height)),
|
||||
HitboxBehavior::Normal,
|
||||
)
|
||||
});
|
||||
#[cfg(test)]
|
||||
let hitbox = {
|
||||
let _ = line_origin;
|
||||
None
|
||||
};
|
||||
|
||||
let segment = LineNumberSegment {
|
||||
shaped_line,
|
||||
hitbox,
|
||||
};
|
||||
|
||||
let buffer_row = DisplayPoint::new(display_row, 0).to_point(snapshot).row;
|
||||
let multi_buffer_row = MultiBufferRow(buffer_row);
|
||||
|
||||
Some((multi_buffer_row, segment))
|
||||
});
|
||||
|
||||
let mut line_numbers: HashMap<MultiBufferRow, LineNumberLayout> = HashMap::default();
|
||||
for (buffer_row, segment) in segments {
|
||||
line_numbers
|
||||
.entry(buffer_row)
|
||||
.or_insert_with(|| LineNumberLayout {
|
||||
segments: Default::default(),
|
||||
})
|
||||
.segments
|
||||
.push(segment);
|
||||
}
|
||||
Arc::new(line_numbers)
|
||||
}
|
||||
|
||||
@@ -5852,34 +5880,36 @@ impl EditorElement {
|
||||
let line_height = layout.position_map.line_height;
|
||||
window.set_cursor_style(CursorStyle::Arrow, &layout.gutter_hitbox);
|
||||
|
||||
for LineNumberLayout {
|
||||
shaped_line,
|
||||
hitbox,
|
||||
} in layout.line_numbers.values()
|
||||
{
|
||||
let Some(hitbox) = hitbox else {
|
||||
continue;
|
||||
};
|
||||
for line_layout in layout.line_numbers.values() {
|
||||
for LineNumberSegment {
|
||||
shaped_line,
|
||||
hitbox,
|
||||
} in &line_layout.segments
|
||||
{
|
||||
let Some(hitbox) = hitbox else {
|
||||
continue;
|
||||
};
|
||||
|
||||
let Some(()) = (if !is_singleton && hitbox.is_hovered(window) {
|
||||
let color = cx.theme().colors().editor_hover_line_number;
|
||||
let Some(()) = (if !is_singleton && hitbox.is_hovered(window) {
|
||||
let color = cx.theme().colors().editor_hover_line_number;
|
||||
|
||||
let line = self.shape_line_number(shaped_line.text.clone(), color, window);
|
||||
line.paint(hitbox.origin, line_height, window, cx).log_err()
|
||||
} else {
|
||||
shaped_line
|
||||
.paint(hitbox.origin, line_height, window, cx)
|
||||
.log_err()
|
||||
}) else {
|
||||
continue;
|
||||
};
|
||||
let line = self.shape_line_number(shaped_line.text.clone(), color, window);
|
||||
line.paint(hitbox.origin, line_height, window, cx).log_err()
|
||||
} else {
|
||||
shaped_line
|
||||
.paint(hitbox.origin, line_height, window, cx)
|
||||
.log_err()
|
||||
}) else {
|
||||
continue;
|
||||
};
|
||||
|
||||
// In singleton buffers, we select corresponding lines on the line number click, so use | -like cursor.
|
||||
// In multi buffers, we open file at the line number clicked, so use a pointing hand cursor.
|
||||
if is_singleton {
|
||||
window.set_cursor_style(CursorStyle::IBeam, hitbox);
|
||||
} else {
|
||||
window.set_cursor_style(CursorStyle::PointingHand, hitbox);
|
||||
// In singleton buffers, we select corresponding lines on the line number click, so use | -like cursor.
|
||||
// In multi buffers, we open file at the line number clicked, so use a pointing hand cursor.
|
||||
if is_singleton {
|
||||
window.set_cursor_style(CursorStyle::IBeam, hitbox);
|
||||
} else {
|
||||
window.set_cursor_style(CursorStyle::PointingHand, hitbox);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -9793,11 +9823,17 @@ impl EditorLayout {
|
||||
}
|
||||
}
|
||||
|
||||
struct LineNumberLayout {
|
||||
#[derive(Debug)]
|
||||
struct LineNumberSegment {
|
||||
shaped_line: ShapedLine,
|
||||
hitbox: Option<Hitbox>,
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
struct LineNumberLayout {
|
||||
segments: SmallVec<[LineNumberSegment; 1]>,
|
||||
}
|
||||
|
||||
struct ColoredRange<T> {
|
||||
start: T,
|
||||
end: T,
|
||||
@@ -10861,6 +10897,7 @@ mod tests {
|
||||
&snapshot,
|
||||
&(DisplayRow(0)..DisplayRow(6)),
|
||||
Some(DisplayRow(3)),
|
||||
false,
|
||||
)
|
||||
})
|
||||
.unwrap();
|
||||
@@ -10879,6 +10916,7 @@ mod tests {
|
||||
&snapshot,
|
||||
&(DisplayRow(3)..DisplayRow(6)),
|
||||
Some(DisplayRow(1)),
|
||||
false,
|
||||
)
|
||||
})
|
||||
.unwrap();
|
||||
@@ -10895,6 +10933,7 @@ mod tests {
|
||||
&snapshot,
|
||||
&(DisplayRow(0)..DisplayRow(3)),
|
||||
Some(DisplayRow(6)),
|
||||
false,
|
||||
)
|
||||
})
|
||||
.unwrap();
|
||||
@@ -10904,6 +10943,81 @@ mod tests {
|
||||
assert_eq!(relative_rows[&DisplayRow(2)], 3);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
fn test_shape_line_numbers_wrapping(cx: &mut TestAppContext) {
|
||||
init_test(cx, |_| {});
|
||||
let window = cx.add_window(|window, cx| {
|
||||
let buffer = MultiBuffer::build_simple(&sample_text(6, 6, 'a'), cx);
|
||||
Editor::new(EditorMode::full(), buffer, None, window, cx)
|
||||
});
|
||||
|
||||
update_test_language_settings(cx, |s| {
|
||||
s.defaults.preferred_line_length = Some(5_u32);
|
||||
s.defaults.soft_wrap = Some(language_settings::SoftWrap::PreferredLineLength);
|
||||
});
|
||||
|
||||
let editor = window.root(cx).unwrap();
|
||||
let style = cx.update(|cx| editor.read(cx).style().unwrap().clone());
|
||||
let line_height = window
|
||||
.update(cx, |_, window, _| {
|
||||
style.text.line_height_in_pixels(window.rem_size())
|
||||
})
|
||||
.unwrap();
|
||||
let element = EditorElement::new(&editor, style);
|
||||
let snapshot = window
|
||||
.update(cx, |editor, window, cx| editor.snapshot(window, cx))
|
||||
.unwrap();
|
||||
|
||||
let layouts = cx
|
||||
.update_window(*window, |_, window, cx| {
|
||||
element.layout_line_numbers(
|
||||
None,
|
||||
GutterDimensions {
|
||||
left_padding: Pixels::ZERO,
|
||||
right_padding: Pixels::ZERO,
|
||||
width: px(30.0),
|
||||
margin: Pixels::ZERO,
|
||||
git_blame_entries_width: None,
|
||||
},
|
||||
line_height,
|
||||
gpui::Point::default(),
|
||||
DisplayRow(0)..DisplayRow(6),
|
||||
&(0..6)
|
||||
.map(|row| RowInfo {
|
||||
buffer_row: Some(row),
|
||||
..Default::default()
|
||||
})
|
||||
.collect::<Vec<_>>(),
|
||||
&BTreeMap::default(),
|
||||
Some(DisplayPoint::new(DisplayRow(0), 0)),
|
||||
&snapshot,
|
||||
window,
|
||||
cx,
|
||||
)
|
||||
})
|
||||
.unwrap();
|
||||
assert_eq!(layouts.len(), 3);
|
||||
|
||||
let relative_rows = window
|
||||
.update(cx, |editor, window, cx| {
|
||||
let snapshot = editor.snapshot(window, cx);
|
||||
element.calculate_relative_line_numbers(
|
||||
&snapshot,
|
||||
&(DisplayRow(0)..DisplayRow(6)),
|
||||
Some(DisplayRow(3)),
|
||||
true,
|
||||
)
|
||||
})
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(relative_rows[&DisplayRow(0)], 3);
|
||||
assert_eq!(relative_rows[&DisplayRow(1)], 2);
|
||||
assert_eq!(relative_rows[&DisplayRow(2)], 1);
|
||||
// current line has no relative number
|
||||
assert_eq!(relative_rows[&DisplayRow(4)], 1);
|
||||
assert_eq!(relative_rows[&DisplayRow(5)], 2);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_vim_visual_selections(cx: &mut TestAppContext) {
|
||||
init_test(cx, |_| {});
|
||||
@@ -11017,7 +11131,13 @@ mod tests {
|
||||
state
|
||||
.line_numbers
|
||||
.get(&MultiBufferRow(0))
|
||||
.map(|line_number| line_number.shaped_line.text.as_ref()),
|
||||
.map(|line_number| line_number
|
||||
.segments
|
||||
.first()
|
||||
.unwrap()
|
||||
.shaped_line
|
||||
.text
|
||||
.as_ref()),
|
||||
Some("1")
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user