From 5d1ef847702ae1493e795129c1c5e8a539d4ff19 Mon Sep 17 00:00:00 2001
From: kske <kai@kske.dev>
Date: Wed, 22 Dec 2021 16:51:33 +0200
Subject: [PATCH] Discard identity changes, discard divergent branches

---
 .../java/dev/kske/undoredo/core/Change.java   |  6 ++--
 .../dev/kske/undoredo/core/ChangeManager.java |  6 ++--
 .../undoredo/core/UnlimitedChangeManager.java | 23 ++++++++++++---
 .../kske/undoredo/core/ChangeManagerTest.java | 28 +++++++++++++++++++
 .../undoredo/javafx/ChangeManagerWrapper.java |  9 ++++--
 5 files changed, 61 insertions(+), 11 deletions(-)

diff --git a/core/src/main/java/dev/kske/undoredo/core/Change.java b/core/src/main/java/dev/kske/undoredo/core/Change.java
index 4252e03..b088c7a 100644
--- a/core/src/main/java/dev/kske/undoredo/core/Change.java
+++ b/core/src/main/java/dev/kske/undoredo/core/Change.java
@@ -2,7 +2,7 @@ package dev.kske.undoredo.core;
 
 /**
  * Base interface for changes to be registered in an undo manager.
- * 
+ *
  * @author Maximilian K&auml;fer
  * @since 0.0.1
  */
@@ -17,7 +17,7 @@ public interface Change {
 
 	/**
 	 * Inverts this change.
-	 * 
+	 *
 	 * @implSpec This method is not supposed to alter the state of this change, but rather to create
 	 *           a new complementary change.
 	 * @return the inverted change
@@ -26,6 +26,8 @@ public interface Change {
 	Change invert();
 
 	/**
+	 * @apiNote If this method returns {@code true} when adding this change to a change manager, it
+	 *          will be discarded immediately and therefore can be garbage collected.
 	 * @return whether the application of this change would result in an identical state
 	 * @since 0.0.1
 	 */
diff --git a/core/src/main/java/dev/kske/undoredo/core/ChangeManager.java b/core/src/main/java/dev/kske/undoredo/core/ChangeManager.java
index 5410cfc..8b6c6ad 100644
--- a/core/src/main/java/dev/kske/undoredo/core/ChangeManager.java
+++ b/core/src/main/java/dev/kske/undoredo/core/ChangeManager.java
@@ -19,12 +19,14 @@ import java.util.*;
 public interface ChangeManager<C extends Change> {
 
 	/**
-	 * Applies the given change and appends it to the change list.
+	 * Applies the given change and appends it to the change list. If the change is an identity, no
+	 * action is taken.
 	 *
 	 * @param change the change to add
+	 * @return whether the change has been added
 	 * @since 0.0.1
 	 */
-	void addChange(C change);
+	boolean addChange(C change);
 
 	/**
 	 * @return the change that was applied last
diff --git a/core/src/main/java/dev/kske/undoredo/core/UnlimitedChangeManager.java b/core/src/main/java/dev/kske/undoredo/core/UnlimitedChangeManager.java
index 4b0d13b..d3da43e 100644
--- a/core/src/main/java/dev/kske/undoredo/core/UnlimitedChangeManager.java
+++ b/core/src/main/java/dev/kske/undoredo/core/UnlimitedChangeManager.java
@@ -3,6 +3,8 @@ package dev.kske.undoredo.core;
 import java.util.*;
 
 /**
+ * A simple change manager with a linear history model backed by an array list.
+ *
  * @param <C> the change type to store in this change manager
  * @author Maximilian K&auml;fer
  * @author Kai S. K. Engelbart
@@ -15,11 +17,24 @@ public final class UnlimitedChangeManager<C extends Change> implements ChangeMan
 	private int	index		= -1;
 	private int	markedIndex	= -1;
 
+	/**
+	 * @implNode As this change manager uses a linear history model, all changes behind the last
+	 *           applied change will be discarded and therefore can be garbage collected.
+	 */
 	@Override
-	public void addChange(C change) {
-		change.apply();
-		changes.add(change);
-		++index;
+	public boolean addChange(C change) {
+		if (!change.isIdentity()) {
+			change.apply();
+
+			// Remove divergent changes
+			for (int i = index + 1; i < changes.size(); ++i)
+				changes.remove(i);
+
+			changes.add(change);
+			++index;
+			return true;
+		}
+		return false;
 	}
 
 	@Override
diff --git a/core/src/test/java/dev/kske/undoredo/core/ChangeManagerTest.java b/core/src/test/java/dev/kske/undoredo/core/ChangeManagerTest.java
index 51ac613..21ab08d 100644
--- a/core/src/test/java/dev/kske/undoredo/core/ChangeManagerTest.java
+++ b/core/src/test/java/dev/kske/undoredo/core/ChangeManagerTest.java
@@ -30,7 +30,20 @@ class ChangeManagerTest {
 	@Order(10)
 	void testAddChange() {
 		assertSame(0, wrapper.value);
+		assertTrue(manager.addChange(change));
+		assertSame(1, wrapper.value);
+	}
+
+	/**
+	 * Tests whether identity changes are ignored when adding them to the change manager.
+	 *
+	 * @since 0.0.1
+	 */
+	@Test
+	@Order(15)
+	void testDiscardIdentity() {
 		manager.addChange(change);
+		assertFalse(manager.addChange(new IntChange(wrapper, 0)));
 		assertSame(1, wrapper.value);
 	}
 
@@ -100,6 +113,21 @@ class ChangeManagerTest {
 		assertEquals(change, manager.getLastChange().get());
 	}
 
+	/**
+	 * Tests whether the changes after the current index are discarded when adding a change.
+	 *
+	 * @since 0.0.1
+	 */
+	@Test
+	@Order(55)
+	void testDiscardDivergentHistory() {
+		manager.addChange(change);
+		manager.undo();
+		assertTrue(manager.addChange(new IntChange(wrapper, 2)));
+		assertFalse(manager.isRedoAvailable());
+		assertSame(2, wrapper.value);
+	}
+
 	/**
 	 * Tests marking a change.
 	 *
diff --git a/javafx/src/main/java/dev/kske/undoredo/javafx/ChangeManagerWrapper.java b/javafx/src/main/java/dev/kske/undoredo/javafx/ChangeManagerWrapper.java
index 0c16cb6..b059104 100644
--- a/javafx/src/main/java/dev/kske/undoredo/javafx/ChangeManagerWrapper.java
+++ b/javafx/src/main/java/dev/kske/undoredo/javafx/ChangeManagerWrapper.java
@@ -50,9 +50,12 @@ public class ChangeManagerWrapper<C extends Change, M extends ChangeManager<C>>
 	}
 
 	@Override
-	public void addChange(C change) {
-		manager.addChange(change);
-		updateProperties();
+	public boolean addChange(C change) {
+		if (manager.addChange(change)) {
+			updateProperties();
+			return true;
+		}
+		return false;
 	}
 
 	@Override