Add range-based whitespace and newline removal to buffer formatting (#53942)

## Summary

When formatting specific ranges (e.g. via Format Selections), trailing
whitespace removal and final newline enforcement currently operate on
the entire file. This PR makes these operations range-aware, so they
only affect lines within the target ranges.

As noted in #16509, users want `remove_trailing_whitespace_on_save` and
`ensure_final_newline_on_save` to suppress jitter by only touching
changed lines. This PR provides the foundational infrastructure for that
— range-based whitespace and newline operations — which a follow-up PR
will wire into the `format_on_save` modifications mode.

## Changes

- Added `remove_trailing_whitespace_in_ranges` and
`ensure_final_newline_in_range` to `Buffer`
- When a `FormattableBuffer` has ranges set, whitespace and newline
operations now use the range-based variants
- `format_ranges_via_lsp` returns `Ok(None)` instead of an error when
the LSP doesn't support range formatting
- Extracted `anchor_ranges_to_row_ranges` helper to deduplicate
conversion logic

<details><summary>Tests (4)</summary>

- `test_trailing_whitespace_in_ranges` — only modified lines have
whitespace removed
- `test_trailing_whitespace_empty_ranges` — empty ranges → no changes
- `test_final_newline_modified_last_line` — newline added when last line
is in range
- `test_final_newline_unmodified_last_line` — newline not added when
last line is outside range

</details>

---

Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Release Notes:

- Improved Format Selection to scope trailing whitespace removal and
final newline enforcement to the selected range instead of the entire
file
- Improved handling of LSP servers that do not support range formatting
by gracefully skipping instead of producing an error

---------

Co-authored-by: Kirill Bulatov <kirill@zed.dev>
This commit is contained in:
G36maidandKirill Bulatov authored and GitHub committed 2026-07-01 17:36:36 +00:00
1 parent 21b0a6821e
commit 2a3f40aba8
3 files changed
+350 -41

No files matched your search

+85 -27
View File
@@ -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<Diff> {
/// 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<u32>]>,
cx: &App,
) -> Task<Diff> {
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::<str>::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<Self>) {
/// 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<u32>]>) -> 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::<str>::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<Range<usize>> {
/// 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<u32>]>,
) -> Vec<Range<usize>> {
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<u32> = 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<Range<usize>> {
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);
}
+224 -2
View File
@@ -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::<Vec<_>>();
// 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::<Vec<_>>();
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, |_| {});
+41 -12
View File
@@ -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::<Vec<_>>()
})
});
// 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<LanguageServer>,
settings: &LanguageSettings,
cx: &mut AsyncApp,
) -> Result<Vec<(Range<Anchor>, Arc<str>)>> {
) -> Result<Option<Vec<(Range<Anchor>, Arc<str>)>>> {
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)))
}
}