Skip to content

Fix for moving nested tables when using iterable_compare_func. #541

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dtorres-sf
Copy link
Contributor

This fixes issue #540 . Before this change the reference params were being swapped right after there was a move. This is because the move needed to have the original paths, but child changes needed the new paths. The problem was that nested moves swapped the reference parameters again after the move was recorded. This made the paths inaccurate since the parent did not have the params swapped but the child did.

Instead, we are no longer swapping when building the tree, but rather when we request the paths. The paths will not be swapped for the iterable_item_moved but it will be swapped for all other changes if there was a parent with an iterable_item_moved.

This fixes issue seperman#540. Before this change the reference params were being swapped right after there was a move. This is because the move needed to have the original paths, but child changes needed the new paths. The problem was that nested moves swapped the reference parameters again after the move was recorded. This made the paths inaccurate since the parent did not have the params swapped but the child did.

Instead, we are no longer swapping when building the tree, but rather when we request the paths. The paths will not be swapped for the iterable_item_moved but it will be swapped for all other changes if there was a parent with an iterable_item_moved.
@dtorres-sf
Copy link
Contributor Author

@seperman Can you take a look at this? This change only affects cases that use iterable_compare_func. Right now, iterable_compare_func is broken if you move an item in an array and in a nested array. It will not have the correct diff results, and the delta object is incorrect which means you cannot replay changes using deltas.

Thanks!!

@dtorres-sf
Copy link
Contributor Author

@seperman Would be great to get your thoughts here! Let me know if we can push this forward. The failing tests are just due to unsupported python versions for the tests.

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.

1 participant