Fix XML Beautify indentation after xmlns self-closing element#2608
Open
alleria173 wants to merge 1 commit into
Open
Fix XML Beautify indentation after xmlns self-closing element#2608alleria173 wants to merge 1 commit into
alleria173 wants to merge 1 commit into
Conversation
vkbeautify splits `<b xmlns="foo" />` into two fragments during preprocessing. The `<b` fragment incorrectly incremented the indent depth, but the corresponding decrement never fired because `/>` was on the xmlns fragment, not a standalone self-closing token. Subsequent siblings were therefore indented one level too deep. Fix by moving the xmlns branch before the generic `/>` branch in the if-else chain and decrementing depth when the xmlns fragment ends with `/>`. The vkbeautify xml and createShiftArr functions are inlined into XMLBeautify.mjs to avoid patching node_modules directly. Fixes gchq#2501 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The XML Beautify operation produces incorrect indentation for any element that follows a self-closing tag with an
xmlnsattribute (e.g.<b xmlns="foo" />).Root cause: vkbeautify's preprocessor splits
<b xmlns="foo" />into two fragments —<bandxmlns="foo" />— by inserting a delimiter before thexmlns=token. The<bfragment matches the opening-element rule and increments the indentation depth, but the corresponding decrement never fires because the closing/>is on thexmlnsfragment rather than a standalone<elm/>token. Every sibling after the self-closing element is therefore indented one level too deep.The fix moves the
xmlnsbranch before the generic/>branch in the if-else chain, and decrements depth when the xmlns fragment ends with/>(signalling a self-closing element). This restores the correct depth for all subsequent siblings.Because modifying
node_modules/vkbeautify/index.jsdirectly would be overwritten bynpm install, the relevant functions have been inlined intoXMLBeautify.mjswith the fix applied and thevkbeautifyimport removed.createShiftArradds 31 lines (lines 9–39) and the xml formatterxmlBeautifyadds 69 lines (lines 41–109). Both carry a JSDoc attribution to the original library:Adapted from vkBeautify (c) 2012 Vadim Kiryukhin, MIT/GPL dual licence.Existing Issue
Fixes #2501 #2501
Screenshots
N/A — no visual changes.
AI disclosure
Claude Code (claude.ai/code) was used to assist in diagnosing the root cause, implementing the fix, and generating test cases. All code has been reviewed and understood by the submitter.
Test Coverage
A new test file
tests/operations/tests/XMLBeautify.mjshas been added with 8 test cases covering baseline behaviour, the two xmlns regression scenarios from the issue, xmlns on non-self-closing tags, multiple xmlns attributes on a single self-closing element, correct indentation of siblings after the fix, XML declarations, and configurable indent strings. All 2064 operation tests pass (npm test).