[GitHub] sdedic commented on a change in pull request #639: [NETBEANS-691]: Corrected undo conversion of constructor to factory w…

[GitBox <gi...@apache.org>]

sdedic commented on a change in pull request #639: [NETBEANS-691]: Corrected undo conversion of constructor to factory w…
URL: https://github.com/apache/incubator-netbeans/pull/639#discussion_r208242260
 
 

 ##########
 File path: openide.text/src/org/openide/text/UndoRedoManager.java
 ##########
 @@ -251,7 +251,7 @@ void beforeUndoAtSavepoint(WrapUndoEdit edit) {
     }
         
     private void undoSaveActions() {
-        if (onSaveTasksEdit != null) {
+        if (onSaveTasksEdit != null && onSaveTasksEdit.canUndo()){
 
 Review comment:
   re: 
   > So should I check for canRedo() inside redoSaveActions?
   yes, whether to do it was the original question. 
   
   Never mind, your subtle change is necessary as `undo` should be never called on an action that gives `!canUndo()`. Please do the same for the Redo, I don't know how to simulate situation which would trigger the bug, but the check follows the API spec. Accidentaly thrown `CannotUndoException` would certainly corrupt the undo/redo processing queue. 
   
   Thanks.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists