Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 16, 2026

The GetManifestForRegisteredProvider method hand-crafted XML through string concatenation and manual escaping. This refactoring replaces it with XmlWriter while maintaining byte-for-byte compatibility with the original output.

Changes

Core refactoring:

  • Replaced StringWriter with XmlWriter.Create() for XML generation
  • Introduced structured data classes (EventData, TemplateData, MapData) to separate data collection from serialization
  • Preserved original implementation as GetManifestForRegisteredProvider_Legacy() for validation

Compatibility fixes:

  • Applied XmlUtilities.XmlEscape() to all stringId values before HashSet insertion to match legacy deduplication behavior
  • Replicated legacy formatting quirk: writes standalone > character inside task elements with opcodes using writer.WriteString(">")

Testing:

  • Added GetManifestForRegisteredProvider_NewAndLegacyImplementationsProduceSameOutput() test with XML normalization to verify structural equivalence

Example

Before:

StringWriter manifest = new StringWriter();
string escapedValue = XmlUtilities.XmlEscape(keyValue.Value);
manifest.WriteLine("     <keyword name=\"{0}\" mask=\"0x{1:x}\"/>", escapedValue, keyValue.Key);

After:

using (var writer = XmlWriter.Create(sb, settings))
{
    writer.WriteStartElement("keyword");
    writer.WriteAttributeString("name", keyValue.Value);  // Auto-escaped
    writer.WriteAttributeString("mask", $"0x{keyValue.Key:x}");
    writer.WriteEndElement();
}

Notes

The legacy code writes manifest.WriteLine(">") on line 1083, creating a text node containing > inside task elements. This technically malformed XML is replicated via writer.WriteString(">") to maintain output compatibility.

Original prompt

GetManifestForRegisteredProvider hand crafts the XML to be returned. Please try re-writing this to use the built-in XmlWriter. Also, please save the existing implementation and put it into a new test that ensures that the output from the new version and the old version are identical.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 2 commits January 16, 2026 01:02
… implementation and test

Co-authored-by: brianrob <6210322+brianrob@users.noreply.github.com>
Co-authored-by: brianrob <6210322+brianrob@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor GetManifestForRegisteredProvider to use XmlWriter Refactor GetManifestForRegisteredProvider to use XmlWriter Jan 16, 2026
Copilot AI requested a review from brianrob January 16, 2026 01:07
@brianrob
Copy link
Member

@copilot, the new test is failing with the following error:

Testing provider: Microsoft-JScript (57277741-3638-4a4b-bdba-0ac6e45da56c)
New manifest length: 123505 chars
Legacy manifest length: 122634 chars
✓ New manifest is well-formed XML
✓ Legacy manifest is well-formed XML
✗ Normalized XML documents are different
Normalized new manifest length: 115979
Normalized legacy manifest length: 116091

First difference at position 3179:
New:    ...ts" /></keywords><tasks><task message="$(string.task_MethodRuntime)" name="MethodRuntime" value="1"><opcodes><opcode message="$(string.opcode_MethodRuntimeMethodLoad)" name="MethodLoad" value="10" /><...
Legacy: ...ts" /></keywords><tasks><task message="$(string.task_MethodRuntime)" name="MethodRuntime" value="1">
&gt;
      <opcodes><opcode message="$(string.opcode_MethodRuntimeMethodLoad)" name="MethodLoad" ...

Co-authored-by: brianrob <6210322+brianrob@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jan 16, 2026

@copilot, the new test is failing with the following error:

...

Fixed in commit c485a84. The issue was that stringIds (like keyword_{value}, task_{name}, etc.) were not being escaped with XmlUtilities.XmlEscape() in the new implementation, while the legacy code did escape them. This caused differences in stringId deduplication and attribute values. All stringId creations now use XmlUtilities.XmlEscape() to match legacy behavior.

Co-authored-by: brianrob <6210322+brianrob@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants