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

Bug 235657

Summary: [Markers][Undo] regression: can no longer undo marker operations
Product: [Eclipse Project] Platform Reporter: Kim Horne <eclipse>
Component: IDEAssignee: Dani Megert <daniel_megert>
Status: VERIFIED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: daniel_megert, emoffatt, hsoliwal, susan
Version: 3.4   
Target Milestone: 3.7 M5   
Hardware: All   
OS: All   
Whiteboard:
Attachments:
Description Flags
intial patch
none
Patch 2
none
Fix none

Description Kim Horne CLA 2008-06-04 13:45:14 EDT
I20080523-0100

We advertised the ability to undo marker operations in 3.3 (such as adding a task or bookmark) but this no longer works in 3.4
Comment 1 Hitesh CLA 2008-12-11 06:15:04 EST
Created attachment 120175 [details]
intial patch

Initial patch to support undo/redo.

The existing marker-operations do not allow undo/redo of certain operations after a few ops. The cached markers get stale , so,that would need to be looked into.
  
What do you think about the patch?
Comment 2 Kim Horne CLA 2008-12-15 08:33:05 EST
Some comments:

1) you're using 1.5 APIs - Integer.parseInt(int) is a no-go.  Make sure you're set up with a 1.4 VM as a target VM so this doesn't happne

2) You use actions in the ExtendedMarkersView.  That's not the end of the world, but I think everything else in the new markers view uses the menus API.  It would be good if the actions could be added via that mechanism
Comment 3 Hitesh CLA 2009-01-02 06:02:19 EST
Created attachment 121426 [details]
Patch 2

> 
> 2) You use actions in the ExtendedMarkersView.  That's not the end of the
> world, but I think everything else in the new markers view uses the menus API. 
> It would be good if the actions could be added via that mechanism
> 
Currently it is not possible to hook into global Undo / Redo actions by declaring handlers to their command ids.See Bug 259859.

There are few difficulties in adding support for undo, redo to the new marker based views.

The common infrastructure for markers allows adding new marker views with arbitrary marker types. The existing marker operations add pre-decided undocontexts based on types. This makes the undo/redo for Tasks and BookMarks work across views (for example, all markers view and the receptive view itself). Staying consistent with the above behaviour, the undo context would have to be on a per marker type rather than per view basis. However this is not good idea and would make things complex and confusing.

Although this patch attempts this, but having the undocontext on per view basis makes it difficult to pass its reference to places where editing happens. For example ,the celleditors in the various fields and the property page for marker. 

Eric , your thoughts ?
Comment 4 Susan McCourt CLA 2009-02-19 13:40:53 EST
just noticed this.  Eric, are you looking after this one?  I'm assuming Tod is the wrong assignee.
Comment 5 Eric Moffatt CLA 2009-02-20 11:26:59 EST
Susan, Hitesh has been working in this area mostly (I just review his patches...;-).

Hitesh, I'll take a look at the patch but would it be possible to make the Undo/Redo work for the marker types we handle (without breaking other things) ? This would at least avoid the regression. 

I'm less concerned about supporting it for folks doing their own marker types, it'd be nice if they had a way to hook it up but fixing the regression is a must.
Comment 6 Dani Megert CLA 2010-10-05 09:38:55 EDT
Ping!
Comment 7 Dani Megert CLA 2010-11-17 04:39:17 EST
Just for the records: Undo does work if the Navigator or Package Explorer is active.
Comment 8 Dani Megert CLA 2011-01-06 11:20:48 EST
Let me fix this.
Comment 9 Dani Megert CLA 2011-01-07 05:52:03 EST
Created attachment 186256 [details]
Fix
Comment 10 Dani Megert CLA 2011-01-07 05:52:28 EST
Fixed in HEAD.
Available in builds >= N20110107-2000.
Comment 11 Dani Megert CLA 2011-01-25 03:27:12 EST
Verified in I20110124-1800.