Bugzilla – Full Text Bug Listing
| Summary: | [Command] CommandStack canRedo should check Command canExecute | ||
|---|---|---|---|
| Product: | [Tools] GEF | Reporter: | Paul Bilnoski <bilnoski> |
| Component: | GEF-Legacy GEF (MVC) | Assignee: | Alexander Nyßen <nyssen> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | nyssen |
| Version: | unspecified | ||
| Target Milestone: | 3.10.0 (Mars) M2 | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | |||
I found a mistake in the description, it should instead read: "If a change is made to the same model element via the JFace viewer, the "UNDO" action becomes unavailable because the command itself revalidates when asked to by the command stack." Actually Command seems to be missing a canRedo(), which should be called by CommandStack as you described. Calling canExecute() to check whether redo() can be called does not seem to be appropriate because redo() will only by default delegate to execute() and may be implemented differently. I think we should add canRedo() to Command, delegating to canExecute() by default and call this from within CommandStack#canRedo(). We will also have to overwrite in within CompoundCommand to collect the canRedo()-results from nested commands. Implemented the following changes: - Added canRedo() to Command, which by default delegates to canExecute(), similar to redo() delegates to execute(). Subclasses may overwrite to decide whether a command is redoable. - Added an implementation for canRedo() within CompoundCommand, which overwrites the default implementation within Command and delegates to the canRedo() of the nested commands (similar as for canExecute() and canUndo()). - Changed implementation of CommandStack#canRedo() to evaluate the canRedo() of the current redoable command. - Fixed that undo() did not check that the undoable command is indeed undoable (like done for redo()) but execute the undo in each case. Now, as redo() it is a no-op in case the current undoable command is not undoable. All changes committed to origin/master. Resolving as fixed in 3.10.0M2. |
Build Identifier: 3.6.1 I have a GEF viewer and a JFace tree viewer both visualizing the same model and both allowing edit operations, such as the ability to reparent an item. In the GEF viewer this is done through handling a RequestConstants.REQ_ADD ("add children") command. When a reparent is done, the "undo" action is correctly available and the "redo" action is correctly unavailable. If a change is made to the same model element via the JFace viewer, the "redo" action becomes unavailable because the command itself revalidates when asked to by the command stack. However, if the "undo" is performed, and the "redo" action correctly becomes available and then the model element is updated, the "redo" action is still available even though it is invalid. This is because the command stack simply tests "canRedo" by checking that a command exists on the redoable stack, but does not check that the command is again executable. I have subclassed CommandStack and overridden "canRedo" as such: public boolean canRedo() { Command toRedo = getRedoCommand(); if (toRedo == null) return false; return toRedo.canExecute(); } A proposed patch to the code could access internals, and might be instead: public boolean canRedo() { if (redoable.isEmpty()) return false; return ((Command)redoable.peek()).canExecute(); } Reproducible: Always