fixed IllegalArgumentException on undo

[grigoriliev]

I changed the TextChange.mergeWith method so that the undo merging mimics the behavior of a regular text editor. This also fixes #322 (java.lang.IllegalArgumentException: Unexpected change received).

[alt-grr]

I think that also adding test case for

adding a character then removing it and then hit Ctrl+Z. Or by adding several characters then removing those characters and then hit Ctrl+Z

would be nice.

---

Hmm, ok, seems that #373 is still not materialized in code... So I don't know if it is currently possible to write any test for that behavior 😞

[JordanMartinez]

How does your PR fix only the bug without also stopping merges entirely? I don't see how it would do that.

[JordanMartinez]

@kuc One could still write the code that ultimately gets executed:

area.replace(0, 0, "a");
area.replace(0, 1, "");
area.undo();

[grigoriliev]

For undo/redo to work similar to other text editors merges should only occur when there is a sequence of inserting characters or sequence of removing characters. That is what this fix does - it doesn't stop merges entirely, but only allows merging of 'insert only' changes and 'remove only' changes.

[JordanMartinez]

@grigoriliev Thank you for your work and patience. Please do three things:

• update this PR to start from the current master branch (just added TestFX capabilities)
• write the tests that follow
• add your code to the project to insure it fixes the issue.

There are at least 2 things we'll need to test:

• A test that reproduces the current bug we have in Deleting text that was just inserted merges those changes into a change that does nothing, causing an Exception #322.
• A test that insures style changes (e.g. setStyle(from, to, style)) are not negatively affected by your changes

As a starting point for writing a TestFX test, you can copy and paste this code:

import javafx.scene.Scene;
import javafx.stage.Stage;
import org.fxmisc.flowless.VirtualizedScrollPane;
import org.fxmisc.richtext.InlineCssTextArea;
import org.junit.Test;
import org.testfx.framework.junit.ApplicationTest;

public class TestFX_Test extends ApplicationTest {

    private InlineCssTextArea area;
    private VirtualizedScrollPane<InlineCssTextArea> vsPane;

    @Override
    public void start(Stage primaryStage) {
        area = new InlineCssTextArea();
        vsPane = new VirtualizedScrollPane<>(area);

        Scene scene = new Scene(vsPane, 500, 500);
        primaryStage.setScene(scene);
        primaryStage.show();
    }
    
    @Test
    public void testSomething() {
        // note: when directly accessing the area, you must use `interact(Runnable)` to run that code
        //  on the JavaFX Application Thread. Otherwise, it will throw an error.
        
        clickOn(area).write("some text");
        
        interact(() -> area.replaceText("Some text here."));
        
        interact(() -> {
            assert area.getLength() != 0;
        });
    }
}

[JordanMartinez]

After looking at @grigoriliev's solution, I cleaned up his code to make it more readable in #458 and added the test above. I didn't see the need for the setStyle test as I think I may have misunderstood how his solution worked. A TestFX test was also not necessary.

[JordanMartinez]

@grigoriliev We decided to revert back to the original merge method in #496. You can still use your version of the merge method by calling setUndoManager and use the code the area uses to create an UndoManager but change only the merge part of it.