zdm/undo-redo
Archived
3
Fork 0
Code Issues 2 Pull Requests Releases 1 Activity

Correctly Deal with Identity Changes and Divergent History #5

Merged
kske merged 3 commits from b/divergent-history into develop 2021-12-24 22:16:46 +01:00
Conversation 2 Commits 3 Files Changed 8 +120 -13
kske commented 2021-12-22 15:53:38 +01:00
Owner
Copy Link
  • Identity changes are now immediately discarded by the change manager.
  • addChange returns whether the change was actually added or discarded.
  • When "overwriting" the history by adding change after performing an undo, the new change is added at the correct position and the divergent history is discarded.
* Identity changes are now immediately discarded by the change manager. * `addChange` returns whether the change was actually added or discarded. * When "overwriting" the history by adding change after performing an undo, the new change is added at the correct position and the divergent history is discarded.
kske self-assigned this 2021-12-22 15:53:38 +01:00
kske added 2 commits 2021-12-22 15:53:39 +01:00
Fix javafx module artifact name
All checks were successful
zdm/undo-redo/pipeline/head This commit looks good
Details
1155541350
Discard identity changes, discard divergent branches
All checks were successful
zdm/undo-redo/pipeline/head This commit looks good
Details
5d1ef84770
kske requested review from delvh 2021-12-22 15:53:43 +01:00
kske requested review from mpk 2021-12-22 15:53:43 +01:00
delvh started working 2021-12-22 16:52:08 +01:00
delvh reviewed 2021-12-22 17:06:46 +01:00
core/src/main/java/dev/kske/undoredo/core/UnlimitedChangeManager.java
@ -16,2 +18,4 @@
private int markedIndex = -1;
/**
* @implNode As this change manager uses a linear history model, all changes behind the last
delvh commented 2021-12-22 17:06:13 +01:00
Owner

I can definitely see needing this Javadoc more often.
What about implementing a superclass called LinearHistoryChangeManager that automatically supplies the Javadoc and general implementations of functions (undo, redo, and addChange given a new protected abstract function removeDivergentChanges)?

I can definitely see needing this Javadoc more often. What about implementing a superclass called `LinearHistoryChangeManager` that automatically supplies the Javadoc and general implementations of functions (`undo`, `redo`, and `addChange` given a new protected abstract function `removeDivergentChanges`)?
kske commented 2021-12-23 09:00:14 +01:00
Author
Owner

I think we need to create more change manager implementations before deciding what belongs in the superclass and what doesn't. In any case, this branch is probably the wrong place add this.

I think we need to create more change manager implementations before deciding what belongs in the superclass and what doesn't. In any case, this branch is probably the wrong place add this.
👀 1
delvh marked this conversation as resolved
core/src/test/java/dev/kske/undoredo/core/ChangeManagerTest.java
@ -103,0 +124,4 @@
manager.addChange(change);
manager.undo();
assertTrue(manager.addChange(new IntChange(wrapper, 2)));
assertFalse(manager.isRedoAvailable());
delvh commented 2021-12-22 16:59:09 +01:00
Owner

Perhaps you should
a) test by adding two changes that are undone
b) Add an assertSame for the size of changes stored

Perhaps you should a) test by adding two changes that are undone b) Add an assertSame for the size of changes stored
kske commented 2021-12-23 08:38:55 +01:00
Author
Owner

Good point. I actually discovered a bug through this. The fix is coming in the next commit.

Good point. I actually discovered a bug through this. The fix is coming in the next commit.
kske marked this conversation as resolved
delvh worked for 14min 40s 2021-12-22 17:06:48 +01:00
kske added 1 commit 2021-12-23 08:42:38 +01:00
Fix divergent changes removal, improve unit test
All checks were successful
zdm/undo-redo/pipeline/head This commit looks good
Details
d484839c8b
delvh approved these changes 2021-12-23 14:18:42 +01:00
kske merged commit 4a70d954ef into develop 2021-12-24 22:16:46 +01:00
kske deleted branch b/divergent-history 2021-12-24 22:16:46 +01:00
This repo is archived. You cannot comment on pull requests.
Reviewers
No Reviewers
mpk
delvh
Labels
Clear labels
1
1
13
13
2
2
21
21
3
3
34
34
5
5
55
55
8
8
bug
Something isn't working
could have
This doesn't hurt
documentation
Improvements or additions to documentation
duplicate
This issue or pull request already exists
enhancement
New feature or request
help wanted
Extra attention is needed
must have
This is absolutely necessary
question
Further information is requested
should have
This should be implemented
stale
This issue or pull request had no recent activity
wont have
This won't be implemented (for now)
L
Branches where 300 < X <= 600 lines are added and more than one work day is needed
M
Branches where 100 < X <=300 lines are added
S
Branches where X <= 100 lines are added or less than 30 minutes are needed
XL
Branch where X > 600 lines are added and more than two work days are needed
bug
Something is not working
bugfix
Fixes a bug
discussion
This has to be discussed
documentation
Related to documentation
feature
Introduces a new feature
maintenance
Related to project maintenance
postponed
Won't be done in the immediate future
refactoring
Refactoring code or architectures
wontfix
Will not be fixed in this state of development
No Label
Milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees kske
2 Participants
Total Time Spent: 14 minutes 40 seconds
delvh
14 minutes 40 seconds
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: zdm/undo-redo#5
No description provided.
Delete Branch "b/divergent-history"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?