Add delete changes at end of change sequence in ChangeRecorder
#54
Conversation
This file contains 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
added 3 commits
December 27, 2022 13:24
…m right after the element is removed from its parent
TomWerm
approved these changes
Jan 5, 2023
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.
This PR changes how delete changes are added to the change sequence recorded by the
ChangeRecorder.After recording finishes, the change recorder adds delete changes for every element implicitly deleted in the change sequence (i.e. for elements removed from their containment but not reinserted somewhere again). When inserting a delete change into the change sequence, any change after the delete change must not reference the deleted object anymore (as in theory the deleted object does not exist anymore at this point).
In the current behavior, the recorder adds the delete changes directly after the element is removed from its containment. However, this positioning does not guarantee that the element is never again referenced in changes after its delete change. As an example, a non-containment reference to the affected element might be unset after the element is removed from containment. While this would be easy to detect, since we also remove contained elements (see #48) such reference-after-delete issues might also occur more disguised when not the affected element itself but one of its descendants is referenced.
As such, this PR changes the behavior of the change recorder to append delete changes at the end of a change sequence. While identifying the earliest position in the sequence to insert the delete change would be possible, I opted for this easier solution simply as I didn't see any benefit of having the delete change as early as possible.
Additionally, this PR fixes the rare problem that an element and one of its descendants are both marked for deletion (when they are both removed from their containment), resulting in a duplicate delete change for the descendant (once from itself, once from its ancestor).