fix: rename GUID-suffixed scripts when name conflicts are resolved#52
fix: rename GUID-suffixed scripts when name conflicts are resolved#52Biraru wants to merge 1 commit into
Conversation
|
Have you tested if this causes any weird regressions regarding #14? |
|
Yes, i just did and |
|
Pretty sure #14 would need to be resolved first to make sure this doesn't cause any bugs. |
|
@coderabbitai review |
📝 WalkthroughSummary by CodeRabbit
WalkthroughSyncDaemon.handleDeleted in src/index.ts now performs an additional step after pruning: when the deleted node had a parentGuid, it searches the parent's descendant scripts for same-named sibling nodes, rewrites the last matching sibling's script file (suppressing the watcher), and upserts that sibling into the sourcemap using its previous path. ChangesSibling Rename on Deletion
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Watcher
participant SyncDaemon
participant FileSystem
participant SourceMap
Watcher->>SyncDaemon: handleDeleted(node)
SyncDaemon->>SourceMap: prune subtree
SyncDaemon->>SourceMap: find same-name sibling under parentGuid
alt sibling found
SyncDaemon->>Watcher: suppress next change event
SyncDaemon->>FileSystem: rewrite sibling script file
SyncDaemon->>SourceMap: upsert sibling subtree with prevPath
end
Related issues: Suggested labels: bug, enhancement Suggested reviewers: none identified 🐰 A script named twice, one gone, one stayed, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/index.ts`:
- Around line 312-314: The restore logic in the node handling path is matching
too broadly because getDescendantScripts(node.parentGuid) returns the full
subtree, not just direct siblings. In the block that builds sameNameScriptNodes,
update the filtering around node.parentGuid so only scripts with
sibling.parentGuid === node.parentGuid and the same name are considered, using
the existing tree lookup in this section to keep the restore restricted to true
siblings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| if (node.parentGuid) { | ||
| const siblingScriptNodes = this.tree.getDescendantScripts(node.parentGuid); | ||
| const sameNameScriptNodes = siblingScriptNodes.filter(sibling => sibling.name === node.name); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm getDescendantScripts recurses into grandchildren (not just direct children)
# and check TreeNode's parentGuid/name fields.
fd -e ts . src | xargs rg -n -A15 'getDescendantScripts' --type=ts
rg -n -B2 -A10 'interface TreeNode' --type=tsRepository: Ransomwave/azul
Length of output: 3539
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant implementations and types with line numbers.
sed -n '240,290p' src/fs/treeManager.ts
printf '\n----\n'
sed -n '300,345p' src/index.ts
printf '\n----\n'
rg -n -A20 -B5 'class FileWriter|getFilePath\\(|writeScript\\(|deleteScript\\(' src -t ts
printf '\n----\n'
rg -n -A12 -B4 'interface TreeNode|type TreeNode|class TreeNode' src -t tsRepository: Ransomwave/azul
Length of output: 3149
Restrict this restore to direct siblings. getDescendantScripts(node.parentGuid) walks the full subtree, so sameNameScriptNodes can match a nested descendant under a child folder instead of a true sibling. Add a sibling.parentGuid === node.parentGuid check here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/index.ts` around lines 312 - 314, The restore logic in the node handling
path is matching too broadly because getDescendantScripts(node.parentGuid)
returns the full subtree, not just direct siblings. In the block that builds
sameNameScriptNodes, update the filtering around node.parentGuid so only scripts
with sibling.parentGuid === node.parentGuid and the same name are considered,
using the existing tree lookup in this section to keep the restore restricted to
true siblings.
2 participants
Fixes #51