Skip to content

Commit 4420c8c

Browse files
committed
Preserve trailing whitespace in files that already have it
When reading a file, detect whether it has trailing whitespace on any line. If it does, normalization only converts line endings to LF but leaves trailing whitespace intact. This prevents the agent from producing noisy whitespace-only diffs when editing files in repos that have trailing whitespace. Files without trailing whitespace continue to be fully normalized (line endings + trailing ws stripped), ensuring edits stay clean. - Add has_trailing_whitespace flag to FileFormat (serde-compatible) - Add detect_trailing_whitespace() and normalize_line_endings() - Thread preserve_trailing_ws through file_updater functions - Update Explorer, ACP explorer, and all call sites - Add tests for both modes
1 parent 3837af5 commit 4420c8c

7 files changed

Lines changed: 263 additions & 44 deletions

File tree

crates/code_assistant/src/acp/explorer.rs

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@ use std::sync::{Arc, OnceLock, RwLock};
77
use crate::config::{DefaultProjectManager, ProjectManager};
88
use crate::types::Project;
99
use command_executor::CommandExecutor;
10-
use fs_explorer::encoding::{apply_file_format, detect_line_ending, normalize_content};
10+
use fs_explorer::encoding::{
11+
apply_file_format, detect_line_ending, detect_trailing_whitespace, normalize_content,
12+
normalize_line_endings,
13+
};
1114
use fs_explorer::file_updater::{
1215
apply_matches, apply_replacements_normalized, extract_stable_ranges, find_replacement_matches,
1316
reconstruct_formatted_replacements,
@@ -165,15 +168,22 @@ impl AcpCodeExplorer {
165168
.await?;
166169

167170
let line_ending = detect_line_ending(&response.content);
171+
let has_trailing_whitespace = detect_trailing_whitespace(&response.content);
168172
self.store_format(
169173
&abs,
170174
FileFormat {
171175
encoding: FileEncoding::UTF8,
172176
line_ending,
177+
has_trailing_whitespace,
173178
},
174179
);
175180

176-
Ok((normalize_content(&response.content), abs))
181+
let normalized = if has_trailing_whitespace {
182+
normalize_line_endings(&response.content)
183+
} else {
184+
normalize_content(&response.content)
185+
};
186+
Ok((normalized, abs))
177187
}
178188

179189
async fn write_entire(&self, path: &Path, content: &str) -> Result<String> {
@@ -191,15 +201,23 @@ impl AcpCodeExplorer {
191201
abs.clone(),
192202
))
193203
.await?;
204+
194205
let line_ending = detect_line_ending(&response.content);
206+
let has_trailing_whitespace = detect_trailing_whitespace(&response.content);
195207
self.store_format(
196208
&abs,
197209
FileFormat {
198210
encoding: FileEncoding::UTF8,
199211
line_ending,
212+
has_trailing_whitespace,
200213
},
201214
);
202-
Ok(normalize_content(&response.content))
215+
let normalized = if has_trailing_whitespace {
216+
normalize_line_endings(&response.content)
217+
} else {
218+
normalize_content(&response.content)
219+
};
220+
Ok(normalized)
203221
}
204222
}
205223

@@ -285,7 +303,12 @@ impl CodeExplorer for AcpCodeExplorer {
285303
replacements: &[FileReplacement],
286304
) -> Result<String> {
287305
let (original_content, _) = self.read_entire(path).await?;
288-
let updated = apply_replacements_normalized(&original_content, replacements)?;
306+
let format = self.format_for_path(path);
307+
let updated = apply_replacements_normalized(
308+
&original_content,
309+
replacements,
310+
format.has_trailing_whitespace,
311+
)?;
289312
self.write_entire(path, &updated).await
290313
}
291314

@@ -297,13 +320,22 @@ impl CodeExplorer for AcpCodeExplorer {
297320
_command_executor: &dyn CommandExecutor,
298321
) -> Result<(String, Option<Vec<FileReplacement>>)> {
299322
let (original_content, _) = self.read_entire(path).await?;
300-
let (matches, has_conflicts) = find_replacement_matches(&original_content, replacements)?;
301-
let updated_content = apply_matches(&original_content, &matches, replacements)?;
323+
let format = self.format_for_path(path);
324+
let preserve_trailing_ws = format.has_trailing_whitespace;
325+
let (matches, has_conflicts) =
326+
find_replacement_matches(&original_content, replacements, preserve_trailing_ws)?;
327+
let updated_content = apply_matches(
328+
&original_content,
329+
&matches,
330+
replacements,
331+
preserve_trailing_ws,
332+
)?;
302333
let final_content = self.write_entire(path, &updated_content).await?;
303334
let updated_replacements = if has_conflicts {
304335
None
305336
} else {
306-
let stable_ranges = extract_stable_ranges(&original_content, &matches);
337+
let stable_ranges =
338+
extract_stable_ranges(&original_content, &matches, preserve_trailing_ws);
307339
reconstruct_formatted_replacements(
308340
&original_content,
309341
&final_content,

crates/code_assistant/src/tests/format_on_save_tests.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ fn test_stable_range_extraction_simple() -> Result<()> {
2525
end: 42, // End of "console.log('hello');"
2626
}];
2727

28-
let stable_ranges = extract_stable_ranges(content, &matches);
28+
let stable_ranges = extract_stable_ranges(content, &matches, false);
2929

3030
// Should have stable ranges before and after the match
3131
assert_eq!(stable_ranges.len(), 2);
@@ -63,7 +63,7 @@ fn test_stable_range_extraction_multiple_matches() -> Result<()> {
6363
},
6464
];
6565

66-
let stable_ranges = extract_stable_ranges(content, &matches);
66+
let stable_ranges = extract_stable_ranges(content, &matches, false);
6767

6868
// Should have one stable range between the two matches
6969
assert_eq!(stable_ranges.len(), 1);
@@ -91,7 +91,7 @@ fn test_parameter_reconstruction_simple() -> Result<()> {
9191
end: 21, // End of "const y=2;"
9292
}];
9393

94-
let stable_ranges = extract_stable_ranges(original_content, &matches);
94+
let stable_ranges = extract_stable_ranges(original_content, &matches, false);
9595

9696
let original_replacements = vec![FileReplacement {
9797
search: "const y=2;".to_string(),
@@ -128,7 +128,7 @@ fn test_parameter_reconstruction_failure() -> Result<()> {
128128
end: 32, // End of "// Comment"
129129
}];
130130

131-
let stable_ranges = extract_stable_ranges(original_content, &matches);
131+
let stable_ranges = extract_stable_ranges(original_content, &matches, false);
132132

133133
let original_replacements = vec![FileReplacement {
134134
search: "// Comment".to_string(),

crates/code_assistant/src/tests/mocks.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -550,7 +550,7 @@ impl CodeExplorer for MockExplorer {
550550
.ok_or_else(|| anyhow::anyhow!("File not found: {}", path.display()))?
551551
.clone();
552552

553-
let updated_content = apply_replacements_normalized(&content, replacements)?;
553+
let updated_content = apply_replacements_normalized(&content, replacements, false)?;
554554

555555
// Update the stored content
556556
files.insert(path.to_path_buf(), updated_content.clone());
@@ -569,7 +569,9 @@ impl CodeExplorer for MockExplorer {
569569
let original_content = self.read_file(path).await?;
570570

571571
// Find matches and detect adjacency/overlap
572-
let (matches, has_conflicts) = find_replacement_matches(&original_content, replacements)?;
572+
573+
let (matches, has_conflicts) =
574+
find_replacement_matches(&original_content, replacements, false)?;
573575

574576
// Apply replacements first
575577
let updated_content = self.apply_replacements(path, replacements).await?;
@@ -601,7 +603,7 @@ impl CodeExplorer for MockExplorer {
601603
let updated_replacements = if has_conflicts {
602604
None
603605
} else {
604-
let stable_ranges = extract_stable_ranges(&original_content, &matches);
606+
let stable_ranges = extract_stable_ranges(&original_content, &matches, false);
605607
reconstruct_formatted_replacements(
606608
&original_content,
607609
&final_content,

crates/fs_explorer/src/encoding.rs

Lines changed: 81 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -150,21 +150,49 @@ pub fn detect_line_ending(content: &str) -> LineEnding {
150150
}
151151
}
152152

153+
/// Detects whether the content has trailing whitespace on any line
154+
pub fn detect_trailing_whitespace(content: &str) -> bool {
155+
// Normalize line endings first so we split correctly
156+
let content = content.replace("\r\n", "\n").replace('\r', "\n");
157+
content.lines().any(|line| {
158+
let trimmed = line.trim_end();
159+
trimmed.len() < line.len()
160+
})
161+
}
162+
153163
/// Normalizes text content by:
154164
/// 1. Converting all line endings to LF (\n)
155165
/// 2. Removing trailing whitespace from each line
156166
/// 3. Preserving empty lines
157167
/// 4. NOT adding a trailing newline (this happens at file write time)
158168
pub fn normalize_content(content: &str) -> String {
169+
normalize_content_impl(content, true)
170+
}
171+
172+
/// Normalizes line endings to LF but preserves trailing whitespace on each line.
173+
/// Used for files that originally had trailing whitespace to avoid noisy diffs.
174+
pub fn normalize_line_endings(content: &str) -> String {
175+
normalize_content_impl(content, false)
176+
}
177+
178+
/// Normalizes content: always converts line endings to LF,
179+
/// optionally strips trailing whitespace from each line.
180+
fn normalize_content_impl(content: &str, strip_trailing_ws: bool) -> String {
159181
// First normalize all line endings to LF
160182
let content = content.replace("\r\n", "\n").replace('\r', "\n");
161183

162-
// Process each line - preserve empty lines but trim trailing whitespace
163-
content
164-
.lines()
165-
.map(|line| line.trim_end())
166-
.collect::<Vec<&str>>()
167-
.join("\n")
184+
if strip_trailing_ws {
185+
// Process each line - preserve empty lines but trim trailing whitespace
186+
content
187+
.lines()
188+
.map(|line| line.trim_end())
189+
.collect::<Vec<&str>>()
190+
.join("\n")
191+
} else {
192+
// Only normalize line endings, preserve trailing whitespace
193+
// .lines() strips the trailing newline, which is what we want
194+
content.lines().collect::<Vec<&str>>().join("\n")
195+
}
168196
}
169197

170198
/// Restores the original line endings of content
@@ -264,11 +292,13 @@ mod tests {
264292
let crlf_format = FileFormat {
265293
encoding: FileEncoding::UTF8,
266294
line_ending: LineEnding::Crlf,
295+
has_trailing_whitespace: false,
267296
};
268297

269298
let cr_format = FileFormat {
270299
encoding: FileEncoding::UTF8,
271300
line_ending: LineEnding::CR,
301+
has_trailing_whitespace: false,
272302
};
273303

274304
assert_eq!(
@@ -296,6 +326,7 @@ mod tests {
296326
let format = FileFormat {
297327
encoding: FileEncoding::UTF8,
298328
line_ending: LineEnding::LF,
329+
has_trailing_whitespace: false,
299330
};
300331

301332
// All cases should result in content with exactly one trailing newline
@@ -315,4 +346,48 @@ mod tests {
315346
assert_eq!(content, expected);
316347
}
317348
}
349+
350+
#[test]
351+
fn test_detect_trailing_whitespace() {
352+
// No trailing whitespace
353+
assert!(!detect_trailing_whitespace("Line1\nLine2\nLine3"));
354+
// Trailing spaces on a line
355+
assert!(detect_trailing_whitespace("Line1 \nLine2\nLine3"));
356+
// Trailing tab
357+
assert!(detect_trailing_whitespace("Line1\t\nLine2\nLine3"));
358+
// Only on last line
359+
assert!(detect_trailing_whitespace("Line1\nLine2\nLine3 "));
360+
// With CRLF
361+
assert!(detect_trailing_whitespace("Line1 \r\nLine2\r\nLine3"));
362+
// No trailing whitespace with CRLF
363+
assert!(!detect_trailing_whitespace("Line1\r\nLine2\r\nLine3"));
364+
}
365+
366+
#[test]
367+
fn test_normalize_line_endings() {
368+
// Should preserve trailing whitespace but normalize line endings
369+
assert_eq!(
370+
normalize_line_endings("Line1 \r\nLine2 \r\nLine3"),
371+
"Line1 \nLine2 \nLine3"
372+
);
373+
// CR line endings
374+
assert_eq!(
375+
normalize_line_endings("Line1 \rLine2 \rLine3"),
376+
"Line1 \nLine2 \nLine3"
377+
);
378+
// LF already - no change
379+
assert_eq!(
380+
normalize_line_endings("Line1 \nLine2 \nLine3"),
381+
"Line1 \nLine2 \nLine3"
382+
);
383+
}
384+
385+
#[test]
386+
fn test_normalize_content_strips_trailing_ws() {
387+
// normalize_content should strip trailing whitespace
388+
assert_eq!(
389+
normalize_content("Line1 \nLine2 \nLine3"),
390+
"Line1\nLine2\nLine3"
391+
);
392+
}
318393
}

0 commit comments

Comments
 (0)