agent: Fix edit_file failing to match exact mid-line old_text fragments (#61489)
Fixes the `edit_file` tool failing to resolve `old_text` fragments that only cover part of a longer line. The streaming matcher compared the query against whole trimmed buffer lines, so an exact substring like `keyboard WASD, voxel-based` inside a much longer line failed the fuzzy similarity threshold and the edit was rejected with "did not match any content". Supersedes #60615 — thanks to @ziimakc for the report, reproduction, and initial investigation there. This PR takes a different approach based on review feedback on that PR. ## Approach - `StreamingFuzzyMatcher::push()` is unchanged: it still fuzzy-matches completed lines incrementally so the UI can reveal the likely edit location while `old_text` streams in. - `finish()` now first searches the buffer for the complete raw query as an exact byte substring (rope-native, overlap-aware KMP; no copy of the buffer). Exact matches take precedence; fuzzy matching remains the fallback for indentation drift, typos, and inserted/omitted lines. - Exact matches are capped at two occurrences — enough to prove ambiguity — and ambiguous or overlapping matches are rejected with the existing "matched multiple locations" error instead of silently editing one. - The matcher now returns match candidates with their line alignment and exact/fuzzy provenance in one result, and the edit pipeline uses that to reindent correctly: for an exact mid-line match the first replacement line keeps the retained prefix (zero delta) while subsequent lines get the contextual indentation. - The streaming parser now preserves the trailing newline of finalized `old_text` (it still strips it from `new_text` to avoid duplicate line endings, per #52661), so `"foo\n"` can disambiguate a standalone `foo` line from `foo suffix`. ## Notes - Exact-search cost is O(buffer + query) at finalization only; the buffer is never copied into a `String` (the main concern with the original PR). - New regression tests cover mid-line fragments, exact-over-fuzzy precedence, overlapping-match ambiguity, multiline replacement indentation (spaces and tabs), trailing-newline disambiguation, and newline-only edits. Closes https://github.com/zed-industries/zed/issues/59369 Release Notes: - Fixed agent edits failing when the text to replace is only part of a line
This commit is contained in:
1 parent
737f3384f0
commit
d5f26118c5
4 files changed
+411
-86
No files matched your search
@@ -324,6 +324,79 @@ mod tests {
|
||||
assert_eq!(new_text, "line 1\nmodified line 2\nline 3\n");
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_streaming_edit_exact_fragments(cx: &mut TestAppContext) {
|
||||
let content = concat!(
|
||||
"fn spaces() {\n",
|
||||
" spaces_old();\n",
|
||||
"}\n",
|
||||
"fn tabs() {\n",
|
||||
"\ttabs_old();\n",
|
||||
"}\n",
|
||||
"controls: keyboard WASD, voxel-based\n",
|
||||
"prefix OLD suffix\n",
|
||||
"foo suffix\n",
|
||||
"foo\n",
|
||||
);
|
||||
let (edit_tool, _project, _action_log, _fs, _thread) =
|
||||
setup_test(cx, json!({"file.rs": content})).await;
|
||||
let result = cx
|
||||
.update(|cx| {
|
||||
edit_tool.clone().run(
|
||||
ToolInput::resolved(EditFileToolInput {
|
||||
path: "root/file.rs".into(),
|
||||
edits: vec![
|
||||
Edit {
|
||||
old_text: "keyboard WASD, voxel-based".into(),
|
||||
new_text: "arrow keys".into(),
|
||||
},
|
||||
Edit {
|
||||
old_text: "spaces_old();".into(),
|
||||
new_text: "spaces_new();\nspaces_more();".into(),
|
||||
},
|
||||
Edit {
|
||||
old_text: "tabs_old();".into(),
|
||||
new_text: "tabs_new();\ntabs_more();".into(),
|
||||
},
|
||||
Edit {
|
||||
old_text: "OLD".into(),
|
||||
new_text: "NEW\n".into(),
|
||||
},
|
||||
Edit {
|
||||
old_text: "foo\n".into(),
|
||||
new_text: "bar\n".into(),
|
||||
},
|
||||
],
|
||||
}),
|
||||
ToolCallEventStream::test().0,
|
||||
cx,
|
||||
)
|
||||
})
|
||||
.await;
|
||||
|
||||
let EditFileToolOutput::Success { new_text, .. } = result.unwrap() else {
|
||||
panic!("expected success");
|
||||
};
|
||||
assert_eq!(
|
||||
new_text,
|
||||
concat!(
|
||||
"fn spaces() {\n",
|
||||
" spaces_new();\n",
|
||||
" spaces_more();\n",
|
||||
"}\n",
|
||||
"fn tabs() {\n",
|
||||
"\ttabs_new();\n",
|
||||
"\ttabs_more();\n",
|
||||
"}\n",
|
||||
"controls: arrow keys\n",
|
||||
"prefix NEW\n",
|
||||
" suffix\n",
|
||||
"foo suffix\n",
|
||||
"bar\n",
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_streaming_edit_first_line_missing_indent(cx: &mut TestAppContext) {
|
||||
// Reproduces https://github.com/zed-industries/zed/issues/60302: the
|
||||
@@ -627,6 +700,33 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_streaming_edit_rejects_overlapping_matches(cx: &mut TestAppContext) {
|
||||
let (edit_tool, _project, _action_log, _fs, _thread) =
|
||||
setup_test(cx, json!({"file.txt": "aaaaa"})).await;
|
||||
let result = cx
|
||||
.update(|cx| {
|
||||
edit_tool.clone().run(
|
||||
ToolInput::resolved(EditFileToolInput {
|
||||
path: "root/file.txt".into(),
|
||||
edits: vec![Edit {
|
||||
old_text: "aaaa".into(),
|
||||
new_text: "replacement".into(),
|
||||
}],
|
||||
}),
|
||||
ToolCallEventStream::test().0,
|
||||
cx,
|
||||
)
|
||||
})
|
||||
.await;
|
||||
|
||||
let EditFileToolOutput::Error { error, diff, .. } = result.unwrap_err() else {
|
||||
panic!("expected error");
|
||||
};
|
||||
assert!(error.contains("matched multiple locations"));
|
||||
assert!(diff.is_empty());
|
||||
}
|
||||
|
||||
/// When the edit fails after a session is created but before any edits are
|
||||
/// actually applied (e.g., the first `old_text` doesn't match), the empty
|
||||
/// diff placeholder in the UI should be replaced with the error message.
|
||||
|
||||
@@ -16,14 +16,14 @@ use language::{Buffer, BufferEditSource, BufferEvent, LanguageRegistry};
|
||||
use language_model::LanguageModelToolResultContent;
|
||||
use project::lsp_store::{FormatTrigger, LspFormatTarget};
|
||||
use project::{AgentLocation, Project, ProjectPath};
|
||||
use reindent::{Reindenter, compute_indent_delta, compute_rest_indent_delta};
|
||||
use reindent::{IndentDelta, Reindenter, compute_indent_delta, compute_rest_indent_delta};
|
||||
use schemars::JsonSchema;
|
||||
use serde::{Deserialize, Serialize, de::DeserializeOwned};
|
||||
use std::ops::Range;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
use streaming_diff::{CharOperation, StreamingDiff};
|
||||
use streaming_fuzzy_matcher::StreamingFuzzyMatcher;
|
||||
use streaming_fuzzy_matcher::{SearchMatch, SearchMatches, StreamingFuzzyMatcher};
|
||||
use streaming_parser::{EditEvent, StreamingParser, WriteEvent};
|
||||
use text::ToOffset;
|
||||
use ui::SharedString;
|
||||
@@ -393,6 +393,7 @@ enum EditPipelineEntry {
|
||||
edit_cursor: usize,
|
||||
reindenter: Reindenter,
|
||||
original_snapshot: text::BufferSnapshot,
|
||||
strip_trailing_newline: bool,
|
||||
},
|
||||
}
|
||||
|
||||
@@ -506,7 +507,10 @@ impl EditPipeline {
|
||||
if !chunk.is_empty() {
|
||||
matcher.push(chunk, None);
|
||||
}
|
||||
let range = extract_match(
|
||||
let ResolvedMatch {
|
||||
search_match: SearchMatch { range, line_pairs },
|
||||
is_exact,
|
||||
} = extract_match(
|
||||
matcher.finish(),
|
||||
buffer,
|
||||
edit_index,
|
||||
@@ -535,16 +539,20 @@ impl EditPipeline {
|
||||
.unwrap_or("")
|
||||
.chars(),
|
||||
);
|
||||
let first_line_delta = compute_indent_delta(buffer_indent, query_indent);
|
||||
|
||||
let contextual_delta = compute_indent_delta(buffer_indent, query_indent);
|
||||
let first_line_delta = if is_exact {
|
||||
// An exact range may start mid-line, where the retained prefix already
|
||||
// provides the first replacement line's indentation.
|
||||
IndentDelta::Spaces(0)
|
||||
} else {
|
||||
contextual_delta
|
||||
};
|
||||
// Query row 0 is excluded: its delta is `first_line_delta`,
|
||||
// which intentionally differs when the model stripped the
|
||||
// first line's indentation.
|
||||
let rest_delta = compute_rest_indent_delta(
|
||||
first_line_delta,
|
||||
matcher
|
||||
.line_pairs(&range)
|
||||
.unwrap_or(&[])
|
||||
contextual_delta,
|
||||
line_pairs
|
||||
.iter()
|
||||
.filter(|(query_row, _)| *query_row != 0)
|
||||
.filter_map(|(query_row, buffer_row)| {
|
||||
@@ -556,6 +564,7 @@ impl EditPipeline {
|
||||
}),
|
||||
);
|
||||
|
||||
let strip_trailing_newline = snapshot.contains_str_at(range.end, "\n");
|
||||
let old_text_in_buffer = snapshot.text_for_range(range.clone()).collect::<String>();
|
||||
|
||||
log::debug!(
|
||||
@@ -572,6 +581,7 @@ impl EditPipeline {
|
||||
edit_cursor: range.start,
|
||||
reindenter: Reindenter::with_deltas(first_line_delta, rest_delta),
|
||||
original_snapshot: text_snapshot,
|
||||
strip_trailing_newline,
|
||||
});
|
||||
|
||||
cx.update(|cx| {
|
||||
@@ -625,11 +635,17 @@ impl EditPipeline {
|
||||
mut edit_cursor,
|
||||
mut reindenter,
|
||||
original_snapshot,
|
||||
strip_trailing_newline,
|
||||
}) = self.current_edit.take()
|
||||
else {
|
||||
return Ok(());
|
||||
};
|
||||
|
||||
let chunk = if strip_trailing_newline {
|
||||
chunk.strip_suffix('\n').unwrap_or(chunk)
|
||||
} else {
|
||||
chunk
|
||||
};
|
||||
let mut final_text = reindenter.push(chunk);
|
||||
final_text.push_str(&reindenter.finish());
|
||||
|
||||
@@ -933,32 +949,46 @@ fn apply_char_operations(
|
||||
}
|
||||
}
|
||||
|
||||
struct ResolvedMatch {
|
||||
search_match: SearchMatch,
|
||||
is_exact: bool,
|
||||
}
|
||||
|
||||
fn extract_match(
|
||||
matches: Vec<Range<usize>>,
|
||||
matches: SearchMatches,
|
||||
buffer: &Entity<Buffer>,
|
||||
edit_index: &usize,
|
||||
file_changed_since_last_read: bool,
|
||||
cx: &mut AsyncApp,
|
||||
) -> Result<Range<usize>, String> {
|
||||
) -> Result<ResolvedMatch, String> {
|
||||
let (matches, is_exact) = match matches {
|
||||
SearchMatches::Exact(matches) => (matches, true),
|
||||
SearchMatches::Fuzzy(matches) => (matches, false),
|
||||
};
|
||||
let file_changed_since_last_read_message = if file_changed_since_last_read {
|
||||
" The file has changed on disk since you last read it."
|
||||
} else {
|
||||
""
|
||||
};
|
||||
|
||||
match matches.len() {
|
||||
0 => Err(format!(
|
||||
match matches.as_slice() {
|
||||
[] => Err(format!(
|
||||
"Could not find matching text for edit at index {}. \
|
||||
The old_text did not match any content in the file.{} \
|
||||
Please read the file again to get the current content.",
|
||||
edit_index, file_changed_since_last_read_message,
|
||||
)),
|
||||
1 => Ok(matches.into_iter().next().unwrap()),
|
||||
[search_match] => Ok(ResolvedMatch {
|
||||
search_match: search_match.clone(),
|
||||
is_exact,
|
||||
}),
|
||||
_ => {
|
||||
let snapshot = buffer.read_with(cx, |buffer, _cx| buffer.snapshot());
|
||||
let lines = matches
|
||||
.iter()
|
||||
.map(|range| (snapshot.offset_to_point(range.start).row + 1).to_string())
|
||||
.map(|search_match| {
|
||||
(snapshot.offset_to_point(search_match.range.start).row + 1).to_string()
|
||||
})
|
||||
.collect::<Vec<_>>()
|
||||
.join(", ");
|
||||
Err(format!(
|
||||
|
||||
@@ -4,6 +4,9 @@ use std::{cmp, ops::Range};
|
||||
const REPLACEMENT_COST: u32 = 1;
|
||||
const INSERTION_COST: u32 = 3;
|
||||
const DELETION_COST: u32 = 10;
|
||||
// Two matches are enough to prove ambiguity, so stop scanning early; exact
|
||||
// matches are byte-identical and can't be ranked.
|
||||
const MAX_EXACT_MATCHES: usize = 2;
|
||||
|
||||
/// A streaming fuzzy matcher that can process text chunks incrementally
|
||||
/// and return the best match found so far at each step.
|
||||
@@ -12,6 +15,7 @@ pub struct StreamingFuzzyMatcher {
|
||||
query_lines: Vec<String>,
|
||||
line_hint: Option<u32>,
|
||||
incomplete_line: String,
|
||||
raw_query: String,
|
||||
matches: Vec<SearchMatch>,
|
||||
matrix: SearchMatrix,
|
||||
}
|
||||
@@ -19,9 +23,15 @@ pub struct StreamingFuzzyMatcher {
|
||||
/// A match candidate: the matched byte range plus the 0-based
|
||||
/// `(query_row, buffer_row)` line pairs the search aligned to produce it.
|
||||
#[derive(Clone, Debug)]
|
||||
struct SearchMatch {
|
||||
range: Range<usize>,
|
||||
line_pairs: Vec<(u32, u32)>,
|
||||
pub(super) struct SearchMatch {
|
||||
pub(super) range: Range<usize>,
|
||||
pub(super) line_pairs: Vec<(u32, u32)>,
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
pub(super) enum SearchMatches {
|
||||
Exact(Vec<SearchMatch>),
|
||||
Fuzzy(Vec<SearchMatch>),
|
||||
}
|
||||
|
||||
impl StreamingFuzzyMatcher {
|
||||
@@ -32,6 +42,7 @@ impl StreamingFuzzyMatcher {
|
||||
query_lines: Vec::new(),
|
||||
line_hint: None,
|
||||
incomplete_line: String::new(),
|
||||
raw_query: String::new(),
|
||||
matches: Vec::new(),
|
||||
matrix: SearchMatrix::new(buffer_line_count + 1),
|
||||
}
|
||||
@@ -42,23 +53,6 @@ impl StreamingFuzzyMatcher {
|
||||
&self.query_lines
|
||||
}
|
||||
|
||||
/// Returns the 0-based `(query_row, buffer_row)` line pairs that the
|
||||
/// search aligned for the match with the given range. Lines that were
|
||||
/// skipped on either side of the alignment are absent.
|
||||
pub fn line_pairs(&self, range: &Range<usize>) -> Option<&[(u32, u32)]> {
|
||||
self.matches
|
||||
.iter()
|
||||
.find(|search_match| search_match.range == *range)
|
||||
.map(|search_match| search_match.line_pairs.as_slice())
|
||||
}
|
||||
|
||||
fn match_ranges(&self) -> Vec<Range<usize>> {
|
||||
self.matches
|
||||
.iter()
|
||||
.map(|search_match| search_match.range.clone())
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Push a new chunk of text and get the best match found so far.
|
||||
///
|
||||
/// This method accumulates text chunks and processes complete lines.
|
||||
@@ -70,6 +64,7 @@ impl StreamingFuzzyMatcher {
|
||||
/// query so far, or `None` if no suitable match exists yet.
|
||||
pub fn push(&mut self, chunk: &str, line_hint: Option<u32>) -> Option<Range<usize>> {
|
||||
// Add the chunk to our incomplete line buffer
|
||||
self.raw_query.push_str(chunk);
|
||||
self.incomplete_line.push_str(chunk);
|
||||
self.line_hint = line_hint;
|
||||
|
||||
@@ -98,7 +93,26 @@ impl StreamingFuzzyMatcher {
|
||||
///
|
||||
/// This processes any remaining incomplete line before returning the final
|
||||
/// match result.
|
||||
pub fn finish(&mut self) -> Vec<Range<usize>> {
|
||||
pub fn finish(&mut self) -> SearchMatches {
|
||||
let exact_ranges = find_exact_matches(&self.snapshot, &self.raw_query);
|
||||
if !exact_ranges.is_empty() {
|
||||
if !self.incomplete_line.is_empty() {
|
||||
self.query_lines
|
||||
.push(std::mem::take(&mut self.incomplete_line));
|
||||
}
|
||||
let matches = exact_ranges
|
||||
.into_iter()
|
||||
.map(|range| {
|
||||
let start_row = self.snapshot.offset_to_point(range.start).row;
|
||||
let line_pairs = (0..self.query_lines.len())
|
||||
.map(|query_row| (query_row as u32, start_row + query_row as u32))
|
||||
.collect();
|
||||
SearchMatch { range, line_pairs }
|
||||
})
|
||||
.collect();
|
||||
return SearchMatches::Exact(matches);
|
||||
}
|
||||
|
||||
// Process any remaining incomplete line
|
||||
if !self.incomplete_line.is_empty() {
|
||||
if let [only_match] = self.matches.as_mut_slice() {
|
||||
@@ -118,7 +132,7 @@ impl StreamingFuzzyMatcher {
|
||||
only_match
|
||||
.line_pairs
|
||||
.push(((self.query_lines.len() - 1) as u32, extended_row));
|
||||
return self.match_ranges();
|
||||
return SearchMatches::Fuzzy(self.matches.clone());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -126,7 +140,7 @@ impl StreamingFuzzyMatcher {
|
||||
.push(std::mem::take(&mut self.incomplete_line));
|
||||
self.matches = self.resolve_location_fuzzy();
|
||||
}
|
||||
self.match_ranges()
|
||||
SearchMatches::Fuzzy(self.matches.clone())
|
||||
}
|
||||
|
||||
fn resolve_location_fuzzy(&mut self) -> Vec<SearchMatch> {
|
||||
@@ -285,6 +299,62 @@ impl StreamingFuzzyMatcher {
|
||||
}
|
||||
}
|
||||
|
||||
// Aho-Corasick's overlapping search requires a contiguous haystack, while its
|
||||
// streaming search skips overlapping matches. KMP detects overlapping ambiguity
|
||||
// while scanning rope chunks without copying the buffer.
|
||||
fn find_exact_matches(snapshot: &TextBufferSnapshot, query: &str) -> Vec<Range<usize>> {
|
||||
if query.is_empty() {
|
||||
return Vec::new();
|
||||
}
|
||||
|
||||
let query = query.as_bytes();
|
||||
let trailing_line_ending_len = if query.ends_with(b"\r\n") {
|
||||
2
|
||||
} else if query.ends_with(b"\n") {
|
||||
1
|
||||
} else {
|
||||
0
|
||||
};
|
||||
let mut prefix_lengths = vec![0; query.len()];
|
||||
let mut prefix_length = 0;
|
||||
for query_index in 1..query.len() {
|
||||
while prefix_length > 0 && query[query_index] != query[prefix_length] {
|
||||
prefix_length = prefix_lengths[prefix_length - 1];
|
||||
}
|
||||
if query[query_index] == query[prefix_length] {
|
||||
prefix_length += 1;
|
||||
prefix_lengths[query_index] = prefix_length;
|
||||
}
|
||||
}
|
||||
|
||||
let mut matches = Vec::new();
|
||||
let mut matched_length = 0;
|
||||
for (offset, byte) in snapshot
|
||||
.bytes_in_range(0..snapshot.len())
|
||||
.flatten()
|
||||
.copied()
|
||||
.enumerate()
|
||||
{
|
||||
while matched_length > 0 && byte != query[matched_length] {
|
||||
matched_length = prefix_lengths[matched_length - 1];
|
||||
}
|
||||
if byte == query[matched_length] {
|
||||
matched_length += 1;
|
||||
}
|
||||
if matched_length == query.len() {
|
||||
let raw_end = offset + 1;
|
||||
let start = raw_end - query.len();
|
||||
let end = raw_end - trailing_line_ending_len;
|
||||
matches.push(start..end);
|
||||
if matches.len() == MAX_EXACT_MATCHES {
|
||||
break;
|
||||
}
|
||||
matched_length = prefix_lengths[matched_length - 1];
|
||||
}
|
||||
}
|
||||
matches
|
||||
}
|
||||
|
||||
fn fuzzy_eq(left: &str, right: &str) -> bool {
|
||||
const THRESHOLD: f64 = 0.8;
|
||||
|
||||
@@ -563,7 +633,7 @@ mod tests {
|
||||
assert_location_resolution(
|
||||
concat!(
|
||||
" Lorem\n",
|
||||
"« ipsum»\n",
|
||||
" «ipsum»\n",
|
||||
" dolor sit amet\n",
|
||||
" consecteur",
|
||||
),
|
||||
@@ -776,6 +846,111 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
fn test_exact_match_takes_precedence_over_fuzzy_match() {
|
||||
let buffer = TextBuffer::new(
|
||||
ReplicaId::LOCAL,
|
||||
BufferId::new(1).unwrap(),
|
||||
concat!(
|
||||
"prefix keyboard WASD, voxel-based suffix\n",
|
||||
"keyboard WASD, voxel-baseX\n",
|
||||
),
|
||||
);
|
||||
let snapshot = buffer.snapshot();
|
||||
let mut matcher = StreamingFuzzyMatcher::new(snapshot.clone());
|
||||
|
||||
assert_eq!(matcher.push("keyboard WASD, voxel-based", None), None);
|
||||
let SearchMatches::Exact(matches) = matcher.finish() else {
|
||||
panic!("expected an exact match");
|
||||
};
|
||||
let [search_match] = matches.as_slice() else {
|
||||
panic!("expected one match, got {}", matches.len());
|
||||
};
|
||||
assert_eq!(
|
||||
snapshot
|
||||
.text_for_range(search_match.range.clone())
|
||||
.collect::<String>(),
|
||||
"keyboard WASD, voxel-based"
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
fn test_exact_match_uses_trailing_newline_to_disambiguate() {
|
||||
let buffer = TextBuffer::new(
|
||||
ReplicaId::LOCAL,
|
||||
BufferId::new(1).unwrap(),
|
||||
"foo suffix\nfoo\n",
|
||||
);
|
||||
let snapshot = buffer.snapshot();
|
||||
let mut matcher = StreamingFuzzyMatcher::new(snapshot.clone());
|
||||
|
||||
matcher.push("foo\n", None);
|
||||
let SearchMatches::Exact(matches) = matcher.finish() else {
|
||||
panic!("expected an exact match");
|
||||
};
|
||||
let [search_match] = matches.as_slice() else {
|
||||
panic!("expected one match, got {}", matches.len());
|
||||
};
|
||||
assert_eq!(
|
||||
snapshot
|
||||
.text_for_range(search_match.range.clone())
|
||||
.collect::<String>(),
|
||||
"foo"
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
fn test_exact_newline_only_match_excludes_line_ending() {
|
||||
let buffer = TextBuffer::new(ReplicaId::LOCAL, BufferId::new(1).unwrap(), "a\nb");
|
||||
let mut matcher = StreamingFuzzyMatcher::new(buffer.snapshot().clone());
|
||||
|
||||
matcher.push("\n", None);
|
||||
let SearchMatches::Exact(matches) = matcher.finish() else {
|
||||
panic!("expected an exact match");
|
||||
};
|
||||
let [search_match] = matches.as_slice() else {
|
||||
panic!("expected one match, got {}", matches.len());
|
||||
};
|
||||
assert_eq!(search_match.range, 1..1);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
fn test_exact_overlapping_matches_are_ambiguous() {
|
||||
let buffer = TextBuffer::new(ReplicaId::LOCAL, BufferId::new(1).unwrap(), "aaaaa");
|
||||
let mut matcher = StreamingFuzzyMatcher::new(buffer.snapshot().clone());
|
||||
|
||||
matcher.push("aaaa", None);
|
||||
let matches = matcher.finish();
|
||||
|
||||
assert!(matches!(matches, SearchMatches::Exact(_)));
|
||||
assert_eq!(match_ranges(&matches), vec![0..4, 1..5]);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
fn test_exact_multiline_match_does_not_extend_incomplete_line() {
|
||||
let buffer = TextBuffer::new(
|
||||
ReplicaId::LOCAL,
|
||||
BufferId::new(1).unwrap(),
|
||||
"prefix fragment\nnext\nnext\n",
|
||||
);
|
||||
let snapshot = buffer.snapshot();
|
||||
let mut matcher = StreamingFuzzyMatcher::new(snapshot.clone());
|
||||
|
||||
assert_eq!(matcher.push("fragment\nnext", None), None);
|
||||
let SearchMatches::Exact(matches) = matcher.finish() else {
|
||||
panic!("expected an exact match");
|
||||
};
|
||||
let [search_match] = matches.as_slice() else {
|
||||
panic!("expected one match, got {}", matches.len());
|
||||
};
|
||||
assert_eq!(
|
||||
snapshot
|
||||
.text_for_range(search_match.range.clone())
|
||||
.collect::<String>(),
|
||||
"fragment\nnext"
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
fn test_prefix_of_last_line_resolves_to_correct_range() {
|
||||
let text = indoc! {r#"
|
||||
@@ -797,26 +972,32 @@ mod tests {
|
||||
);
|
||||
let snapshot = buffer.snapshot();
|
||||
|
||||
// Query with a partial last line.
|
||||
// Query with a partial last line. This is a verbatim substring of the
|
||||
// buffer, so it resolves through the exact-match path.
|
||||
let query = "}\n\n\n\nfn render_search";
|
||||
|
||||
let mut matcher = StreamingFuzzyMatcher::new(snapshot.clone());
|
||||
matcher.push(query, None);
|
||||
let matches = matcher.finish();
|
||||
let matched = search_matches(&matches);
|
||||
|
||||
// The match should include the line containing "fn render_search".
|
||||
let matched_text = matches
|
||||
.first()
|
||||
.map(|range| snapshot.text_for_range(range.clone()).collect::<String>());
|
||||
let matched_text = matched.first().map(|search_match| {
|
||||
snapshot
|
||||
.text_for_range(search_match.range.clone())
|
||||
.collect::<String>()
|
||||
});
|
||||
|
||||
assert!(
|
||||
matches.len() == 1,
|
||||
matched.len() == 1,
|
||||
"Expected exactly one match, got {}: {:?}",
|
||||
matches.len(),
|
||||
matched.len(),
|
||||
matched_text,
|
||||
);
|
||||
|
||||
let matched_text = matched_text.unwrap();
|
||||
let Some(matched_text) = matched_text else {
|
||||
panic!("expected a match");
|
||||
};
|
||||
pretty_assertions::assert_eq!(
|
||||
matched_text,
|
||||
"}\n\n\n\nfn render_search",
|
||||
@@ -840,7 +1021,8 @@ mod tests {
|
||||
matcher.push(chunk, None);
|
||||
}
|
||||
|
||||
let actual_ranges = matcher.finish();
|
||||
let actual_matches = matcher.finish();
|
||||
let actual_ranges = match_ranges(&actual_matches);
|
||||
|
||||
// If no expected ranges, we expect no match
|
||||
if expected_ranges.is_empty() {
|
||||
@@ -904,13 +1086,13 @@ mod tests {
|
||||
),
|
||||
None,
|
||||
);
|
||||
let matches = matcher.finish();
|
||||
|
||||
assert_eq!(matches.len(), 1);
|
||||
assert_eq!(
|
||||
matcher.line_pairs(&matches[0]),
|
||||
Some(&[(0, 3), (1, 5), (2, 6), (3, 7)][..])
|
||||
);
|
||||
let SearchMatches::Fuzzy(matches) = matcher.finish() else {
|
||||
panic!("expected a fuzzy match");
|
||||
};
|
||||
let [search_match] = matches.as_slice() else {
|
||||
panic!("expected one match, got {}", matches.len());
|
||||
};
|
||||
assert_eq!(search_match.line_pairs, [(0, 3), (1, 5), (2, 6), (3, 7)]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -934,14 +1116,19 @@ mod tests {
|
||||
let mut matcher = StreamingFuzzyMatcher::new(buffer.snapshot().clone());
|
||||
|
||||
// The last query line is incomplete and gets appended to the match by
|
||||
// `finish` via verbatim comparison rather than the fuzzy search.
|
||||
matcher.push("}\n\n\n\nfn render_search", None);
|
||||
let matches = matcher.finish();
|
||||
|
||||
assert_eq!(matches.len(), 1);
|
||||
// `finish` via verbatim comparison rather than the fuzzy search. The
|
||||
// trailing space after `}` keeps the query from being an exact
|
||||
// substring of the buffer, so the fuzzy extension path is exercised.
|
||||
matcher.push("} \n\n\n\nfn render_search", None);
|
||||
let SearchMatches::Fuzzy(matches) = matcher.finish() else {
|
||||
panic!("expected a fuzzy match");
|
||||
};
|
||||
let [search_match] = matches.as_slice() else {
|
||||
panic!("expected one match, got {}", matches.len());
|
||||
};
|
||||
assert_eq!(
|
||||
matcher.line_pairs(&matches[0]),
|
||||
Some(&[(0, 2), (1, 3), (2, 4), (3, 5), (4, 6)][..])
|
||||
search_match.line_pairs,
|
||||
[(0, 2), (1, 3), (2, 4), (3, 5), (4, 6)]
|
||||
);
|
||||
assert_eq!(matcher.query_lines().len(), 5);
|
||||
}
|
||||
@@ -967,11 +1154,26 @@ mod tests {
|
||||
.map(|range| finder.snapshot.text_for_range(range).collect::<String>())
|
||||
}
|
||||
|
||||
fn search_matches(matches: &SearchMatches) -> &[SearchMatch] {
|
||||
match matches {
|
||||
SearchMatches::Exact(matches) | SearchMatches::Fuzzy(matches) => matches,
|
||||
}
|
||||
}
|
||||
|
||||
fn match_ranges(matches: &SearchMatches) -> Vec<Range<usize>> {
|
||||
search_matches(matches)
|
||||
.iter()
|
||||
.map(|search_match| search_match.range.clone())
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn finish(mut finder: StreamingFuzzyMatcher) -> Option<String> {
|
||||
let snapshot = finder.snapshot.clone();
|
||||
let matches = finder.finish();
|
||||
matches
|
||||
.first()
|
||||
.map(|range| snapshot.text_for_range(range.clone()).collect::<String>())
|
||||
search_matches(&matches).first().map(|search_match| {
|
||||
snapshot
|
||||
.text_for_range(search_match.range.clone())
|
||||
.collect::<String>()
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -114,7 +114,7 @@ impl StreamingParser {
|
||||
if partial.new_text.is_some() && !state.buffer_new_text_until_old_text_done {
|
||||
// new_text appeared after old_text, so old_text is done — emit everything.
|
||||
let start = find_char_boundary(old_text, state.old_text_emitted_len);
|
||||
let chunk = normalize_done_chunk(old_text[start..].to_string());
|
||||
let chunk = old_text[start..].to_string();
|
||||
state.old_text_done = true;
|
||||
state.old_text_emitted_len = old_text.len();
|
||||
events.push(EditEvent::OldTextChunk {
|
||||
@@ -217,12 +217,12 @@ impl StreamingParser {
|
||||
state.buffer_new_text_until_old_text_done = false;
|
||||
events.push(EditEvent::OldTextChunk {
|
||||
edit_index: index,
|
||||
chunk: normalize_done_chunk(edit.old_text.clone()),
|
||||
chunk: edit.old_text.clone(),
|
||||
done: true,
|
||||
});
|
||||
events.push(EditEvent::NewTextChunk {
|
||||
edit_index: index,
|
||||
chunk: normalize_done_chunk(edit.new_text.clone()),
|
||||
chunk: edit.new_text.clone(),
|
||||
done: true,
|
||||
});
|
||||
continue;
|
||||
@@ -230,7 +230,7 @@ impl StreamingParser {
|
||||
|
||||
if !state.old_text_done {
|
||||
let start = find_char_boundary(&edit.old_text, state.old_text_emitted_len);
|
||||
let chunk = normalize_done_chunk(edit.old_text[start..].to_string());
|
||||
let chunk = edit.old_text[start..].to_string();
|
||||
state.old_text_done = true;
|
||||
state.old_text_emitted_len = edit.old_text.len();
|
||||
events.push(EditEvent::OldTextChunk {
|
||||
@@ -242,7 +242,7 @@ impl StreamingParser {
|
||||
|
||||
if !state.new_text_done {
|
||||
let start = find_char_boundary(&edit.new_text, state.new_text_emitted_len);
|
||||
let chunk = normalize_done_chunk(edit.new_text[start..].to_string());
|
||||
let chunk = edit.new_text[start..].to_string();
|
||||
state.new_text_done = true;
|
||||
state.new_text_emitted_len = edit.new_text.len();
|
||||
events.push(EditEvent::NewTextChunk {
|
||||
@@ -301,12 +301,12 @@ impl StreamingParser {
|
||||
state.buffer_new_text_until_old_text_done = false;
|
||||
events.push(EditEvent::OldTextChunk {
|
||||
edit_index: previous_index,
|
||||
chunk: normalize_done_chunk(old_text.to_string()),
|
||||
chunk: old_text.to_string(),
|
||||
done: true,
|
||||
});
|
||||
events.push(EditEvent::NewTextChunk {
|
||||
edit_index: previous_index,
|
||||
chunk: normalize_done_chunk(new_text.to_string()),
|
||||
chunk: new_text.to_string(),
|
||||
done: true,
|
||||
});
|
||||
return Some(events);
|
||||
@@ -319,7 +319,7 @@ impl StreamingParser {
|
||||
state.old_text_emitted_len = old_text.len();
|
||||
events.push(EditEvent::OldTextChunk {
|
||||
edit_index: previous_index,
|
||||
chunk: normalize_done_chunk(old_text[start..].to_string()),
|
||||
chunk: old_text[start..].to_string(),
|
||||
done: true,
|
||||
});
|
||||
}
|
||||
@@ -332,7 +332,7 @@ impl StreamingParser {
|
||||
state.buffer_new_text_until_old_text_done = false;
|
||||
events.push(EditEvent::NewTextChunk {
|
||||
edit_index: previous_index,
|
||||
chunk: normalize_done_chunk(new_text[start..].to_string()),
|
||||
chunk: new_text[start..].to_string(),
|
||||
done: true,
|
||||
});
|
||||
}
|
||||
@@ -387,13 +387,6 @@ fn find_char_boundary(text: &str, target: usize) -> usize {
|
||||
pos
|
||||
}
|
||||
|
||||
fn normalize_done_chunk(mut chunk: String) -> String {
|
||||
if chunk.ends_with('\n') {
|
||||
chunk.pop();
|
||||
}
|
||||
chunk
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -555,7 +548,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_done_chunks_strip_trailing_newline() {
|
||||
fn test_done_chunks_preserve_trailing_newlines() {
|
||||
let mut parser = StreamingParser::default();
|
||||
|
||||
let events = parser.finalize_edits(&[Edit {
|
||||
@@ -567,12 +560,12 @@ mod tests {
|
||||
&[
|
||||
EditEvent::OldTextChunk {
|
||||
edit_index: 0,
|
||||
chunk: "before".into(),
|
||||
chunk: "before\n".into(),
|
||||
done: true,
|
||||
},
|
||||
EditEvent::NewTextChunk {
|
||||
edit_index: 0,
|
||||
chunk: "after".into(),
|
||||
chunk: "after\n".into(),
|
||||
done: true,
|
||||
},
|
||||
]
|
||||
@@ -580,7 +573,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_partial_edit_chunks_hold_back_trailing_newline() {
|
||||
fn test_partial_edit_preserves_trailing_newlines() {
|
||||
let mut parser = StreamingParser::default();
|
||||
|
||||
let events = parser.push_edits(&[PartialEdit {
|
||||
@@ -598,12 +591,12 @@ mod tests {
|
||||
&[
|
||||
EditEvent::OldTextChunk {
|
||||
edit_index: 0,
|
||||
chunk: "before".into(),
|
||||
chunk: "before\n".into(),
|
||||
done: true,
|
||||
},
|
||||
EditEvent::NewTextChunk {
|
||||
edit_index: 0,
|
||||
chunk: "after".into(),
|
||||
chunk: "after\n".into(),
|
||||
done: true,
|
||||
},
|
||||
]
|
||||
|
||||
Reference in new issue
Block a user