FXMisc / UndoFX

Undo manager for JavaFX
BSD 2-Clause "Simplified" License
98 stars 17 forks source link

Document that an undone action should be passed back into the undo queue #30

Closed delvh closed 3 years ago

delvh commented 3 years ago

Problem

I just started using UndoFX, and I consistently got an IllegalStateException without knowing why. Through much debugging and by looking at #7, I finally figured out that the parameter of the apply function is supposed to be reinserted into the event stream inside the apply function. This is however nowhere documented.

Responsible Code (UndoManagerImpl)

            C change = changeToApply.get();
            this.expectedChange = change;
            performingAction.suspendWhile(() -> apply.accept(change));
            if(this.expectedChange != null) {
                throw new IllegalStateException("Expected change not received:\n"
                        + this.expectedChange);
            }

Possible remediation actions

Many minor things can be added to improve this situation:

any or all of which detailing that the change is expected to be reinserted in the event stream during the undo action.

Jugen commented 3 years ago

@delvh thanks for submitting this issue. Would you please submit a PR with the remediation actions you have proposed, and I'll happily merge it for you.

(The original author is no longer maintaining UndoFX, I just happen to have access because it's linked to RichTextFX.)