This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 180614 - Rewrite UndoAction and RedoAction to be context-aware
Summary: Rewrite UndoAction and RedoAction to be context-aware
Status: RESOLVED FIXED
Alias: None
Product: platform
Classification: Unclassified
Component: Actions (show other bugs)
Version: 6.x
Hardware: PC Mac OS X
: P3 normal (vote)
Assignee: Jaroslav Tulach
URL: http://deadlock.netbeans.org/hudson/j...
Keywords: API_REVIEW_FAST
Depends on: 180920
Blocks:
  Show dependency tree
 
Reported: 2010-02-11 08:16 UTC by Miloslav Metelka
Modified: 2011-05-24 13:54 UTC (History)
11 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
Raw API change (35.91 KB, patch)
2010-02-18 11:26 UTC, Jaroslav Tulach
Details | Diff
The new diff. (42.19 KB, patch)
2010-03-22 10:45 UTC, Jaroslav Tulach
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Miloslav Metelka 2010-02-11 08:16:19 UTC
There's a usecase in VCS to use an editor pane without a TopComponent. NbEditorKit delegates to UndoAction and RedoAction but without a TopComponent these actions would operate on a wrong context. Thanks.
Once the actions get rewritten the NbEditorKit.NbUndoAction (and NbRedoAction) may be changed to construct the action's instance with a NbEditorUtilities.getDataObject(doc).getLookup().
Comment 1 Jaroslav Tulach 2010-02-18 11:26:06 UTC
Created attachment 94302 [details]
Raw API change

Not fully finished API change that makes UndoAction and RedoAction context aware, introduces new UndoRedo.Provider interface, retrofits TopComponent to implement it and changes CloneableEditorSupport to implement it too. This shall ensure that one can

DataObject obj = DataObject.find(...);
action = undoAction.createContextAware(obj.getLookup());

associate undo or redo action with any object providing CloneableEditorSupport.
Comment 2 Jaroslav Tulach 2010-03-18 17:47:59 UTC
I will polish the patch and integrate tomorrow.
Comment 3 Jaroslav Tulach 2010-03-22 10:45:12 UTC
Created attachment 95515 [details]
The new diff. 

The change is getting more and more complicated. First of all the depending bug 180920 does not seem to be fixed. Second, the retrofitting of TopComponent to implement UndoRedo.Provider is not source compatible change, I need to add dependency on openide.awt for some modules to compile. Last, the retrofitting of CloneableEditorSupport to implement UndoRedo.Provider requires the getUndoRedo field to be made public.
Comment 4 Jaroslav Tulach 2010-03-22 10:46:19 UTC
Víťa promised to look at the NPE 180920 bug which can be simulated after applying the latest patch from this issue.
Comment 5 err 2010-04-28 14:05:46 UTC
Is  Bug 175809 -  undo/redo disabled, but editor has focus
related to this issue?
Comment 6 Jaroslav Tulach 2010-04-29 14:08:44 UTC
The change is source incompatible and requires changes to many modules. Unless really necessary, I'd like to defer it after 6.9 is branched.
Comment 7 Jesse Glick 2010-04-29 20:09:29 UTC
[JG01] Is it intentional in your patch that RedoAction.instance has undo=true while UndoAction.instance has redo=true? Might anyway be simpler to have just one attr, undo=true or undo=false, with

public static Action create(Map<?,?> map) {
  return new UndoRedoAction(Utilities.actionsGlobalContext(), (Boolean) map.get("undo"), true);
}


[JG02] {@link UndoRedo#Provider} will not work as far as I know; you need {@link org.openide.awt.UndoRedo.Provider}.
Comment 8 Jaroslav Tulach 2010-06-25 16:47:33 UTC
I've just created a hudson builder:
http://deadlock.netbeans.org/hudson/job/prototypes-context-aware-undo-redo-180614/
to verify that the changes are sane and remain buildable.
Comment 9 Jaroslav Tulach 2010-06-25 17:17:30 UTC
I am looking at the NbEditorKit code and there is:

    public static class NbUndoAction extends ActionFactory.UndoAction {

        public @Override void actionPerformed(ActionEvent evt, JTextComponent target) {
            Document doc = target.getDocument();
            if (doc.getProperty(BaseDocument.UNDO_MANAGER_PROP) != null) { // Basic way of undo
                super.actionPerformed(evt, target);
            } else { // Deleagte to system undo action
                // Delegate to system undo action
                UndoAction ua = (UndoAction)SystemAction.get(UndoAction.class);
                if (ua != null && ua.isEnabled()) {
                    ua.actionPerformed(evt);
                }
            }
        }

    }

as far as I can tell, if the document provides UndoRedo.Manager as BaseDocument.UNDO_MANAGER_PROP, then we don't need to change anything. Mílo, Ondro?
Comment 10 Jaroslav Tulach 2010-06-28 15:25:46 UTC
I think context sensitive Undo/Redo action is useful with or without its usages in editor. Unless there are objections I will tomorrow apply:
http://hg.netbeans.org/core-main/rev/0e1285942bb5
Comment 11 Jaroslav Tulach 2010-06-29 08:28:08 UTC
core-main#00798124636d
Comment 12 Miloslav Metelka 2010-06-29 10:47:19 UTC
Yes, since UndoRedo.Manager extends javax.swing.undo.UndoManager it should work fine.
Comment 13 Petr Jiricka 2010-07-07 13:33:53 UTC
This is a wild guess, but could this change be the cause of bug 188363?
Comment 14 gwaldon 2011-05-16 17:18:13 UTC
Is-there any reason why CloneableEditorSupport does not implement this (would require getUndoRedo field to be made public)? I have a multiview system with multiple components which default to the same CloneableEditorSupport for their UndoRedo; this provider would be very handy.

Also one of them is a second CloneableEditorSupport and if we could have the getUndoRedo field not final, life would be easier. Just a case scenario.
Comment 15 Jaroslav Tulach 2011-05-24 13:54:03 UTC
Originally I wanted to change CloneableEditorSupport as well, but its getUndoRedo() method is protected. If I wanted to implement the interface, I'd have had to make the method public. That would not be source compatible change - many existing codebases would not compile. I decided to avoid that.

Btw. if you are interested in MultiView watch bug 196810.