fix: preserve manually added files when dir contains fileTree and file nodes#483
Conversation
…e nodes RemoveNodeFromParent used swap-and-truncate which silently deleted the last sibling of a removed fileTree node. Replace with order-preserving deletion so manually added file nodes are not lost during fileTree expansion. Add regression test covering a dir with both fileTree and a manually added file entry.
gardener-prow
Bot
added
do-not-merge/needs-kind
RemoveNodeFromParent inside a range loop shifted the underlying array while the range header still held the original length, causing elements to be visited twice and triggering a false file-collision error. Collect nodes to remove during the loop and delete them after.
| nodeNameToNode[child.File] = child | ||
| } | ||
| } | ||
| for _, child := range toRemove { |
There was a problem hiding this comment.
Phase 1 – We loop through node.Structure with for _, child := range node.Structure. During this loop, we do not change node.Structure. If we find a node that should be removed, we only do toRemove = append(toRemove, child). toRemove is a different slice, so it does not affect node.Structure. The loop continues safely because node.Structure does not change while we are iterating over it.
Phase 2 – After the first loop is completely finished, we start a second loop: for _, child := range toRemove. Now it is safe to modify node.Structure because we are no longer iterating over it.
The old version of the remove function moved the last element into the removed element's position.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: klocke-io The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
LGTM label has been added. DetailsGit tree hash: 2e03392a6e466b0fa44da0c4c527a530a136f2f3 |
gardener-prow
Bot
added
the
approved
2 participants
Summary
RemoveNodeFromParentused a swap-and-truncate pattern to remove nodes, which silently deleted the last sibling of the removedfileTreenodeappend(s[:i], s[i+1:]...)) so manually addedfile:nodes are not lost duringfileTreeexpansionfileTreeand a manually addedfileentryFixes the bug reported where a manually added file appears at the top level of the TOC instead of inside its parent directory's subnav.
Test plan
go test ./pkg/manifest/...— 6/6 passgo test ./...— all passgen-tocwith a manifest containing a dir with bothfileTree:andfile:— manually added file appears under the correct directory in the TOC