From 01503511ad106bac5812f183ef400840d13b5621 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Fri, 8 Nov 2024 13:25:48 -0500 Subject: [PATCH] Fix line number whitespace (#20427) Closes #14025 https://github.com/user-attachments/assets/24b3f321-8246-4203-9510-66a7cf3d22f0 Release Notes: - Fixed bug where toggling line numbers would incorrectly hide whitespace indicators. --- crates/editor/src/element.rs | 163 +++++++++++++++++++++-------------- 1 file changed, 100 insertions(+), 63 deletions(-) diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 2c10273930e2e21f773f6d888c5aa956f6e9ed6b..07e30104fd50e41a6777b266fa261b86c855a664 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -1968,10 +1968,10 @@ impl EditorElement { fn layout_lines( rows: Range, - line_number_layouts: &[Option], snapshot: &EditorSnapshot, style: &EditorStyle, editor_width: Pixels, + is_row_soft_wrapped: impl Copy + Fn(usize) -> bool, cx: &mut WindowContext, ) -> Vec { if rows.start >= rows.end { @@ -2020,9 +2020,9 @@ impl EditorElement { &style.text, MAX_LINE_LEN, rows.len(), - line_number_layouts, snapshot.mode, editor_width, + is_row_soft_wrapped, cx, ) } @@ -2071,6 +2071,7 @@ impl EditorElement { scroll_width: &mut Pixels, resized_blocks: &mut HashMap, selections: &[Selection], + is_row_soft_wrapped: impl Copy + Fn(usize) -> bool, cx: &mut WindowContext, ) -> (AnyElement, Size) { let mut element = match block { @@ -2083,8 +2084,15 @@ impl EditorElement { line_layouts[align_to.row().minus(rows.start) as usize] .x_for_index(align_to.column() as usize) } else { - layout_line(align_to.row(), snapshot, &self.style, editor_width, cx) - .x_for_index(align_to.column() as usize) + layout_line( + align_to.row(), + snapshot, + &self.style, + editor_width, + is_row_soft_wrapped, + cx, + ) + .x_for_index(align_to.column() as usize) }; let selected = selections @@ -2447,6 +2455,7 @@ impl EditorElement { line_height: Pixels, line_layouts: &[LineWithInvisibles], selections: &[Selection], + is_row_soft_wrapped: impl Copy + Fn(usize) -> bool, cx: &mut WindowContext, ) -> Result, HashMap> { let (fixed_blocks, non_fixed_blocks) = snapshot @@ -2484,6 +2493,7 @@ impl EditorElement { scroll_width, &mut resized_blocks, selections, + is_row_soft_wrapped, cx, ); fixed_block_max_width = fixed_block_max_width.max(element_size.width + em_width); @@ -2529,6 +2539,7 @@ impl EditorElement { scroll_width, &mut resized_blocks, selections, + is_row_soft_wrapped, cx, ); @@ -2575,6 +2586,7 @@ impl EditorElement { scroll_width, &mut resized_blocks, selections, + is_row_soft_wrapped, cx, ); @@ -4359,9 +4371,9 @@ impl LineWithInvisibles { text_style: &TextStyle, max_line_len: usize, max_line_count: usize, - line_number_layouts: &[Option], editor_mode: EditorMode, text_width: Pixels, + is_row_soft_wrapped: impl Copy + Fn(usize) -> bool, cx: &mut WindowContext, ) -> Vec { let mut layouts = Vec::with_capacity(max_line_count); @@ -4489,12 +4501,9 @@ impl LineWithInvisibles { if editor_mode == EditorMode::Full { // Line wrap pads its contents with fake whitespaces, // avoid printing them - let inside_wrapped_string = line_number_layouts - .get(row) - .and_then(|layout| layout.as_ref()) - .is_none(); + let is_soft_wrapped = is_row_soft_wrapped(row); if highlighted_chunk.is_tab { - if non_whitespace_added || !inside_wrapped_string { + if non_whitespace_added || !is_soft_wrapped { invisibles.push(Invisible::Tab { line_start_offset: line.len(), line_end_offset: line.len() + line_chunk.len(), @@ -4510,7 +4519,7 @@ impl LineWithInvisibles { (*line_byte as char).is_whitespace(); non_whitespace_added |= !is_whitespace; is_whitespace - && (non_whitespace_added || !inside_wrapped_string) + && (non_whitespace_added || !is_soft_wrapped) }) .map(|(whitespace_index, _)| Invisible::Whitespace { line_offset: line.len() + whitespace_index, @@ -4873,10 +4882,10 @@ impl Element for EditorElement { editor_handle.update(cx, |editor, cx| editor.snapshot(cx)); let line = Self::layout_lines( DisplayRow(0)..DisplayRow(1), - &[], &editor_snapshot, &style, px(f32::MAX), + |_| false, // Single lines never soft wrap cx, ) .pop() @@ -5085,6 +5094,8 @@ impl Element for EditorElement { .buffer_rows(start_row) .take((start_row..end_row).len()) .collect::>(); + let is_row_soft_wrapped = + |row| buffer_rows.get(row).copied().flatten().is_none(); let start_anchor = if start_row == Default::default() { Anchor::min() @@ -5176,10 +5187,10 @@ impl Element for EditorElement { let mut max_visible_line_width = Pixels::ZERO; let mut line_layouts = Self::layout_lines( start_row..end_row, - &line_numbers, &snapshot, &self.style, editor_width, + is_row_soft_wrapped, cx, ); for line_with_invisibles in &line_layouts { @@ -5188,9 +5199,15 @@ impl Element for EditorElement { } } - let longest_line_width = - layout_line(snapshot.longest_row(), &snapshot, &style, editor_width, cx) - .width; + let longest_line_width = layout_line( + snapshot.longest_row(), + &snapshot, + &style, + editor_width, + is_row_soft_wrapped, + cx, + ) + .width; let mut scroll_width = longest_line_width.max(max_visible_line_width) + overscroll.width; @@ -5208,6 +5225,7 @@ impl Element for EditorElement { line_height, &line_layouts, &local_selections, + is_row_soft_wrapped, cx, ) }); @@ -5966,6 +5984,7 @@ fn layout_line( snapshot: &EditorSnapshot, style: &EditorStyle, text_width: Pixels, + is_row_soft_wrapped: impl Copy + Fn(usize) -> bool, cx: &mut WindowContext, ) -> LineWithInvisibles { let chunks = snapshot.highlighted_chunks(row..row + DisplayRow(1), true, style); @@ -5974,9 +5993,9 @@ fn layout_line( &style.text, MAX_LINE_LEN, 1, - &[], snapshot.mode, text_width, + is_row_soft_wrapped, cx, ) .pop() @@ -6661,15 +6680,22 @@ mod tests { "Hardcoded expected invisibles differ from the actual ones in '{input_text}'" ); - init_test(cx, |s| { - s.defaults.show_whitespaces = Some(ShowWhitespaceSetting::All); - s.defaults.tab_size = NonZeroU32::new(TAB_SIZE); - }); + for show_line_numbers in [true, false] { + init_test(cx, |s| { + s.defaults.show_whitespaces = Some(ShowWhitespaceSetting::All); + s.defaults.tab_size = NonZeroU32::new(TAB_SIZE); + }); - let actual_invisibles = - collect_invisibles_from_new_editor(cx, EditorMode::Full, input_text, px(500.0)); + let actual_invisibles = collect_invisibles_from_new_editor( + cx, + EditorMode::Full, + input_text, + px(500.0), + show_line_numbers, + ); - assert_eq!(expected_invisibles, actual_invisibles); + assert_eq!(expected_invisibles, actual_invisibles); + } } #[gpui::test] @@ -6683,14 +6709,17 @@ mod tests { EditorMode::SingleLine { auto_width: false }, EditorMode::AutoHeight { max_lines: 100 }, ] { - let invisibles = collect_invisibles_from_new_editor( - cx, - editor_mode_without_invisibles, - "\t\t\t| | a b", - px(500.0), - ); - assert!(invisibles.is_empty(), + for show_line_numbers in [true, false] { + let invisibles = collect_invisibles_from_new_editor( + cx, + editor_mode_without_invisibles, + "\t\t\t| | a b", + px(500.0), + show_line_numbers, + ); + assert!(invisibles.is_empty(), "For editor mode {editor_mode_without_invisibles:?} no invisibles was expected but got {invisibles:?}"); + } } } @@ -6741,43 +6770,48 @@ mod tests { let resize_step = 10.0; let mut editor_width = 200.0; while editor_width <= 1000.0 { - update_test_language_settings(cx, |s| { - s.defaults.tab_size = NonZeroU32::new(tab_size); - s.defaults.show_whitespaces = Some(ShowWhitespaceSetting::All); - s.defaults.preferred_line_length = Some(editor_width as u32); - s.defaults.soft_wrap = Some(language_settings::SoftWrap::PreferredLineLength); - }); + for show_line_numbers in [true, false] { + update_test_language_settings(cx, |s| { + s.defaults.tab_size = NonZeroU32::new(tab_size); + s.defaults.show_whitespaces = Some(ShowWhitespaceSetting::All); + s.defaults.preferred_line_length = Some(editor_width as u32); + s.defaults.soft_wrap = Some(language_settings::SoftWrap::PreferredLineLength); + }); - let actual_invisibles = collect_invisibles_from_new_editor( - cx, - EditorMode::Full, - &input_text, - px(editor_width), - ); + let actual_invisibles = collect_invisibles_from_new_editor( + cx, + EditorMode::Full, + &input_text, + px(editor_width), + show_line_numbers, + ); - // Whatever the editor size is, ensure it has the same invisible kinds in the same order - // (no good guarantees about the offsets: wrapping could trigger padding and its tests should check the offsets). - let mut i = 0; - for (actual_index, actual_invisible) in actual_invisibles.iter().enumerate() { - i = actual_index; - match expected_invisibles.get(i) { - Some(expected_invisible) => match (expected_invisible, actual_invisible) { - (Invisible::Whitespace { .. }, Invisible::Whitespace { .. }) - | (Invisible::Tab { .. }, Invisible::Tab { .. }) => {} - _ => { - panic!("At index {i}, expected invisible {expected_invisible:?} does not match actual {actual_invisible:?} by kind. Actual invisibles: {actual_invisibles:?}") + // Whatever the editor size is, ensure it has the same invisible kinds in the same order + // (no good guarantees about the offsets: wrapping could trigger padding and its tests should check the offsets). + let mut i = 0; + for (actual_index, actual_invisible) in actual_invisibles.iter().enumerate() { + i = actual_index; + match expected_invisibles.get(i) { + Some(expected_invisible) => match (expected_invisible, actual_invisible) { + (Invisible::Whitespace { .. }, Invisible::Whitespace { .. }) + | (Invisible::Tab { .. }, Invisible::Tab { .. }) => {} + _ => { + panic!("At index {i}, expected invisible {expected_invisible:?} does not match actual {actual_invisible:?} by kind. Actual invisibles: {actual_invisibles:?}") + } + }, + None => { + panic!("Unexpected extra invisible {actual_invisible:?} at index {i}") } - }, - None => panic!("Unexpected extra invisible {actual_invisible:?} at index {i}"), + } } - } - let missing_expected_invisibles = &expected_invisibles[i + 1..]; - assert!( - missing_expected_invisibles.is_empty(), - "Missing expected invisibles after index {i}: {missing_expected_invisibles:?}" - ); + let missing_expected_invisibles = &expected_invisibles[i + 1..]; + assert!( + missing_expected_invisibles.is_empty(), + "Missing expected invisibles after index {i}: {missing_expected_invisibles:?}" + ); - editor_width += resize_step; + editor_width += resize_step; + } } } @@ -6786,6 +6820,7 @@ mod tests { editor_mode: EditorMode, input_text: &str, editor_width: Pixels, + show_line_numbers: bool, ) -> Vec { info!( "Creating editor with mode {editor_mode:?}, width {}px and text '{input_text}'", @@ -6797,11 +6832,13 @@ mod tests { }); let cx = &mut VisualTestContext::from_window(*window, cx); let editor = window.root(cx).unwrap(); + let style = cx.update(|cx| editor.read(cx).style().unwrap().clone()); window .update(cx, |editor, cx| { editor.set_soft_wrap_mode(language_settings::SoftWrap::EditorWidth, cx); editor.set_wrap_width(Some(editor_width), cx); + editor.set_show_line_numbers(show_line_numbers, cx); }) .unwrap(); let (_, state) = cx.draw(point(px(500.), px(500.)), size(px(500.), px(500.)), |_| {