diff --git a/crates/assistant_tools/src/edit_agent/evals/fixtures/disable_cursor_blinking/before.rs b/crates/assistant_tools/src/edit_agent/evals/fixtures/disable_cursor_blinking/before.rs index a070738b60..607daa8ce3 100644 --- a/crates/assistant_tools/src/edit_agent/evals/fixtures/disable_cursor_blinking/before.rs +++ b/crates/assistant_tools/src/edit_agent/evals/fixtures/disable_cursor_blinking/before.rs @@ -9132,7 +9132,7 @@ impl Editor { window: &mut Window, cx: &mut Context, ) { - self.manipulate_immutable_lines(window, cx, |lines| lines.sort()) + self.manipulate_lines(window, cx, |lines| lines.sort()) } pub fn sort_lines_case_insensitive( @@ -9141,7 +9141,7 @@ impl Editor { window: &mut Window, cx: &mut Context, ) { - self.manipulate_immutable_lines(window, cx, |lines| { + self.manipulate_lines(window, cx, |lines| { lines.sort_by_key(|line| line.to_lowercase()) }) } @@ -9152,7 +9152,7 @@ impl Editor { window: &mut Window, cx: &mut Context, ) { - self.manipulate_immutable_lines(window, cx, |lines| { + self.manipulate_lines(window, cx, |lines| { let mut seen = HashSet::default(); lines.retain(|line| seen.insert(line.to_lowercase())); }) @@ -9164,7 +9164,7 @@ impl Editor { window: &mut Window, cx: &mut Context, ) { - self.manipulate_immutable_lines(window, cx, |lines| { + self.manipulate_lines(window, cx, |lines| { let mut seen = HashSet::default(); lines.retain(|line| seen.insert(*line)); }) @@ -9606,20 +9606,20 @@ impl Editor { } pub fn reverse_lines(&mut self, _: &ReverseLines, window: &mut Window, cx: &mut Context) { - self.manipulate_immutable_lines(window, cx, |lines| lines.reverse()) + self.manipulate_lines(window, cx, |lines| lines.reverse()) } pub fn shuffle_lines(&mut self, _: &ShuffleLines, window: &mut Window, cx: &mut Context) { - self.manipulate_immutable_lines(window, cx, |lines| lines.shuffle(&mut thread_rng())) + self.manipulate_lines(window, cx, |lines| lines.shuffle(&mut thread_rng())) } - fn manipulate_lines( + fn manipulate_lines( &mut self, window: &mut Window, cx: &mut Context, - mut manipulate: M, + mut callback: Fn, ) where - M: FnMut(&str) -> LineManipulationResult, + Fn: FnMut(&mut Vec<&str>), { self.hide_mouse_cursor(&HideMouseCursorOrigin::TypingAction); @@ -9652,14 +9652,18 @@ impl Editor { .text_for_range(start_point..end_point) .collect::(); - let LineManipulationResult { new_text, line_count_before, line_count_after} = manipulate(&text); + let mut lines = text.split('\n').collect_vec(); - edits.push((start_point..end_point, new_text)); + let lines_before = lines.len(); + callback(&mut lines); + let lines_after = lines.len(); + + edits.push((start_point..end_point, lines.join("\n"))); // Selections must change based on added and removed line count let start_row = MultiBufferRow(start_point.row + added_lines as u32 - removed_lines as u32); - let end_row = MultiBufferRow(start_row.0 + line_count_after.saturating_sub(1) as u32); + let end_row = MultiBufferRow(start_row.0 + lines_after.saturating_sub(1) as u32); new_selections.push(Selection { id: selection.id, start: start_row, @@ -9668,10 +9672,10 @@ impl Editor { reversed: selection.reversed, }); - if line_count_after > line_count_before { - added_lines += line_count_after - line_count_before; - } else if line_count_before > line_count_after { - removed_lines += line_count_before - line_count_after; + if lines_after > lines_before { + added_lines += lines_after - lines_before; + } else if lines_before > lines_after { + removed_lines += lines_before - lines_after; } } @@ -9716,171 +9720,6 @@ impl Editor { }) } - fn manipulate_immutable_lines( - &mut self, - window: &mut Window, - cx: &mut Context, - mut callback: Fn, - ) where - Fn: FnMut(&mut Vec<&str>), - { - self.manipulate_lines(window, cx, |text| { - let mut lines: Vec<&str> = text.split('\n').collect(); - let line_count_before = lines.len(); - - callback(&mut lines); - - LineManipulationResult { - new_text: lines.join("\n"), - line_count_before, - line_count_after: lines.len(), - } - }); - } - - fn manipulate_mutable_lines( - &mut self, - window: &mut Window, - cx: &mut Context, - mut callback: Fn, - ) where - Fn: FnMut(&mut Vec>), - { - self.manipulate_lines(window, cx, |text| { - let mut lines: Vec> = text.split('\n').map(Cow::from).collect(); - let line_count_before = lines.len(); - - callback(&mut lines); - - LineManipulationResult { - new_text: lines.join("\n"), - line_count_before, - line_count_after: lines.len(), - } - }); - } - - pub fn convert_indentation_to_spaces( - &mut self, - _: &ConvertIndentationToSpaces, - window: &mut Window, - cx: &mut Context, - ) { - let settings = self.buffer.read(cx).language_settings(cx); - let tab_size = settings.tab_size.get() as usize; - - self.manipulate_mutable_lines(window, cx, |lines| { - // Allocates a reasonably sized scratch buffer once for the whole loop - let mut reindented_line = String::with_capacity(MAX_LINE_LEN); - // Avoids recomputing spaces that could be inserted many times - let space_cache: Vec> = (1..=tab_size) - .map(|n| IndentSize::spaces(n as u32).chars().collect()) - .collect(); - - for line in lines.iter_mut().filter(|line| !line.is_empty()) { - let mut chars = line.as_ref().chars(); - let mut col = 0; - let mut changed = false; - - while let Some(ch) = chars.next() { - match ch { - ' ' => { - reindented_line.push(' '); - col += 1; - } - '\t' => { - // \t are converted to spaces depending on the current column - let spaces_len = tab_size - (col % tab_size); - reindented_line.extend(&space_cache[spaces_len - 1]); - col += spaces_len; - changed = true; - } - _ => { - // If we dont append before break, the character is consumed - reindented_line.push(ch); - break; - } - } - } - - if !changed { - reindented_line.clear(); - continue; - } - // Append the rest of the line and replace old reference with new one - reindented_line.extend(chars); - *line = Cow::Owned(reindented_line.clone()); - reindented_line.clear(); - } - }); - } - - pub fn convert_indentation_to_tabs( - &mut self, - _: &ConvertIndentationToTabs, - window: &mut Window, - cx: &mut Context, - ) { - let settings = self.buffer.read(cx).language_settings(cx); - let tab_size = settings.tab_size.get() as usize; - - self.manipulate_mutable_lines(window, cx, |lines| { - // Allocates a reasonably sized buffer once for the whole loop - let mut reindented_line = String::with_capacity(MAX_LINE_LEN); - // Avoids recomputing spaces that could be inserted many times - let space_cache: Vec> = (1..=tab_size) - .map(|n| IndentSize::spaces(n as u32).chars().collect()) - .collect(); - - for line in lines.iter_mut().filter(|line| !line.is_empty()) { - let mut chars = line.chars(); - let mut spaces_count = 0; - let mut first_non_indent_char = None; - let mut changed = false; - - while let Some(ch) = chars.next() { - match ch { - ' ' => { - // Keep track of spaces. Append \t when we reach tab_size - spaces_count += 1; - changed = true; - if spaces_count == tab_size { - reindented_line.push('\t'); - spaces_count = 0; - } - } - '\t' => { - reindented_line.push('\t'); - spaces_count = 0; - } - _ => { - // Dont append it yet, we might have remaining spaces - first_non_indent_char = Some(ch); - break; - } - } - } - - if !changed { - reindented_line.clear(); - continue; - } - // Remaining spaces that didn't make a full tab stop - if spaces_count > 0 { - reindented_line.extend(&space_cache[spaces_count - 1]); - } - // If we consume an extra character that was not indentation, add it back - if let Some(extra_char) = first_non_indent_char { - reindented_line.push(extra_char); - } - // Append the rest of the line and replace old reference with new one - reindented_line.extend(chars); - *line = Cow::Owned(reindented_line.clone()); - reindented_line.clear(); - } - }); - } - pub fn convert_to_upper_case( &mut self, _: &ConvertToUpperCase, @@ -21318,13 +21157,6 @@ pub struct LineHighlight { pub type_id: Option, } -struct LineManipulationResult { - pub new_text: String, - pub line_count_before: usize, - pub line_count_after: usize, -} - - fn render_diff_hunk_controls( row: u32, status: &DiffHunkStatus, diff --git a/script/danger/dangerfile.ts b/script/danger/dangerfile.ts index 3f9c80586e..6ed4a27fed 100644 --- a/script/danger/dangerfile.ts +++ b/script/danger/dangerfile.ts @@ -94,3 +94,24 @@ for (const promptPath of modifiedPrompts) { ); } } + +const FIXTURE_CHANGE_ATTESTATION = "Changes to test fixtures are intentional and necessary."; + +const FIXTURES_PATHS = ["crates/assistant_tools/src/edit_agent/evals/fixtures"]; + +const modifiedFixtures = danger.git.modified_files.filter((file) => + FIXTURES_PATHS.some((fixturePath) => file.includes(fixturePath)), +); + +if (modifiedFixtures.length > 0) { + if (!body.includes(FIXTURE_CHANGE_ATTESTATION)) { + const modifiedFixturesStr = modifiedFixtures.map((path) => "`" + path + "`").join(", "); + fail( + [ + `This PR modifies eval or test fixtures (${modifiedFixturesStr}), which are typically expected to remain unchanged.`, + "If these changes are intentional and required, please add the following attestation to your PR description: ", + `"${FIXTURE_CHANGE_ATTESTATION}"`, + ].join("\n\n"), + ); + } +}