| Summary: | [Markers][Undo] regression: can no longer undo marker operations | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Kim Horne <eclipse> | ||||||||
| Component: | IDE | Assignee: | 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
Kim Horne
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?
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 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 ? just noticed this. Eric, are you looking after this one? I'm assuming Tod is the wrong assignee. 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. Ping! Just for the records: Undo does work if the Navigator or Package Explorer is active. Let me fix this. Created attachment 186256 [details]
Fix
Fixed in HEAD. Available in builds >= N20110107-2000. Verified in I20110124-1800. |