From 7169da85b0906fdb5fec49bf9235b26ba3ea094a Mon Sep 17 00:00:00 2001 From: grigoriliev Date: Tue, 6 Dec 2016 09:53:19 +0200 Subject: [PATCH 1/3] fixed IllegalArgumentException on undo --- .../org/fxmisc/richtext/model/TextChange.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/richtextfx/src/main/java/org/fxmisc/richtext/model/TextChange.java b/richtextfx/src/main/java/org/fxmisc/richtext/model/TextChange.java index 04c00ef8f..61a2ae273 100644 --- a/richtextfx/src/main/java/org/fxmisc/richtext/model/TextChange.java +++ b/richtextfx/src/main/java/org/fxmisc/richtext/model/TextChange.java @@ -42,6 +42,22 @@ public TextChange(int position, S removed, S inserted) { * {@code null} otherwise. */ public Optional mergeWith(Self latter) { + if(this.insertedLength() > 0 && this.removedLength() > 0) { + return Optional.empty(); + } + + if(latter.insertedLength() > 0 && latter.removedLength() > 0) { + return Optional.empty(); + } + + if(this.insertedLength() > 0 && latter.removedLength() > 0) { + return Optional.empty(); + } + + if(this.removedLength() > 0 && latter.insertedLength() > 0) { + return Optional.empty(); + } + if(latter.position == this.position + this.insertedLength()) { S removedText = concat(this.removed, latter.removed); S addedText = concat(this.inserted, latter.inserted); From bedaffb7e11aff26098e0cdc7cc9f2a60b0ef40c Mon Sep 17 00:00:00 2001 From: Jordan Martinez Date: Fri, 17 Mar 2017 21:31:58 -0700 Subject: [PATCH 2/3] Make mergeWith code more readable; update javadoc --- .../org/fxmisc/richtext/model/TextChange.java | 78 +++++++++---------- 1 file changed, 38 insertions(+), 40 deletions(-) diff --git a/richtextfx/src/main/java/org/fxmisc/richtext/model/TextChange.java b/richtextfx/src/main/java/org/fxmisc/richtext/model/TextChange.java index 61a2ae273..e7c11d277 100644 --- a/richtextfx/src/main/java/org/fxmisc/richtext/model/TextChange.java +++ b/richtextfx/src/main/java/org/fxmisc/richtext/model/TextChange.java @@ -5,6 +5,33 @@ public abstract class TextChange> { + public static enum ChangeType { + /** Indicates that the change will insert something but not remove anything */ + INSERTION, + /** Indicates that the change will delete something but not insert anything */ + DELETION, + /** Indicates that the change will remove something and insert something as its replacement */ + REPLACEMENT + } + + private ChangeType type; + public final ChangeType getType() { + if (type == null) { + if (insertedLength() == 0) { + if (removedLength() == 0) { + throw new IllegalStateException("Cannot get the type of a change that neither inserts nor deletes anything."); + } else { + type = ChangeType.DELETION; + } + } else if (removedLength() == 0) { + type = ChangeType.INSERTION; + } else { + type = ChangeType.REPLACEMENT; + } + } + return type; + } + protected final int position; protected final S removed; protected final S inserted; @@ -29,51 +56,21 @@ public TextChange(int position, S removed, S inserted) { protected abstract Self create(int position, S removed, S inserted); /** - * Merges this change with the given change, if possible. - * This change is considered to be the former and the given - * change is considered to be the latter. - * Changes can be merged if either - *
    - *
  • the latter's start matches the former's added text end; or
    • - *
  • the latter's removed text end matches the former's added text end.
    • - *
+ * Merges this change with the given change only if the end of this change's inserted text + * equals the latter's position and both are either insertion or deletion changes. + * * @param latter change to merge with this change. * @return a new merged change if changes can be merged, * {@code null} otherwise. */ public Optional mergeWith(Self latter) { - if(this.insertedLength() > 0 && this.removedLength() > 0) { - return Optional.empty(); - } - - if(latter.insertedLength() > 0 && latter.removedLength() > 0) { - return Optional.empty(); - } - - if(this.insertedLength() > 0 && latter.removedLength() > 0) { - return Optional.empty(); - } - - if(this.removedLength() > 0 && latter.insertedLength() > 0) { - return Optional.empty(); - } - - if(latter.position == this.position + this.insertedLength()) { + if(this.getType() != ChangeType.REPLACEMENT + && this.getType() == latter.getType() + && this.getInsertionEnd() == latter.position) { S removedText = concat(this.removed, latter.removed); S addedText = concat(this.inserted, latter.inserted); return Optional.of(create(this.position, removedText, addedText)); - } - else if(latter.position + latter.removedLength() == this.position + this.insertedLength()) { - if(this.position <= latter.position) { - S addedText = concat(sub(this.inserted, 0, latter.position - this.position), latter.inserted); - return Optional.of(create(this.position, this.removed, addedText)); - } - else { - S removedText = concat(sub(latter.removed, 0, this.position - latter.position), this.removed); - return Optional.of(create(latter.position, removedText, latter.inserted)); - } - } - else { + } else { return Optional.empty(); } } @@ -99,9 +96,10 @@ public int hashCode() { public final String toString() { return this.getClass().getSimpleName() + "{\n" + - "\tposition: " + position + "\n" + - "\tremoved: " + removed + "\n" + - "\tinserted: " + inserted + "\n" + + "\tposition: " + position + "\n" + + "\ttype: " + getType() + "\n" + + "\tremoved: " + removed + "\n" + + "\tinserted: " + inserted + "\n" + "}"; } } From b1f0f7bcb10747b034af888855df484067f4efd8 Mon Sep 17 00:00:00 2001 From: Jordan Martinez Date: Fri, 17 Mar 2017 21:37:29 -0700 Subject: [PATCH 3/3] Add test to insure mergeWith does not throw Exception --- .../java/org/fxmisc/richtext/model/AreaTest.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 richtextfx/src/test/java/org/fxmisc/richtext/model/AreaTest.java diff --git a/richtextfx/src/test/java/org/fxmisc/richtext/model/AreaTest.java b/richtextfx/src/test/java/org/fxmisc/richtext/model/AreaTest.java new file mode 100644 index 000000000..84838e90e --- /dev/null +++ b/richtextfx/src/test/java/org/fxmisc/richtext/model/AreaTest.java @@ -0,0 +1,16 @@ +package org.fxmisc.richtext.model; + +import org.fxmisc.richtext.InlineCssTextArea; +import org.junit.Test; + +public class AreaTest { + + private InlineCssTextArea area = new InlineCssTextArea(); + + @Test + public void deletingTextThatWasJustInsertedShouldNotMergeTheTwoChanges() { + area.replaceText(0, 0, "text"); + area.replaceText(0, area.getLength(), ""); + area.undo(); + } +}