Skip to content

Commit 66f47b7

Browse files
committed
refactor: eliminate code duplication between parseXml and parseXmlForDiff
- Refactored parseXml to accept optional ParseXmlOptions parameter - parseXmlForDiff now delegates to parseXml with processEntities: false - Added explanatory comment in multiApplyDiffTool.ts about why parseXmlForDiff is used - Improved JSDoc documentation with specific use cases for parseXmlForDiff This maintains backward compatibility while eliminating code duplication. parseXml continues to be used for general XML parsing (file reads, follow-up questions), while parseXmlForDiff is specifically for diff operations where entity processing must be disabled.
1 parent 3b5167b commit 66f47b7

File tree

2 files changed

+29
-19
lines changed

2 files changed

+29
-19
lines changed

src/core/tools/multiApplyDiffTool.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,9 @@ export async function applyDiffTool(
108108
if (argsXmlTag) {
109109
// Parse file entries from XML (new way)
110110
try {
111+
// IMPORTANT: We use parseXmlForDiff here instead of parseXml to prevent HTML entity decoding
112+
// This ensures exact character matching when comparing parsed content against original file content
113+
// Without this, special characters like & would be decoded to & causing diff mismatches
111114
const parsed = parseXmlForDiff(argsXmlTag, ["file.diff.content"]) as ParsedXmlResult
112115
const files = Array.isArray(parsed.file) ? parsed.file : [parsed.file].filter(Boolean)
113116

src/utils/xml.ts

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,36 @@
11
import { XMLParser } from "fast-xml-parser"
22

3+
/**
4+
* Options for XML parsing
5+
*/
6+
interface ParseXmlOptions {
7+
/**
8+
* Whether to process HTML entities (e.g., & to &).
9+
* Default: true for general parsing, false for diff operations
10+
*/
11+
processEntities?: boolean
12+
}
13+
314
/**
415
* Parses an XML string into a JavaScript object
516
* @param xmlString The XML string to parse
17+
* @param stopNodes Optional array of node names to stop parsing at
18+
* @param options Optional parsing options
619
* @returns Parsed JavaScript object representation of the XML
720
* @throws Error if the XML is invalid or parsing fails
821
*/
9-
export function parseXml(xmlString: string, stopNodes?: string[]): unknown {
22+
export function parseXml(xmlString: string, stopNodes?: string[], options?: ParseXmlOptions): unknown {
1023
const _stopNodes = stopNodes ?? []
24+
const processEntities = options?.processEntities ?? true
25+
1126
try {
1227
const parser = new XMLParser({
1328
ignoreAttributes: false,
1429
attributeNamePrefix: "@_",
1530
parseAttributeValue: false,
1631
parseTagValue: false,
1732
trimValues: true,
33+
processEntities,
1834
stopNodes: _stopNodes,
1935
})
2036

@@ -30,27 +46,18 @@ export function parseXml(xmlString: string, stopNodes?: string[]): unknown {
3046
* Parses an XML string for diffing purposes, ensuring no HTML entities are decoded.
3147
* This is a specialized version of parseXml to be used exclusively by diffing tools
3248
* to prevent mismatches caused by entity processing.
49+
*
50+
* Use this instead of parseXml when:
51+
* - Comparing parsed content against original file content
52+
* - Performing diff operations where exact character matching is required
53+
* - Processing XML that will be used in search/replace operations
54+
*
3355
* @param xmlString The XML string to parse
56+
* @param stopNodes Optional array of node names to stop parsing at
3457
* @returns Parsed JavaScript object representation of the XML
3558
* @throws Error if the XML is invalid or parsing fails
3659
*/
3760
export function parseXmlForDiff(xmlString: string, stopNodes?: string[]): unknown {
38-
const _stopNodes = stopNodes ?? []
39-
try {
40-
const parser = new XMLParser({
41-
ignoreAttributes: false,
42-
attributeNamePrefix: "@_",
43-
parseAttributeValue: false,
44-
parseTagValue: false,
45-
trimValues: true,
46-
processEntities: false, // Do not process HTML entities, keep them as is
47-
stopNodes: _stopNodes,
48-
})
49-
50-
return parser.parse(xmlString)
51-
} catch (error) {
52-
// Enhance error message for better debugging
53-
const errorMessage = error instanceof Error ? error.message : "Unknown error"
54-
throw new Error(`Failed to parse XML: ${errorMessage}`)
55-
}
61+
// Delegate to parseXml with processEntities disabled
62+
return parseXml(xmlString, stopNodes, { processEntities: false })
5663
}

0 commit comments

Comments
 (0)