diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 413c50d262..d41143d12f 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -2279,14 +2279,21 @@ impl Buffer { }) } - /// Spawns a background task that searches the buffer for any whitespace - /// at the ends of a lines, and returns a `Diff` that removes that whitespace. - pub fn remove_trailing_whitespace(&self, cx: &App) -> Task { + /// Spawns a background task that returns a `Diff` removing trailing whitespace from line ends. + /// + /// When `modified_rows` is `Some`, only lines whose row falls within one of the given ranges + /// are trimmed; when it is `None`, the whole buffer is scanned. + pub fn remove_trailing_whitespace( + &self, + modified_rows: Option<&[Range]>, + cx: &App, + ) -> Task { let old_text = self.as_rope().clone(); let line_ending = self.line_ending(); let base_version = self.version(); + let modified_rows = modified_rows.map(|rows| rows.to_vec()); cx.background_spawn(async move { - let ranges = trailing_whitespace_ranges(&old_text); + let ranges = trailing_whitespace_ranges(&old_text, modified_rows.as_deref()); let empty = Arc::::from(""); Diff { base_version, @@ -2299,28 +2306,60 @@ impl Buffer { }) } - /// Ensures that the buffer ends with a single newline character, and - /// no other whitespace. Skips if the buffer is empty. - pub fn ensure_final_newline(&mut self, cx: &mut Context) { + /// Returns a `Diff` ensuring the buffer ends with a trailing newline. + /// + /// When `modified_rows` is `None`, the whole buffer is considered: trailing whitespace and + /// blank lines at the end of the file are collapsed into a single newline. + /// + /// When `modified_rows` is `Some`, the operation is scoped to a "Format Selection": a single + /// newline is appended only when the last line is non-empty and its row falls within one of + /// the ranges. Trailing blank lines are left intact so that formatting a selection cannot + /// delete unselected rows. + pub fn ensure_final_newline(&self, modified_rows: Option<&[Range]>) -> Diff { let len = self.len(); - if len == 0 { - return; - } - let mut offset = len; - for chunk in self.as_rope().reversed_chunks_in_range(0..len) { - let non_whitespace_len = chunk - .trim_end_matches(|c: char| c.is_ascii_whitespace()) - .len(); - offset -= chunk.len(); - offset += non_whitespace_len; - if non_whitespace_len != 0 { - if offset == len - 1 && chunk.get(non_whitespace_len..) == Some("\n") { - return; - } - break; + let line_ending = self.line_ending(); + let base_version = self.version(); + let newline = Arc::::from("\n"); + + let edits = if len == 0 { + Vec::new() + } else if let Some(modified_rows) = modified_rows { + let max_point = self.max_point(); + let last_line_is_empty = max_point.column == 0; + let last_row_is_modified = modified_rows + .iter() + .any(|range| range.contains(&max_point.row)); + if last_line_is_empty || !last_row_is_modified { + Vec::new() + } else { + Vec::from([(len..len, newline)]) } + } else { + let mut offset = len; + let mut already_normalized = false; + for chunk in self.as_rope().reversed_chunks_in_range(0..len) { + let non_whitespace_len = chunk + .trim_end_matches(|c: char| c.is_ascii_whitespace()) + .len(); + offset -= chunk.len(); + offset += non_whitespace_len; + if non_whitespace_len != 0 { + already_normalized = + offset == len - 1 && chunk.get(non_whitespace_len..) == Some("\n"); + break; + } + } + if already_normalized { + Vec::new() + } else { + Vec::from([(offset..len, newline)]) + } + }; + Diff { + base_version, + line_ending, + edits, } - self.edit([(offset..len, "\n")], None, cx); } /// Applies a diff to the buffer. If the buffer has changed since the given diff was @@ -6133,10 +6172,24 @@ impl CharClassifier { /// /// This could also be done with a regex search, but this implementation /// avoids copying text. -pub fn trailing_whitespace_ranges(rope: &Rope) -> Vec> { +/// Returns the byte ranges of trailing whitespace at the end of each line. +/// +/// When `row_ranges` is `Some`, only lines whose row falls within one of the ranges are +/// included. The single pass keeps filtering cheap, avoiding collecting every range up front. +pub(crate) fn trailing_whitespace_ranges( + rope: &Rope, + row_ranges: Option<&[Range]>, +) -> Vec> { let mut ranges = Vec::new(); + let is_row_included = |row: u32| match row_ranges { + Some(row_ranges) => row_ranges.iter().any(|range| range.contains(&row)), + None => true, + }; + let mut offset = 0; + let mut current_row: u32 = 0; + let mut prev_row: Option = None; let mut prev_chunk_trailing_whitespace_range = 0..0; for chunk in rope.chunks() { let mut prev_line_trailing_whitespace_range = 0..0; @@ -6148,19 +6201,24 @@ pub fn trailing_whitespace_ranges(rope: &Rope) -> Vec> { if i == 0 && trimmed_line_len == 0 { trailing_whitespace_range.start = prev_chunk_trailing_whitespace_range.start; } - if !prev_line_trailing_whitespace_range.is_empty() { - ranges.push(prev_line_trailing_whitespace_range); + if let Some(row) = prev_row { + if !prev_line_trailing_whitespace_range.is_empty() && is_row_included(row) { + ranges.push(prev_line_trailing_whitespace_range); + } } + prev_row = Some(current_row); offset = line_end_offset + 1; + current_row += 1; prev_line_trailing_whitespace_range = trailing_whitespace_range; } offset -= 1; + current_row -= 1; prev_chunk_trailing_whitespace_range = prev_line_trailing_whitespace_range; } - if !prev_chunk_trailing_whitespace_range.is_empty() { + if !prev_chunk_trailing_whitespace_range.is_empty() && is_row_included(current_row) { ranges.push(prev_chunk_trailing_whitespace_range); } diff --git a/crates/language/src/buffer_tests.rs b/crates/language/src/buffer_tests.rs index ba7fd7ea95..cdcd90314c 100644 --- a/crates/language/src/buffer_tests.rs +++ b/crates/language/src/buffer_tests.rs @@ -571,7 +571,7 @@ async fn test_normalize_whitespace(cx: &mut gpui::TestAppContext) { // Spawn a task to format the buffer's whitespace. // Pause so that the formatting task starts running. - let format = buffer.update(cx, |buffer, cx| buffer.remove_trailing_whitespace(cx)); + let format = buffer.update(cx, |buffer, cx| buffer.remove_trailing_whitespace(None, cx)); yield_now().await; // Edit the buffer while the normalization task is running. @@ -3792,7 +3792,7 @@ fn test_trailing_whitespace_ranges(mut rng: StdRng) { } let rope = Rope::from(text.as_str()); - let actual_ranges = trailing_whitespace_ranges(&rope); + let actual_ranges = trailing_whitespace_ranges(&rope, None); let expected_ranges = TRAILING_WHITESPACE_REGEX .find_iter(&text) .map(|m| m.range()) @@ -3805,6 +3805,228 @@ fn test_trailing_whitespace_ranges(mut rng: StdRng) { ); } +#[gpui::test(iterations = 500)] +fn test_trailing_whitespace_ranges_in_rows(mut rng: StdRng) { + let mut text = String::new(); + for _ in 0..rng.random_range(0..16) { + for _ in 0..rng.random_range(0..36) { + text.push(match rng.random_range(0..10) { + 0..=1 => ' ', + 3 => '\t', + _ => rng.random_range('a'..='z'), + }); + } + text.push('\n'); + } + match rng.random_range(0..10) { + 0..=1 => drop(text.pop()), + 2..=3 => text.push_str(&"\n".repeat(rng.random_range(1..5))), + _ => {} + } + + let rope = Rope::from(text.as_str()); + let all_ranges = trailing_whitespace_ranges(&rope, None); + let lines = text.split('\n').collect::>(); + + // A range covering every row must reproduce the unfiltered full scan exactly. + assert_eq!( + trailing_whitespace_ranges(&rope, Some(&[0..u32::MAX])), + all_ranges, + "full-coverage mismatch for lines:\n{lines:?}", + ); + + // For a random (possibly gappy) subset of rows, the filtered variant must equal + // the full scan restricted to ranges whose line is in the subset. + let max_row = rope.max_point().row; + let mut row_ranges = Vec::new(); + let mut row = 0; + while row <= max_row { + let span = rng.random_range(0..=3); + if span > 0 { + let end = (row + span).min(max_row + 1); + row_ranges.push(row..end); + row = end; + } + row += 1; + } + + let expected = all_ranges + .iter() + .filter(|range| { + let row = rope.offset_to_point(range.start).row; + row_ranges.iter().any(|r| r.contains(&row)) + }) + .cloned() + .collect::>(); + assert_eq!( + trailing_whitespace_ranges(&rope, Some(&row_ranges)), + expected, + "subset mismatch for ranges {row_ranges:?} and lines:\n{lines:?}", + ); +} + +#[gpui::test] +async fn test_trailing_whitespace_in_ranges(cx: &mut gpui::TestAppContext) { + // line 0: "zero" (no trailing whitespace) + // line 1: "one " (2 trailing spaces) + // line 2: "two" (no trailing whitespace) + // line 3: "three " (3 trailing spaces) + // line 4: "four" (no trailing whitespace) + // line 5: "five " (4 trailing spaces) + let text = ["zero", "one ", "two", "three ", "four", "five "].join("\n"); + let buffer = cx.new(|cx| Buffer::local(text, cx)); + + // Only rows 1 and 5 are modified, so only those lines get cleaned; line 3 stays untouched. + let modified_rows = [1u32..2, 5..6]; + let diff = buffer + .update(cx, |buffer, cx| { + buffer.remove_trailing_whitespace(Some(&modified_rows), cx) + }) + .await; + buffer.update(cx, |buffer, cx| { + buffer.apply_diff(diff, cx); + assert_eq!( + buffer.text(), + ["zero", "one", "two", "three ", "four", "five"].join("\n") + ); + }); +} + +#[gpui::test] +async fn test_trailing_whitespace_empty_ranges(cx: &mut gpui::TestAppContext) { + let text = ["zero", "one ", "two "].join("\n"); + let buffer = cx.new(|cx| Buffer::local(text.clone(), cx)); + + let diff = buffer + .update(cx, |buffer, cx| { + buffer.remove_trailing_whitespace(Some(&[]), cx) + }) + .await; + buffer.update(cx, |buffer, cx| { + buffer.apply_diff(diff, cx); + assert_eq!(buffer.text(), text); + }); +} + +#[gpui::test] +async fn test_final_newline_modified_last_line(cx: &mut gpui::TestAppContext) { + // No final newline; the modified range (rows 0..3) includes the last line (row 2). + let text = "line0\nline1\nline2"; + let buffer = cx.new(|cx| Buffer::local(text, cx)); + + buffer.update(cx, |buffer, cx| { + let diff = buffer.ensure_final_newline(Some(&[0u32..3])); + buffer.apply_diff(diff, cx); + assert_eq!(buffer.text(), "line0\nline1\nline2\n"); + }); +} + +#[gpui::test] +async fn test_final_newline_unmodified_last_line(cx: &mut gpui::TestAppContext) { + // No final newline; the modified range (rows 0..2) excludes the last line (row 2), so nothing changes. + let text = "line0\nline1\nline2"; + let buffer = cx.new(|cx| Buffer::local(text, cx)); + + buffer.update(cx, |buffer, cx| { + let diff = buffer.ensure_final_newline(Some(&[0u32..2])); + buffer.apply_diff(diff, cx); + assert_eq!(buffer.text(), "line0\nline1\nline2"); + }); +} + +// An empty last line (file already ends with a newline) is left untouched, even with extra +// trailing blank lines. With `None` these would collapse; scoped to rows they must not, to +// avoid deleting unselected rows. +#[gpui::test] +async fn test_final_newline_does_not_collapse_trailing_blank_lines(cx: &mut gpui::TestAppContext) { + let text = "line0\nline1\n\n"; + let buffer = cx.new(|cx| Buffer::local(text, cx)); + + buffer.update(cx, |buffer, cx| { + let diff = buffer.ensure_final_newline(Some(&[0u32..4])); + buffer.apply_diff(diff, cx); + assert_eq!(buffer.text(), "line0\nline1\n\n"); + }); +} + +// When scoped to rows, only a newline is inserted; unlike the `None` (whole-buffer) case, it +// does not trim trailing whitespace on the last line. +#[gpui::test] +async fn test_final_newline_in_range_only_inserts(cx: &mut gpui::TestAppContext) { + let text = "line0\nline1 "; + let buffer = cx.new(|cx| Buffer::local(text, cx)); + + buffer.update(cx, |buffer, cx| { + let diff = buffer.ensure_final_newline(Some(&[0u32..2])); + buffer.apply_diff(diff, cx); + assert_eq!(buffer.text(), "line0\nline1 \n"); + }); +} + +#[gpui::test] +async fn test_final_newline_whole_buffer(cx: &mut gpui::TestAppContext) { + // (input, expected) pairs for the whole-buffer (`None`) case. + let cases = [ + // Content without a trailing newline gets exactly one appended. + ("line0\nline1", "line0\nline1\n"), + // A buffer already ending in a single newline is left untouched. + ("line0\nline1\n", "line0\nline1\n"), + // Trailing blank lines and whitespace at the end of the file collapse to one newline. + ("line0\nline1\n\n\n", "line0\nline1\n"), + ("line0\nline1 \n ", "line0\nline1\n"), + // An empty buffer stays empty. + ("", ""), + ]; + + for (input, expected) in cases { + let buffer = cx.new(|cx| Buffer::local(input, cx)); + buffer.update(cx, |buffer, cx| { + let diff = buffer.ensure_final_newline(None); + buffer.apply_diff(diff, cx); + assert_eq!(buffer.text(), expected, "wrong result for input {input:?}"); + }); + } +} + +#[gpui::test] +async fn test_trailing_whitespace_in_ranges_crlf(cx: &mut gpui::TestAppContext) { + let text = "zero\r\none \r\ntwo\r\nthree \r\nfour\r\nfive "; + let buffer = cx.new(|cx| { + let buffer = Buffer::local(text, cx); + assert_eq!(buffer.line_ending(), LineEnding::Windows); + buffer + }); + + let modified_rows = [1u32..2, 5..6]; + let diff = buffer + .update(cx, |buffer, cx| { + buffer.remove_trailing_whitespace(Some(&modified_rows), cx) + }) + .await; + buffer.update(cx, |buffer, cx| { + buffer.apply_diff(diff, cx); + assert_eq!(buffer.text(), "zero\none\ntwo\nthree \nfour\nfive"); + assert_eq!(buffer.line_ending(), LineEnding::Windows); + }); +} + +#[gpui::test] +async fn test_final_newline_in_range_crlf(cx: &mut gpui::TestAppContext) { + let text = "line0\r\nline1\r\nline2"; + let buffer = cx.new(|cx| { + let buffer = Buffer::local(text, cx); + assert_eq!(buffer.line_ending(), LineEnding::Windows); + buffer + }); + + buffer.update(cx, |buffer, cx| { + let diff = buffer.ensure_final_newline(Some(&[0u32..3])); + buffer.apply_diff(diff, cx); + assert_eq!(buffer.text(), "line0\nline1\nline2\n"); + assert_eq!(buffer.line_ending(), LineEnding::Windows); + }); +} + #[gpui::test] fn test_words_in_range(cx: &mut gpui::App) { init_settings(cx, |_| {}); diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index feb1936c47..e7ac11e773 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -1635,12 +1635,36 @@ impl LocalLspStore { .handle .read_with(cx, |buffer, _| buffer.max_point().row > 0); + // When formatting a selection, only the rows it spans may be touched. + let selection_row_ranges = buffer.ranges.as_ref().map(|ranges| { + buffer.handle.read_with(cx, |buffer, _cx| { + let snapshot = buffer.snapshot(); + ranges + .iter() + .map(|range| { + let start = range.start.to_point(&snapshot); + let end = range.end.to_point(&snapshot); + // A selection ending at column 0 of a row only includes that row's + // preceding line break, not its content, so it shouldn't be trimmed. + let end_row = if end.column == 0 && end.row > start.row { + end.row + } else { + end.row + 1 + }; + start.row..end_row + }) + .collect::>() + }) + }); + // handle whitespace formatting if settings.remove_trailing_whitespace_on_save { zlog::trace!(logger => "removing trailing whitespace"); let diff = buffer .handle - .read_with(cx, |buffer, cx| buffer.remove_trailing_whitespace(cx)) + .read_with(cx, |buffer, cx| { + buffer.remove_trailing_whitespace(selection_row_ranges.as_deref(), cx) + }) .await; extend_formatting_transaction(buffer, formatting_transaction_id, cx, |buffer, cx| { buffer.apply_diff(diff, cx); @@ -1649,8 +1673,11 @@ impl LocalLspStore { if settings.ensure_final_newline_on_save { zlog::trace!(logger => "ensuring final newline"); + let diff = buffer.handle.read_with(cx, |buffer, _cx| { + buffer.ensure_final_newline(selection_row_ranges.as_deref()) + }); extend_formatting_transaction(buffer, formatting_transaction_id, cx, |buffer, cx| { - buffer.ensure_final_newline(cx); + buffer.apply_diff(diff, cx); })?; } @@ -1925,6 +1952,7 @@ impl LocalLspStore { ) .await .context("Failed to format ranges via language server")? + .unwrap_or_default() } else { zlog::trace!(logger => "formatting full"); Self::format_via_lsp( @@ -2175,12 +2203,8 @@ impl LocalLspStore { formatting_transaction_id, cx, |buffer, cx| { - zlog::info!( - "Applying edits {edits:?}. Content: {:?}", - buffer.text() - ); + zlog::trace!("Applying {} edits", edits.len()); buffer.edit(edits, None, cx); - zlog::info!("Applied edits. New Content: {:?}", buffer.text()); }, )?; } @@ -2322,14 +2346,18 @@ impl LocalLspStore { language_server: &Arc, settings: &LanguageSettings, cx: &mut AsyncApp, - ) -> Result, Arc)>> { + ) -> Result, Arc)>>> { let capabilities = &language_server.capabilities(); let range_formatting_provider = capabilities.document_range_formatting_provider.as_ref(); - if range_formatting_provider == Some(&OneOf::Left(false)) { - anyhow::bail!( - "{} language server does not support range formatting", + if !matches!( + range_formatting_provider, + Some(OneOf::Left(true) | OneOf::Right(_)) + ) { + log::debug!( + "Skipping range formatting: language server {} does not support range formatting", language_server.name() ); + return Ok(None); } let uri = file_path_to_lsp_url(abs_path)?; @@ -2388,8 +2416,9 @@ impl LocalLspStore { ) })? .await + .map(Some) } else { - Ok(Vec::with_capacity(0)) + Ok(Some(Vec::with_capacity(0))) } }