refactor(Scanner): Move mapping creation from mergeable to diffItem#2253
Open
marcelklehr wants to merge 7 commits intodevelopfrom
Open
refactor(Scanner): Move mapping creation from mergeable to diffItem#2253marcelklehr wants to merge 7 commits intodevelopfrom
marcelklehr wants to merge 7 commits intodevelopfrom
Conversation
…similarity fixes #2230 Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
marcelklehr
force-pushed
the
fix/2230/crank-up-children-similarity
branch
from
aa5995f to
1458fd5
Compare
With this change, mappings will only be created if both childrenSimilarity and mergeable give a go. Before, mergeable would sometimes create mappings prematurely. Signed-off-by: Marcel Klehr <mklehr@gmx.net>
marcelklehr
force-pushed
the
fix/2230/crank-up-children-similarity
branch
from
1458fd5 to
6c0df45
Compare
marcelklehr
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Refactors the sync scanning flow so mapping creation happens inside Scanner.diffItem rather than being accumulated in strategy-specific mergeable callbacks, addressing issue #2230.
Changes:
- Extend
Scannerto acceptMappingsand optionally create mappings during scanning. - Remove strategy-level “collect then add mappings” logic in
Default,Merge, andUnidirectionalstrategies. - Add a regression test covering item moves where similarly named folders exist in different branches.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ui/views/native/Home.vue | Template formatting change (unrelated to scanner refactor). |
| src/test/test.js | Adds regression test for move behavior with duplicate folder names. |
| src/lib/strategies/Unidirectional.ts | Routes mapping creation through Scanner (passes mappings, toggles flags). |
| src/lib/strategies/Merge.ts | Removes newMappings collection and relies on Scanner mapping creation. |
| src/lib/strategies/Default.ts | Removes newMappings flow; updates Scanner instantiation flags. |
| src/lib/Scanner.ts | Adds mappings + createMappings option; creates mappings in diffItem; adds debug logging. |
| src/lib/Mappings.ts | Prevents duplicate mappings by removing an existing mapping before adding. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
marcelklehr
force-pushed
the
fix/2230/crank-up-children-similarity
branch
from
4527c27 to
f0ba511
Compare
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.
fixes #2230