Yehowshua/ZIm
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Remove edit action markers literals from source (#27778)

Browse source 
Edit action markers look like git conflicts and can trip up tooling used
to resolve git conflicts. This PR creates them programmatically so that
they don't appear in source code.

Release Notes:

- N/A
This commit is contained in:
Agus Zubiaga 2025-03-31 10:48:35 -03:00 • committed by GitHub
parent 9b40770e9f
commit 9b44bacc28
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 203 additions and 147 deletions
Download patch file Download diff file Expand all files Collapse all files

344
crates/assistant_tools/src/edit_files_tool/edit_action.rs
View file

@ -59,6 +59,20 @@ enum State {
CloseFence, CloseFence,
} }
/// used to avoid having source code that looks like git-conflict markers
macro_rules! marker_sym {
($char:expr) => {
concat!($char, $char, $char, $char, $char, $char, $char)
};
}
const SEARCH_MARKER: &str = concat!(marker_sym!('<'), " SEARCH");
const DIVIDER: &str = marker_sym!('=');
const NL_DIVIDER: &str = concat!("\n", marker_sym!('='));
const REPLACE_MARKER: &str = concat!(marker_sym!('>'), " REPLACE");
const NL_REPLACE_MARKER: &str = concat!("\n", marker_sym!('>'), " REPLACE");
const FENCE: &str = "```";
impl EditActionParser { impl EditActionParser {
/// Creates a new `EditActionParser` /// Creates a new `EditActionParser`
pub fn new() -> Self { pub fn new() -> Self {
@ -87,13 +101,6 @@ impl EditActionParser {
pub fn parse_chunk(&mut self, input: &str) -> Vec<(EditAction, String)> { pub fn parse_chunk(&mut self, input: &str) -> Vec<(EditAction, String)> {
use State::*; use State::*;
const FENCE: &[u8] = b"```";
const SEARCH_MARKER: &[u8] = b"<<<<<<< SEARCH";
const DIVIDER: &[u8] = b"=======";
const NL_DIVIDER: &[u8] = b"\n=======";
const REPLACE_MARKER: &[u8] = b">>>>>>> REPLACE";
const NL_REPLACE_MARKER: &[u8] = b"\n>>>>>>> REPLACE";
let mut actions = Vec::new(); let mut actions = Vec::new();
for byte in input.bytes() { for byte in input.bytes() {
@ -227,7 +234,7 @@ impl EditActionParser {
self.to_state(State::Default); self.to_state(State::Default);
} }
fn expect_marker(&mut self, byte: u8, marker: &'static [u8], trailing_newline: bool) -> bool { fn expect_marker(&mut self, byte: u8, marker: &'static str, trailing_newline: bool) -> bool {
match self.match_marker(byte, marker, trailing_newline) { match self.match_marker(byte, marker, trailing_newline) {
MarkerMatch::Complete => true, MarkerMatch::Complete => true,
MarkerMatch::Partial => false, MarkerMatch::Partial => false,
@ -245,7 +252,7 @@ impl EditActionParser {
} }
} }
fn extend_block_range(&mut self, byte: u8, marker: &[u8], nl_marker: &[u8]) -> bool { fn extend_block_range(&mut self, byte: u8, marker: &str, nl_marker: &str) -> bool {
let marker = if self.block_range.is_empty() { let marker = if self.block_range.is_empty() {
// do not require another newline if block is empty // do not require another newline if block is empty
marker marker
@ -287,7 +294,7 @@ impl EditActionParser {
} }
} }
fn match_marker(&mut self, byte: u8, marker: &[u8], trailing_newline: bool) -> MarkerMatch { fn match_marker(&mut self, byte: u8, marker: &str, trailing_newline: bool) -> MarkerMatch {
if trailing_newline && self.marker_ix >= marker.len() { if trailing_newline && self.marker_ix >= marker.len() {
if byte == b'\n' { if byte == b'\n' {
MarkerMatch::Complete MarkerMatch::Complete
@ -296,7 +303,7 @@ impl EditActionParser {
} else { } else {
MarkerMatch::None MarkerMatch::None
} }
} else if byte == marker[self.marker_ix] { } else if byte == marker.as_bytes()[self.marker_ix] {
self.marker_ix += 1; self.marker_ix += 1;
if self.marker_ix < marker.len() || trailing_newline { if self.marker_ix < marker.len() || trailing_newline {
@ -321,7 +328,7 @@ enum MarkerMatch {
pub struct ParseError { pub struct ParseError {
line: usize, line: usize,
column: usize, column: usize,
expected: &'static [u8], expected: &'static str,
found: u8, found: u8,
} }
@ -330,10 +337,7 @@ impl std::fmt::Display for ParseError {
write!( write!(
f, f,
"input:{}:{}: Expected marker {:?}, found {:?}", "input:{}:{}: Expected marker {:?}, found {:?}",
self.line, self.line, self.column, self.expected, self.found as char
self.column,
String::from_utf8_lossy(self.expected),
self.found as char
) )
} }
} }
@ -344,20 +348,26 @@ mod tests {
use rand::prelude::*; use rand::prelude::*;
use util::line_endings; use util::line_endings;
const WRONG_MARKER: &str = concat!(marker_sym!('<'), " WRONG_MARKER");
#[test] #[test]
fn test_simple_edit_action() { fn test_simple_edit_action() {
let input = r#"src/main.rs // Construct test input using format with multiline string literals
let input = format!(
r#"src/main.rs
``` ```
<<<<<<< SEARCH {}
fn original() {} fn original() {{}}
======= {}
fn replacement() {} fn replacement() {{}}
>>>>>>> REPLACE {}
``` ```
"#; "#,
SEARCH_MARKER, DIVIDER, REPLACE_MARKER
);
let mut parser = EditActionParser::new(); let mut parser = EditActionParser::new();
let actions = parser.parse_chunk(input); let actions = parser.parse_chunk(&input);
assert_no_errors(&parser); assert_no_errors(&parser);
assert_eq!(actions.len(), 1); assert_eq!(actions.len(), 1);
@ -373,18 +383,22 @@ fn replacement() {}
#[test] #[test]
fn test_with_language_tag() { fn test_with_language_tag() {
let input = r#"src/main.rs // Construct test input using format with multiline string literals
let input = format!(
r#"src/main.rs
```rust ```rust
<<<<<<< SEARCH {}
fn original() {} fn original() {{}}
======= {}
fn replacement() {} fn replacement() {{}}
>>>>>>> REPLACE {}
``` ```
"#; "#,
SEARCH_MARKER, DIVIDER, REPLACE_MARKER
);
let mut parser = EditActionParser::new(); let mut parser = EditActionParser::new();
let actions = parser.parse_chunk(input); let actions = parser.parse_chunk(&input);
assert_no_errors(&parser); assert_no_errors(&parser);
assert_eq!(actions.len(), 1); assert_eq!(actions.len(), 1);
@ -400,22 +414,26 @@ fn replacement() {}
#[test] #[test]
fn test_with_surrounding_text() { fn test_with_surrounding_text() {
let input = r#"Here's a modification I'd like to make to the file: // Construct test input using format with multiline string literals
let input = format!(
r#"Here's a modification I'd like to make to the file:
src/main.rs src/main.rs
```rust ```rust
<<<<<<< SEARCH {}
fn original() {} fn original() {{}}
======= {}
fn replacement() {} fn replacement() {{}}
>>>>>>> REPLACE {}
``` ```
This change makes the function better. This change makes the function better.
"#; "#,
SEARCH_MARKER, DIVIDER, REPLACE_MARKER
);
let mut parser = EditActionParser::new(); let mut parser = EditActionParser::new();
let actions = parser.parse_chunk(input); let actions = parser.parse_chunk(&input);
assert_no_errors(&parser); assert_no_errors(&parser);
assert_eq!(actions.len(), 1); assert_eq!(actions.len(), 1);
@ -431,29 +449,33 @@ This change makes the function better.
#[test] #[test]
fn test_multiple_edit_actions() { fn test_multiple_edit_actions() {
let input = r#"First change: // Construct test input using format with multiline string literals
let input = format!(
r#"First change:
src/main.rs src/main.rs
``` ```
<<<<<<< SEARCH {}
fn original() {} fn original() {{}}
======= {}
fn replacement() {} fn replacement() {{}}
>>>>>>> REPLACE {}
``` ```
Second change: Second change:
src/utils.rs src/utils.rs
```rust ```rust
<<<<<<< SEARCH {}
fn old_util() -> bool { false } fn old_util() -> bool {{ false }}
======= {}
fn new_util() -> bool { true } fn new_util() -> bool {{ true }}
>>>>>>> REPLACE {}
``` ```
"#; "#,
SEARCH_MARKER, DIVIDER, REPLACE_MARKER, SEARCH_MARKER, DIVIDER, REPLACE_MARKER
);
let mut parser = EditActionParser::new(); let mut parser = EditActionParser::new();
let actions = parser.parse_chunk(input); let actions = parser.parse_chunk(&input);
assert_no_errors(&parser); assert_no_errors(&parser);
assert_eq!(actions.len(), 2); assert_eq!(actions.len(), 2);
@ -480,32 +502,36 @@ fn new_util() -> bool { true }
#[test] #[test]
fn test_multiline() { fn test_multiline() {
let input = r#"src/main.rs // Construct test input using format with multiline string literals
let input = format!(
r#"src/main.rs
```rust ```rust
<<<<<<< SEARCH {}
fn original() { fn original() {{
println!("This is the original function"); println!("This is the original function");
let x = 42; let x = 42;
if x > 0 { if x > 0 {{
println!("Positive number"); println!("Positive number");
} }}
} }}
======= {}
fn replacement() { fn replacement() {{
println!("This is the replacement function"); println!("This is the replacement function");
let x = 100; let x = 100;
if x > 50 { if x > 50 {{
println!("Large number"); println!("Large number");
} else { }} else {{
println!("Small number"); println!("Small number");
} }}
} }}
>>>>>>> REPLACE {}
``` ```
"#; "#,
SEARCH_MARKER, DIVIDER, REPLACE_MARKER
);
let mut parser = EditActionParser::new(); let mut parser = EditActionParser::new();
let actions = parser.parse_chunk(input); let actions = parser.parse_chunk(&input);
assert_no_errors(&parser); assert_no_errors(&parser);
assert_eq!(actions.len(), 1); assert_eq!(actions.len(), 1);
@ -523,21 +549,25 @@ fn replacement() {
#[test] #[test]
fn test_write_action() { fn test_write_action() {
let input = r#"Create a new main.rs file: // Construct test input using format with multiline string literals
let input = format!(
r#"Create a new main.rs file:
src/main.rs src/main.rs
```rust ```rust
<<<<<<< SEARCH {}
======= {}
fn new_function() { fn new_function() {{
println!("This function is being added"); println!("This function is being added");
} }}
>>>>>>> REPLACE {}
``` ```
"#; "#,
SEARCH_MARKER, DIVIDER, REPLACE_MARKER
);
let mut parser = EditActionParser::new(); let mut parser = EditActionParser::new();
let actions = parser.parse_chunk(input); let actions = parser.parse_chunk(&input);
assert_no_errors(&parser); assert_no_errors(&parser);
assert_eq!(actions.len(), 1); assert_eq!(actions.len(), 1);
@ -553,16 +583,20 @@ fn new_function() {
#[test] #[test]
fn test_empty_replace() { fn test_empty_replace() {
let input = r#"src/main.rs // Construct test input using format with multiline string literals
let input = format!(
r#"src/main.rs
```rust ```rust
<<<<<<< SEARCH {}
fn this_will_be_deleted() { fn this_will_be_deleted() {{
println!("Deleting this function"); println!("Deleting this function");
} }}
======= {}
>>>>>>> REPLACE {}
``` ```
"#; "#,
SEARCH_MARKER, DIVIDER, REPLACE_MARKER
);
let mut parser = EditActionParser::new(); let mut parser = EditActionParser::new();
let actions = parser.parse_chunk(&input); let actions = parser.parse_chunk(&input);
@ -597,16 +631,20 @@ fn this_will_be_deleted() {
#[test] #[test]
fn test_empty_both() { fn test_empty_both() {
let input = r#"src/main.rs // Construct test input using format with multiline string literals
let input = format!(
r#"src/main.rs
```rust ```rust
<<<<<<< SEARCH {}
======= {}
>>>>>>> REPLACE {}
``` ```
"#; "#,
SEARCH_MARKER, DIVIDER, REPLACE_MARKER
);
let mut parser = EditActionParser::new(); let mut parser = EditActionParser::new();
let actions = parser.parse_chunk(input); let actions = parser.parse_chunk(&input);
assert_eq!(actions.len(), 1); assert_eq!(actions.len(), 1);
assert_eq!( assert_eq!(
@ -621,31 +659,24 @@ fn this_will_be_deleted() {
#[test] #[test]
fn test_resumability() { fn test_resumability() {
let input_part1 = r#"src/main.rs // Construct test input using format with multiline string literals
```rust let input_part1 = format!("src/main.rs\n```rust\n{}\nfn ori", SEARCH_MARKER);
<<<<<<< SEARCH
fn ori"#;
let input_part2 = r#"ginal() {} let input_part2 = format!("ginal() {{}}\n{}\nfn replacement() {{}}", DIVIDER);
=======
fn replacement() {}"#;
let input_part3 = r#" let input_part3 = format!("\n{}\n```\n", REPLACE_MARKER);
>>>>>>> REPLACE
```
"#;
let mut parser = EditActionParser::new(); let mut parser = EditActionParser::new();
let actions1 = parser.parse_chunk(input_part1); let actions1 = parser.parse_chunk(&input_part1);
assert_no_errors(&parser); assert_no_errors(&parser);
assert_eq!(actions1.len(), 0); assert_eq!(actions1.len(), 0);
let actions2 = parser.parse_chunk(input_part2); let actions2 = parser.parse_chunk(&input_part2);
// No actions should be complete yet // No actions should be complete yet
assert_no_errors(&parser); assert_no_errors(&parser);
assert_eq!(actions2.len(), 0); assert_eq!(actions2.len(), 0);
let actions3 = parser.parse_chunk(input_part3); let actions3 = parser.parse_chunk(&input_part3);
// The third chunk should complete the action // The third chunk should complete the action
assert_no_errors(&parser); assert_no_errors(&parser);
assert_eq!(actions3.len(), 1); assert_eq!(actions3.len(), 1);
@ -663,18 +694,17 @@ fn replacement() {}"#;
#[test] #[test]
fn test_parser_state_preservation() { fn test_parser_state_preservation() {
let mut parser = EditActionParser::new(); let mut parser = EditActionParser::new();
let actions1 = parser.parse_chunk("src/main.rs\n```rust\n<<<<<<< SEARCH\n"); let first_chunk = format!("src/main.rs\n```rust\n{}\n", SEARCH_MARKER);
let actions1 = parser.parse_chunk(&first_chunk);
// Check parser is in the correct state // Check parser is in the correct state
assert_no_errors(&parser); assert_no_errors(&parser);
assert_eq!(parser.state, State::SearchBlock); assert_eq!(parser.state, State::SearchBlock);
assert_eq!( assert_eq!(parser.action_source, first_chunk.as_bytes());
parser.action_source,
b"src/main.rs\n```rust\n<<<<<<< SEARCH\n"
);
// Continue parsing // Continue parsing
let actions2 = parser.parse_chunk("original code\n=======\n"); let second_chunk = format!("original code\n{}\n", DIVIDER);
let actions2 = parser.parse_chunk(&second_chunk);
assert_no_errors(&parser); assert_no_errors(&parser);
assert_eq!(parser.state, State::ReplaceBlock); assert_eq!(parser.state, State::ReplaceBlock);
@ -683,7 +713,8 @@ fn replacement() {}"#;
b"original code" b"original code"
); );
let actions3 = parser.parse_chunk("replacement code\n>>>>>>> REPLACE\n```\n"); let third_chunk = format!("replacement code\n{}\n```\n", REPLACE_MARKER);
let actions3 = parser.parse_chunk(&third_chunk);
// After complete parsing, state should reset // After complete parsing, state should reset
assert_no_errors(&parser); assert_no_errors(&parser);
@ -699,18 +730,21 @@ fn replacement() {}"#;
#[test] #[test]
fn test_invalid_search_marker() { fn test_invalid_search_marker() {
let input = r#"src/main.rs let input = format!(
r#"src/main.rs
```rust ```rust
<<<<<<< WRONG_MARKER {}
fn original() {} fn original() {{}}
======= {}
fn replacement() {} fn replacement() {{}}
>>>>>>> REPLACE {}
``` ```
"#; "#,
WRONG_MARKER, DIVIDER, REPLACE_MARKER
);
let mut parser = EditActionParser::new(); let mut parser = EditActionParser::new();
let actions = parser.parse_chunk(input); let actions = parser.parse_chunk(&input);
assert_eq!(actions.len(), 0); assert_eq!(actions.len(), 0);
assert_eq!(parser.errors().len(), 1); assert_eq!(parser.errors().len(), 1);
@ -718,33 +752,40 @@ fn replacement() {}
assert_eq!( assert_eq!(
error.to_string(), error.to_string(),
"input:3:9: Expected marker \"<<<<<<< SEARCH\", found 'W'" format!(
"input:3:9: Expected marker \"{}\", found 'W'",
SEARCH_MARKER
)
); );
} }
#[test] #[test]
fn test_missing_closing_fence() { fn test_missing_closing_fence() {
let input = r#"src/main.rs // Construct test input using format with multiline string literals
let input = format!(
r#"src/main.rs
```rust ```rust
<<<<<<< SEARCH {}
fn original() {} fn original() {{}}
======= {}
fn replacement() {} fn replacement() {{}}
>>>>>>> REPLACE {}
<!-- Missing closing fence --> <!-- Missing closing fence -->
src/utils.rs src/utils.rs
```rust ```rust
<<<<<<< SEARCH {}
fn utils_func() {} fn utils_func() {{}}
======= {}
fn new_utils_func() {} fn new_utils_func() {{}}
>>>>>>> REPLACE {}
``` ```
"#; "#,
SEARCH_MARKER, DIVIDER, REPLACE_MARKER, SEARCH_MARKER, DIVIDER, REPLACE_MARKER
);
let mut parser = EditActionParser::new(); let mut parser = EditActionParser::new();
let actions = parser.parse_chunk(input); let actions = parser.parse_chunk(&input);
// Only the second block should be parsed // Only the second block should be parsed
assert_eq!(actions.len(), 1); assert_eq!(actions.len(), 1);
@ -851,38 +892,53 @@ fn new_utils_func() {}
// The system prompt includes some text that would produce errors // The system prompt includes some text that would produce errors
assert_eq!( assert_eq!(
errors[0].to_string(), errors[0].to_string(),
"input:102:1: Expected marker \"<<<<<<< SEARCH\", found '3'" format!(
"input:102:1: Expected marker \"{}\", found '3'",
SEARCH_MARKER
)
); );
#[cfg(not(windows))] #[cfg(not(windows))]
assert_eq!( assert_eq!(
errors[1].to_string(), errors[1].to_string(),
"input:109:0: Expected marker \"<<<<<<< SEARCH\", found '\\n'" format!(
"input:109:0: Expected marker \"{}\", found '\\n'",
SEARCH_MARKER
)
); );
#[cfg(windows)] #[cfg(windows)]
assert_eq!( assert_eq!(
errors[1].to_string(), errors[1].to_string(),
"input:108:1: Expected marker \"<<<<<<< SEARCH\", found '\\r'" format!(
"input:108:1: Expected marker \"{}\", found '\\r'",
SEARCH_MARKER
)
); );
} }
#[test] #[test]
fn test_print_error() { fn test_print_error() {
let input = r#"src/main.rs let input = format!(
r#"src/main.rs
```rust ```rust
<<<<<<< WRONG_MARKER {}
fn original() {} fn original() {{}}
======= {}
fn replacement() {} fn replacement() {{}}
>>>>>>> REPLACE {}
``` ```
"#; "#,
WRONG_MARKER, DIVIDER, REPLACE_MARKER
);
let mut parser = EditActionParser::new(); let mut parser = EditActionParser::new();
parser.parse_chunk(input); parser.parse_chunk(&input);
assert_eq!(parser.errors().len(), 1); assert_eq!(parser.errors().len(), 1);
let error = &parser.errors()[0]; let error = &parser.errors()[0];
let expected_error = r#"input:3:9: Expected marker "<<<<<<< SEARCH", found 'W'"#; let expected_error = format!(
r#"input:3:9: Expected marker "{}", found 'W'"#,
SEARCH_MARKER
);
assert_eq!(format!("{}", error), expected_error); assert_eq!(format!("{}", error), expected_error);
} }

6
crates/assistant_tools/src/find_replace_file_tool.rs
View file

@ -65,7 +65,7 @@ pub struct FindReplaceFileToolInput {
/// <example> /// <example>
/// If a file contains this code: /// If a file contains this code:
/// ///
/// ```rust /// ```ignore
/// fn check_user_permissions(user_id: &str) -> Result<bool> { /// fn check_user_permissions(user_id: &str) -> Result<bool> {
/// // Check if user exists first /// // Check if user exists first
/// let user = database.find_user(user_id)?; /// let user = database.find_user(user_id)?;
@ -83,7 +83,7 @@ pub struct FindReplaceFileToolInput {
/// Your find string should include at least 3 lines of context before and after the part /// Your find string should include at least 3 lines of context before and after the part
/// you want to change: /// you want to change:
/// ///
/// ``` /// ```ignore
/// fn check_user_permissions(user_id: &str) -> Result<bool> { /// fn check_user_permissions(user_id: &str) -> Result<bool> {
/// // Check if user exists first /// // Check if user exists first
/// let user = database.find_user(user_id)?; /// let user = database.find_user(user_id)?;
@ -100,7 +100,7 @@ pub struct FindReplaceFileToolInput {
/// ///
/// And your replace string might look like: /// And your replace string might look like:
/// ///
/// ``` /// ```ignore
/// fn check_user_permissions(user_id: &str) -> Result<bool> { /// fn check_user_permissions(user_id: &str) -> Result<bool> {
/// // Check if user exists first /// // Check if user exists first
/// let user = database.find_user(user_id)?; /// let user = database.find_user(user_id)?;