diff --git a/packages/builtin-tools/src/tools/FileEditTool/FileEditTool.ts b/packages/builtin-tools/src/tools/FileEditTool/FileEditTool.ts index 29c937b0b..81bf2d67a 100644 --- a/packages/builtin-tools/src/tools/FileEditTool/FileEditTool.ts +++ b/packages/builtin-tools/src/tools/FileEditTool/FileEditTool.ts @@ -70,7 +70,6 @@ import { areFileEditsInputsEquivalent, findActualString, getPatchForEdit, - preserveQuoteStyle, } from './utils.js' // V8/Bun string length limit is ~2^30 characters (~1 billion). For typical @@ -297,7 +296,7 @@ export const FileEditTool = buildTool({ const file = fileContent - // Use findActualString to handle quote normalization + // Use findActualString to find exact match const actualOldString = findActualString(file, old_string) if (!actualOldString) { return { @@ -452,23 +451,16 @@ export const FileEditTool = buildTool({ } } - // 3. Use findActualString to handle quote normalization + // 3. Find the exact string in file content const actualOldString = findActualString(originalFileContents, old_string) || old_string - // Preserve curly quotes in new_string when the file uses them - const actualNewString = preserveQuoteStyle( - old_string, - actualOldString, - new_string, - ) - // 4. Generate patch const { patch, updatedFile } = getPatchForEdit({ filePath: absoluteFilePath, fileContents: originalFileContents, oldString: actualOldString, - newString: actualNewString, + newString: new_string, replaceAll: replace_all, }) diff --git a/packages/builtin-tools/src/tools/FileEditTool/UI.tsx b/packages/builtin-tools/src/tools/FileEditTool/UI.tsx index 8a85a86e7..f00bb9bd7 100644 --- a/packages/builtin-tools/src/tools/FileEditTool/UI.tsx +++ b/packages/builtin-tools/src/tools/FileEditTool/UI.tsx @@ -20,7 +20,7 @@ import { readEditContext } from 'src/utils/readEditContext.js'; import { firstLineOf } from 'src/utils/stringUtils.js'; import type { ThemeName } from 'src/utils/theme.js'; import type { FileEditOutput } from './types.js'; -import { findActualString, getPatchForEdit, preserveQuoteStyle } from './utils.js'; +import { findActualString, getPatchForEdit } from './utils.js'; export function userFacingName( input: @@ -265,12 +265,11 @@ async function loadRejectionDiff( return { patch, firstLine: null, fileContent: undefined }; } const actualOld = findActualString(ctx.content, oldString) || oldString; - const actualNew = preserveQuoteStyle(oldString, actualOld, newString); const { patch } = getPatchForEdit({ filePath, fileContents: ctx.content, oldString: actualOld, - newString: actualNew, + newString: newString, replaceAll, }); return { diff --git a/packages/builtin-tools/src/tools/FileEditTool/__tests__/utils.test.ts b/packages/builtin-tools/src/tools/FileEditTool/__tests__/utils.test.ts index f7855798c..98f2c15d2 100644 --- a/packages/builtin-tools/src/tools/FileEditTool/__tests__/utils.test.ts +++ b/packages/builtin-tools/src/tools/FileEditTool/__tests__/utils.test.ts @@ -4,45 +4,8 @@ import { logMock } from '../../../../../../tests/mocks/log' // Mock log.ts to cut the heavy dependency chain mock.module('src/utils/log.ts', logMock) -const { - normalizeQuotes, - stripTrailingWhitespace, - findActualString, - preserveQuoteStyle, - applyEditToFile, - LEFT_SINGLE_CURLY_QUOTE, - RIGHT_SINGLE_CURLY_QUOTE, - LEFT_DOUBLE_CURLY_QUOTE, - RIGHT_DOUBLE_CURLY_QUOTE, -} = await import('../utils') - -// ─── normalizeQuotes ──────────────────────────────────────────────────── - -describe('normalizeQuotes', () => { - test('converts left single curly to straight', () => { - expect(normalizeQuotes(`${LEFT_SINGLE_CURLY_QUOTE}hello`)).toBe("'hello") - }) - - test('converts right single curly to straight', () => { - expect(normalizeQuotes(`hello${RIGHT_SINGLE_CURLY_QUOTE}`)).toBe("hello'") - }) - - test('converts left double curly to straight', () => { - expect(normalizeQuotes(`${LEFT_DOUBLE_CURLY_QUOTE}hello`)).toBe('"hello') - }) - - test('converts right double curly to straight', () => { - expect(normalizeQuotes(`hello${RIGHT_DOUBLE_CURLY_QUOTE}`)).toBe('hello"') - }) - - test('leaves straight quotes unchanged', () => { - expect(normalizeQuotes('\'hello\' "world"')).toBe('\'hello\' "world"') - }) - - test('handles empty string', () => { - expect(normalizeQuotes('')).toBe('') - }) -}) +const { stripTrailingWhitespace, findActualString, applyEditToFile } = + await import('../utils') // ─── stripTrailingWhitespace ──────────────────────────────────────────── @@ -91,12 +54,6 @@ describe('findActualString', () => { expect(findActualString('hello world', 'hello')).toBe('hello') }) - test('finds match with curly quotes normalized', () => { - const fileContent = `${LEFT_DOUBLE_CURLY_QUOTE}hello${RIGHT_DOUBLE_CURLY_QUOTE}` - const result = findActualString(fileContent, '"hello"') - expect(result).not.toBeNull() - }) - test('returns null when not found', () => { expect(findActualString('hello world', 'xyz')).toBeNull() }) @@ -107,124 +64,13 @@ describe('findActualString', () => { expect(result).toBe('') }) - // ── Tab/space normalization (Bug #2 reproduction) ── - - test('finds match when search uses spaces but file uses tabs', () => { - // File content uses Tab indentation - const fileContent = '\tif (x) {\n\t\treturn 1;\n\t}' - // User copies from Read output which renders tabs as spaces - const searchWithSpaces = ' if (x) {\n return 1;\n }' - const result = findActualString(fileContent, searchWithSpaces) - expect(result).not.toBeNull() - expect(result).toBe(fileContent) - }) - - test('finds match when search mixes tabs and spaces inconsistently', () => { - const fileContent = '\tconst x = 1; // comment' - const searchMixed = ' const x = 1; // comment' - const result = findActualString(fileContent, searchMixed) - expect(result).not.toBeNull() - }) - - test('finds match for single-line tab-to-space mismatch', () => { - const fileContent = '\t\torder_price = NormalizeDouble(ask, digits);' - const searchSpaces = ' order_price = NormalizeDouble(ask, digits);' - const result = findActualString(fileContent, searchSpaces) - expect(result).not.toBeNull() - }) - - // ── CJK / UTF-8 characters (Bug #1 reproduction) ── + // ── CJK / UTF-8 characters ── test('finds match with CJK characters in content', () => { const fileContent = 'input int x = 620; // 止盈点数(点) — 32个pip=320点' const result = findActualString(fileContent, fileContent) expect(result).toBe(fileContent) }) - - test('finds match with CJK characters when tab/space differs', () => { - const fileContent = '\t// 向上突破 → Sell Limit (逆方向做空)' - const searchSpaces = ' // 向上突破 → Sell Limit (逆方向做空)' - const result = findActualString(fileContent, searchSpaces) - expect(result).not.toBeNull() - expect(result).toBe(fileContent) - }) - - // ── Multiline with tabs + CJK (combined Bug #1 + #2) ── - - test('finds multiline match with tabs and CJK characters', () => { - const fileContent = - '\tif(effective_dir == BREAKOUT_UP)\n\t\t{\n\t\t\t// 向上突破\n\t\t}' - const searchSpaces = - ' if(effective_dir == BREAKOUT_UP)\n {\n // 向上突破\n }' - const result = findActualString(fileContent, searchSpaces) - expect(result).not.toBeNull() - expect(result).toBe(fileContent) - }) - - // ── Returned string must be a valid substring of fileContent ── - - test('returned string from tab match is a real substring of fileContent', () => { - const fileContent = 'prefix\n\t\tindented code\nsuffix' - const searchSpaces = 'prefix\n indented code\nsuffix' - const result = findActualString(fileContent, searchSpaces) - expect(result).not.toBeNull() - expect(fileContent.includes(result!)).toBe(true) - }) - - test('returned string from partial tab match is a real substring', () => { - const fileContent = 'line1\n\tif (x) {\n\t\tdoStuff();\n\t}\nline5' - const searchSpaces = ' if (x) {\n doStuff();\n }' - const result = findActualString(fileContent, searchSpaces) - expect(result).not.toBeNull() - expect(fileContent.includes(result!)).toBe(true) - }) - - test('tab match with mixed indentation levels', () => { - const fileContent = - 'class Foo {\n\t\tmethod1() {\n\t\t\treturn 42;\n\t\t}\n}' - const searchSpaces = - 'class Foo {\n method1() {\n return 42;\n }\n}' - const result = findActualString(fileContent, searchSpaces) - expect(result).not.toBeNull() - expect(fileContent.includes(result!)).toBe(true) - }) -}) - -// ─── preserveQuoteStyle ───────────────────────────────────────────────── - -describe('preserveQuoteStyle', () => { - test('returns newString unchanged when no normalization happened', () => { - expect(preserveQuoteStyle('hello', 'hello', 'world')).toBe('world') - }) - - test('converts straight double quotes to curly in replacement', () => { - const oldString = '"hello"' - const actualOldString = `${LEFT_DOUBLE_CURLY_QUOTE}hello${RIGHT_DOUBLE_CURLY_QUOTE}` - const newString = '"world"' - const result = preserveQuoteStyle(oldString, actualOldString, newString) - expect(result).toContain(LEFT_DOUBLE_CURLY_QUOTE) - expect(result).toContain(RIGHT_DOUBLE_CURLY_QUOTE) - }) - - test('converts straight single quotes to curly in replacement', () => { - const oldString = "'hello'" - const actualOldString = `${LEFT_SINGLE_CURLY_QUOTE}hello${RIGHT_SINGLE_CURLY_QUOTE}` - const newString = "'world'" - const result = preserveQuoteStyle(oldString, actualOldString, newString) - expect(result).toContain(LEFT_SINGLE_CURLY_QUOTE) - expect(result).toContain(RIGHT_SINGLE_CURLY_QUOTE) - }) - - test('treats apostrophe in contraction as right curly quote', () => { - const oldString = "'it's a test'" - const actualOldString = `${LEFT_SINGLE_CURLY_QUOTE}it${RIGHT_SINGLE_CURLY_QUOTE}s a test${RIGHT_SINGLE_CURLY_QUOTE}` - const newString = "'don't worry'" - const result = preserveQuoteStyle(oldString, actualOldString, newString) - // The leading ' at position 0 should be LEFT_SINGLE_CURLY_QUOTE - expect(result[0]).toBe(LEFT_SINGLE_CURLY_QUOTE) - // The apostrophe in "don't" (between n and t) should be RIGHT_SINGLE_CURLY_QUOTE - expect(result).toContain(RIGHT_SINGLE_CURLY_QUOTE) - }) }) // ─── applyEditToFile ──────────────────────────────────────────────────── diff --git a/packages/builtin-tools/src/tools/FileEditTool/utils.ts b/packages/builtin-tools/src/tools/FileEditTool/utils.ts index 65ab58b05..6f9c37787 100644 --- a/packages/builtin-tools/src/tools/FileEditTool/utils.ts +++ b/packages/builtin-tools/src/tools/FileEditTool/utils.ts @@ -15,27 +15,6 @@ import { } from 'src/utils/file.js' import type { EditInput, FileEdit } from './types.js' -// Claude can't output curly quotes, so we define them as constants here for Claude to use -// in the code. We do this because we normalize curly quotes to straight quotes -// when applying edits. -export const LEFT_SINGLE_CURLY_QUOTE = '‘' -export const RIGHT_SINGLE_CURLY_QUOTE = '’' -export const LEFT_DOUBLE_CURLY_QUOTE = '“' -export const RIGHT_DOUBLE_CURLY_QUOTE = '”' - -/** - * Normalizes quotes in a string by converting curly quotes to straight quotes - * @param str The string to normalize - * @returns The string with all curly quotes replaced by straight quotes - */ -export function normalizeQuotes(str: string): string { - return str - .replaceAll(LEFT_SINGLE_CURLY_QUOTE, "'") - .replaceAll(RIGHT_SINGLE_CURLY_QUOTE, "'") - .replaceAll(LEFT_DOUBLE_CURLY_QUOTE, '"') - .replaceAll(RIGHT_DOUBLE_CURLY_QUOTE, '"') -} - /** * Strips trailing whitespace from each line in a string while preserving line endings * @param str The string to process @@ -64,261 +43,22 @@ export function stripTrailingWhitespace(str: string): string { } /** - * Normalizes whitespace for fuzzy matching by converting tabs to spaces - * and collapsing leading whitespace on each line to a canonical form. - * This handles the case where Read tool output renders tabs as spaces, - * so users copy spaces from the output but the file actually has tabs. - */ -function normalizeWhitespace(str: string): string { - return str.replace(/\t/g, ' ') -} - -/** - * Finds the actual string in the file content that matches the search string, - * accounting for quote normalization and tab/space differences. - * - * Matching cascade: - * 1. Exact match - * 2. Quote normalization (curly → straight quotes) - * 3. Tab/space normalization (tabs ↔ spaces in leading whitespace) - * 4. Quote + tab/space normalization combined + * Finds the exact string in the file content. * * @param fileContent The file content to search in * @param searchString The string to search for - * @returns The actual string found in the file, or null if not found + * @returns The search string if found, or null if not found */ export function findActualString( fileContent: string, searchString: string, ): string | null { - // First try exact match if (fileContent.includes(searchString)) { return searchString } - - // Try with normalized quotes - const normalizedSearch = normalizeQuotes(searchString) - const normalizedFile = normalizeQuotes(fileContent) - - const searchIndex = normalizedFile.indexOf(normalizedSearch) - if (searchIndex !== -1) { - // Find the actual string in the file that matches - return fileContent.substring(searchIndex, searchIndex + searchString.length) - } - - // Try with tab/space normalization — handles the case where Read output - // renders tabs as spaces and the user copies the rendered version - const wsNormalizedFile = normalizeWhitespace(fileContent) - const wsNormalizedSearch = normalizeWhitespace(searchString) - - const wsSearchIndex = wsNormalizedFile.indexOf(wsNormalizedSearch) - if (wsSearchIndex !== -1) { - // Map the match position back to the original file content. - // We need to find the corresponding range in the original string. - return mapNormalizedMatchBackToFile( - fileContent, - wsNormalizedFile, - wsSearchIndex, - wsNormalizedSearch.length, - ) - } - - // Try combined: quote normalization + tab/space normalization - const combinedFile = normalizeWhitespace(normalizedFile) - const combinedSearch = normalizeWhitespace(normalizedSearch) - - const combinedIndex = combinedFile.indexOf(combinedSearch) - if (combinedIndex !== -1) { - return mapNormalizedMatchBackToFile( - fileContent, - combinedFile, - combinedIndex, - combinedSearch.length, - ) - } - return null } -/** - * Given a match found in a normalized version of fileContent, map the match - * position back to the original fileContent and extract the corresponding - * substring. - * - * Strategy: walk through both strings character by character, building a - * mapping from normalized offset to original offset. When a tab is expanded - * to 4 spaces in the normalized version, the normalized offset advances by 4 - * while the original offset advances by 1. - */ -function mapNormalizedMatchBackToFile( - fileContent: string, - normalizedFile: string, - normalizedStart: number, - normalizedLength: number, -): string { - // Build a sparse mapping from normalized position → original position. - // We only need to map the range [normalizedStart, normalizedStart + normalizedLength]. - let normPos = 0 - let origPos = 0 - let origStart = -1 - let origEnd = -1 - - while ( - origPos < fileContent.length && - normPos <= normalizedStart + normalizedLength - ) { - if (normPos === normalizedStart) { - origStart = origPos - } - if (normPos === normalizedStart + normalizedLength) { - origEnd = origPos - break - } - - const origChar = fileContent[origPos]! - if (origChar === '\t') { - // Tab expands to 4 spaces in normalized version - const nextNormPos = normPos + 4 - // If normalizedStart falls within this expanded tab, snap to origPos - if ( - normPos < normalizedStart && - nextNormPos > normalizedStart && - origStart === -1 - ) { - origStart = origPos - } - if ( - normPos < normalizedStart + normalizedLength && - nextNormPos > normalizedStart + normalizedLength && - origEnd === -1 - ) { - origEnd = origPos + 1 - } - normPos = nextNormPos - origPos++ - } else { - normPos++ - origPos++ - } - } - - // Fallback: if we couldn't map precisely, use character-count heuristic - if (origStart === -1) origStart = 0 - if (origEnd === -1) { - // Approximate: use the ratio of original to normalized length - const ratio = fileContent.length / normalizedFile.length - origEnd = Math.round(origStart + normalizedLength * ratio) - } - - return fileContent.substring(origStart, origEnd) -} - -/** - * When old_string matched via quote normalization (curly quotes in file, - * straight quotes from model), apply the same curly quote style to new_string - * so the edit preserves the file's typography. - * - * Uses a simple open/close heuristic: a quote character preceded by whitespace, - * start of string, or opening punctuation is treated as an opening quote; - * otherwise it's a closing quote. - */ -export function preserveQuoteStyle( - oldString: string, - actualOldString: string, - newString: string, -): string { - // If they're the same, no normalization happened - if (oldString === actualOldString) { - return newString - } - - // Detect which curly quote types were in the file - const hasDoubleQuotes = - actualOldString.includes(LEFT_DOUBLE_CURLY_QUOTE) || - actualOldString.includes(RIGHT_DOUBLE_CURLY_QUOTE) - const hasSingleQuotes = - actualOldString.includes(LEFT_SINGLE_CURLY_QUOTE) || - actualOldString.includes(RIGHT_SINGLE_CURLY_QUOTE) - - if (!hasDoubleQuotes && !hasSingleQuotes) { - return newString - } - - let result = newString - - if (hasDoubleQuotes) { - result = applyCurlyDoubleQuotes(result) - } - if (hasSingleQuotes) { - result = applyCurlySingleQuotes(result) - } - - return result -} - -function isOpeningContext(chars: string[], index: number): boolean { - if (index === 0) { - return true - } - const prev = chars[index - 1] - return ( - prev === ' ' || - prev === '\t' || - prev === '\n' || - prev === '\r' || - prev === '(' || - prev === '[' || - prev === '{' || - prev === '\u2014' || // em dash - prev === '\u2013' // en dash - ) -} - -function applyCurlyDoubleQuotes(str: string): string { - const chars = [...str] - const result: string[] = [] - for (let i = 0; i < chars.length; i++) { - if (chars[i] === '"') { - result.push( - isOpeningContext(chars, i) - ? LEFT_DOUBLE_CURLY_QUOTE - : RIGHT_DOUBLE_CURLY_QUOTE, - ) - } else { - result.push(chars[i]!) - } - } - return result.join('') -} - -function applyCurlySingleQuotes(str: string): string { - const chars = [...str] - const result: string[] = [] - for (let i = 0; i < chars.length; i++) { - if (chars[i] === "'") { - // Don't convert apostrophes in contractions (e.g., "don't", "it's") - // An apostrophe between two letters is a contraction, not a quote - const prev = i > 0 ? chars[i - 1] : undefined - const next = i < chars.length - 1 ? chars[i + 1] : undefined - const prevIsLetter = prev !== undefined && /\p{L}/u.test(prev) - const nextIsLetter = next !== undefined && /\p{L}/u.test(next) - if (prevIsLetter && nextIsLetter) { - // Apostrophe in a contraction — use right single curly quote - result.push(RIGHT_SINGLE_CURLY_QUOTE) - } else { - result.push( - isOpeningContext(chars, i) - ? LEFT_SINGLE_CURLY_QUOTE - : RIGHT_SINGLE_CURLY_QUOTE, - ) - } - } else { - result.push(chars[i]!) - } - } - return result.join('') -} - /** * Transform edits to ensure replace_all always has a boolean value * @param edits Array of edits with optional replace_all diff --git a/src/components/FileEditToolDiff.tsx b/src/components/FileEditToolDiff.tsx index 71acf4338..f8ca04a0f 100644 --- a/src/components/FileEditToolDiff.tsx +++ b/src/components/FileEditToolDiff.tsx @@ -4,7 +4,7 @@ import { Suspense, use, useState } from 'react'; import { useTerminalSize } from '../hooks/useTerminalSize.js'; import { Box, Text } from '@anthropic/ink'; import type { FileEdit } from '@claude-code-best/builtin-tools/tools/FileEditTool/types.js'; -import { findActualString, preserveQuoteStyle } from '@claude-code-best/builtin-tools/tools/FileEditTool/utils.js'; +import { findActualString } from '@claude-code-best/builtin-tools/tools/FileEditTool/utils.js'; import { adjustHunkLineNumbers, CONTEXT_LINES, getPatchForDisplay } from '../utils/diff.js'; import { logError } from '../utils/log.js'; import { CHUNK_SIZE, openForScan, readCapped, scanForContext } from '../utils/readEditContext.js'; @@ -135,6 +135,5 @@ function diffToolInputsOnly(filePath: string, edits: FileEdit[]): DiffData { function normalizeEdit(fileContent: string, edit: FileEdit): FileEdit { const actualOld = findActualString(fileContent, edit.old_string) || edit.old_string; - const actualNew = preserveQuoteStyle(edit.old_string, actualOld, edit.new_string); - return { ...edit, old_string: actualOld, new_string: actualNew }; + return { ...edit, old_string: actualOld }; }