Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 232909

Summary: UndoLimit in CommandStack works not as described
Product: [Tools] GEF Reporter: Marco Lehmann-Mörz <mlehmannm>
Component: GEF-Legacy GEF (MVC)Assignee: gef-inbox <gef-inbox>
Status: RESOLVED WONTFIX QA Contact:
Severity: normal    
Priority: P3 CC: ahunter.eclipse, hudsonr
Version: 3.3.2   
Target Milestone: ---   
Hardware: PC   
OS: Windows XP   
Whiteboard:
Attachments:
Description Flags
Patch that fixes the described problem for me, please review. none

Description Marco Lehmann-Mörz CLA 2008-05-20 03:46:03 EDT 
Build ID: 3.3.2

I set an undo limit of 0 (zero), because undo is not supported. Everytime a command gets executed, the command stack adds the command to its undoable stack. The undo limit is not respected. The check for the undo limit is false and at the wrong location. If you move the check after the push operation and if you change the check from > to >= it might work.
Comment 1 Marco Lehmann-Mörz CLA 2008-05-20 13:24:31 EDT 
Created attachment 101104 [details]
Patch that fixes the described problem for me, please review.
Comment 2 Randy Hudson CLA 2008-05-20 23:05:50 EDT 
The patch makes sense, but unfortunately an undo limit of 0 is currently treated the same as a limit of -1. In fact, we default to 0 when we should be defaulting to -1.

We should try to find a solution that doesn't break the existing behavior, even when the existing behavior is broken. For example, if your command returned false for canUndo(), we could probably flush the whole undo stack, which would also dispose your command, assuming that is your goal. What is the real problem you are trying to solve?
Comment 3 Marco Lehmann-Mörz CLA 2008-05-21 01:54:37 EDT 
Yes, that is my goal. We have dangling references in our program and one source is the command stack. Another solution that works for me is flushing the command stack at appropriate times (e.g. changing the model).

All of our commands return false on canUndo(), so that your solution might work too.

Will the fix be available for the 3.3 stream?
Comment 4 Randy Hudson CLA 2008-05-21 09:41:42 EDT 
> Will the fix be available for the 3.3 stream?

It seems like this is a temporary memory leak, not a permanent one, so I don't see anyone fixing it for 3.3, which has already had 2 maintenance releases. I'm sure you can find other temporary memory leaks in eclipse like actions holding onto the most recent selection, etc.

You can easily workaround the behavior by subclassing and overriding execute(Command) to call flush() afterwards, or simply adding a command stack listener which does the same on POST_EXECUTE.

If you'd like this fixed in the 3.4 development time, a new patch with the alternative approach would really help.

Hmm, I just looked again at what flush() would do, and it would lose the dirty state of the command stack. So, the workaround isn't perfect. But since your editor doesn't support undo/redo, maintaining your own dirty state outside the command stack would be trivial. As soon as anything happens your editor is dirty and never becomes clean until save occurs.
Comment 5 Anthony Hunter CLA 2009-04-02 13:46:56 EDT 
(In reply to comment #4)
> If you'd like this fixed in the 3.4 development time, a new patch with the
> alternative approach would really help.

Marking as won't fix. Please reopen if you are able to provide a patch.