Skip to content

Commit

Permalink
fix issue with sequential atomic edits merging
Browse files Browse the repository at this point in the history 
This was a lot easier to figure out with a test suite! Add a regressin test
that checks that adjacent atomic edits remain separate. Fix it by increasing the
priority of `inAtomicEdit` within `shouldAddToExistingEdit`. Break apart the
large boolean equation into individual statements for greater clarity.
  • Loading branch information
@garfieldnate
garfieldnate committed Aug 13, 2024
1 parent 195bb71 commit 090ecd0
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 48 deletions.
38 changes: 26 additions & 12 deletions src/main/java/edu/umich/soar/visualsoar/ruleeditor/CompoundUndoManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ public class AtomicModeManager implements AutoCloseable {
private AtomicModeManager() {
endCompoundEdit();
inAtomicEdit = true;
startCompoundEdit(null);
}

@Override
Expand Down Expand Up @@ -150,7 +149,7 @@ public void undoableEditHappened(UndoableEditEvent e) {
return;
}

// Not incremental edit, end previous edit and start a new one
// Not incremental edit; end previous edit and start a new one

if (!inAtomicEdit) {
endCompoundEdit();
Expand All @@ -159,16 +158,26 @@ public void undoableEditHappened(UndoableEditEvent e) {
}

private boolean shouldAddToExistingEdit(UndoableEdit ue) {
return
// we have an existing edit to add to
compoundEdit != null
&&
// start new edit after document save
!lastActionWasSave.get()
&&
// start new edit if user switches between insertion/deletion
!editingSwitchedBetweenInsertAndDelete()
&& (isStyleChange(ue) || inAtomicEdit || isIncrementalEditOrBackspace());
if (compoundEdit == null) {
// impossible to add to one because none exists
return false;
}
// honor atomic guarantee if requested by client
if (inAtomicEdit) {
return true;
}

// after save, we start a new edit
if (lastActionWasSave.get()) {
return false;
}

// start new edit if user switches between insertion/deletion
if (editingSwitchedBetweenInsertAndDelete()) {
return false;
}

return (isStyleChange(ue) || isIncrementalEditOrBackspace());
}

private void updatePreviousEditInfo() {
Expand Down Expand Up @@ -314,6 +323,11 @@ public void removeUpdate(DocumentEvent e) {
@Override
public void changedUpdate(DocumentEvent e) {}

@Override
public String toString() {
return "Compound" + super.toString();
}

class MyCompoundEdit extends CompoundEdit {
public boolean isInProgress() {
// in order for the canUndo() and canRedo() methods to work
Expand Down
113 changes: 77 additions & 36 deletions src/test/java/edu/umich/soar/visualsoar/ruleeditor/CompoundUndoManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,46 @@ public void sequentialDeletionsAreJoined() throws BadLocationException {
assertEquals("", editorPane.getText(), "Redo should delete entire input string again");
}

@Test
public void adjacentAtomicEditsAreNotJoinedInAtomicModeAreJoined() throws Exception {
JEditorPane editorPane = new JEditorPane();
Document doc = editorPane.getDocument();
CompoundUndoManager undoManager =
new CompoundUndoManager(editorPane, new BooleanProperty(true));

String inputString = "hello there";
doc.insertString(0, inputString, new SimpleAttributeSet());
try (CompoundUndoManager.AtomicModeManager ignored = undoManager.atomicMode()) {
doc.insertString(3, "xxxxxxxxxx", new SimpleAttributeSet());
doc.remove(0, 12);
}
assertEquals(
"xlo there",
editorPane.getText(),
"Confirming added text contents after first atomic edit");
try (CompoundUndoManager.AtomicModeManager ignored = undoManager.atomicMode()) {
doc.insertString(3, "yyyyy", new SimpleAttributeSet());
doc.remove(5, 2);
}
assertEquals(
"xloyyy there",
editorPane.getText(),
"Confirming added text contents after second atomic edit");

undoManager.undo();
assertEquals(
"xlo there", editorPane.getText(), "undo should apply only to edits in second atomic edit");
undoManager.undo();
assertEquals(
inputString, editorPane.getText(), "undo should apply only to edits in first atomic edit");

undoManager.redo();
assertEquals("xlo there", editorPane.getText(), "redo should apply first atomic edit again");
undoManager.redo();
assertEquals(
"xloyyy there", editorPane.getText(), "redo should apply second atomic edit again");
}

@Test
public void allEditsInAtomicModeAreJoined() throws Exception {
JEditorPane editorPane = new JEditorPane();
Expand Down Expand Up @@ -118,46 +158,47 @@ public void sequentialInsertionsAndDeletionsAreSeparated() throws BadLocationExc

@Test
public void startNewEditAfterDocumentSave() throws BadLocationException {
JEditorPane editorPane = new JEditorPane();
BooleanProperty lastActionWasSave = new BooleanProperty(false);
CompoundUndoManager undoManager =
new CompoundUndoManager(editorPane, lastActionWasSave);

editorPane.setText("hello world!");
editorPane.setCaretPosition(5);
insertOneChar(editorPane, 'a');
insertOneChar(editorPane, 'b');
insertOneChar(editorPane, 'c');
lastActionWasSave.set(true);
insertOneChar(editorPane, '1');
assertFalse(lastActionWasSave.get(), "lastActionWasSave should be set to false on any undoable actions");
insertOneChar(editorPane, '2');
insertOneChar(editorPane, '3');
lastActionWasSave.set(true);
insertOneChar(editorPane, 'x');
insertOneChar(editorPane, 'y');
insertOneChar(editorPane, 'z');
lastActionWasSave.set(true);

assertEquals("helloabc123xyz world!", editorPane.getText(), "Confirming fully-entered text");
undoManager.undo();
assertEquals("helloabc123 world!", editorPane.getText(), "First undo");
undoManager.undo();
assertEquals("helloabc world!", editorPane.getText(), "Second undo");
undoManager.undo();
assertEquals("hello world!", editorPane.getText(), "Third undo");

undoManager.redo();
assertEquals("helloabc world!", editorPane.getText(), "Redo third undo");
undoManager.redo();
assertEquals("helloabc123 world!", editorPane.getText(), "Redo second undo");
undoManager.redo();
assertEquals("helloabc123xyz world!", editorPane.getText(), "Redo first undo");
JEditorPane editorPane = new JEditorPane();
BooleanProperty lastActionWasSave = new BooleanProperty(false);
CompoundUndoManager undoManager = new CompoundUndoManager(editorPane, lastActionWasSave);

editorPane.setText("hello world!");
editorPane.setCaretPosition(5);
insertOneChar(editorPane, 'a');
insertOneChar(editorPane, 'b');
insertOneChar(editorPane, 'c');
lastActionWasSave.set(true);
insertOneChar(editorPane, '1');
assertFalse(
lastActionWasSave.get(),
"lastActionWasSave should be set to false on any undoable actions");
insertOneChar(editorPane, '2');
insertOneChar(editorPane, '3');
lastActionWasSave.set(true);
insertOneChar(editorPane, 'x');
insertOneChar(editorPane, 'y');
insertOneChar(editorPane, 'z');
lastActionWasSave.set(true);

assertEquals("helloabc123xyz world!", editorPane.getText(), "Confirming fully-entered text");
undoManager.undo();
assertEquals("helloabc123 world!", editorPane.getText(), "First undo");
undoManager.undo();
assertEquals("helloabc world!", editorPane.getText(), "Second undo");
undoManager.undo();
assertEquals("hello world!", editorPane.getText(), "Third undo");

undoManager.redo();
assertEquals("helloabc world!", editorPane.getText(), "Redo third undo");
undoManager.redo();
assertEquals("helloabc123 world!", editorPane.getText(), "Redo second undo");
undoManager.redo();
assertEquals("helloabc123xyz world!", editorPane.getText(), "Redo first undo");
}

@Test
public void styleChangesAreJoined() {
// TODO: don't know how to add style change events
// TODO: don't know how to add style change events
}

//////////////// Util methods ///////////////////
Expand Down

0 comments on commit 090ecd0

Please sign in to comment.