From: SourceForge.net <no...@so...> - 2007-09-13 21:24:26
|
Patches item #1782091, was opened at 2007-08-26 12:29 Message generated for change (Comment added) made by ezust You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=300588&aid=1782091&group_id=588 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: None Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Shlomy Reinstein (shlomy) Assigned to: Alan Ezust (ezust) Summary: An interface for dockable windows Initial Comment: The attached patch (jedit.patch) declares a minimalistic (at this stage) interface for dockable windows, that notifies them when the docking area is changed, so they can adjust (e.g. change the layout). In addition, this patch makes jEdit use the same instance of the dockable when the docking area is changed, instead of invoking the BeanShell code for creating the dockable each time. This new behavior is much more sensible, since it is now easy for dockables to maintain their state when they are moved between the docking areas, whereas the current behavior requires the dockable to maintain a view->instance mapping. Also, this patch includes a workaround for a component reparenting problem in Swing: If the dockable instance is moved from its docking area to a floating container (using "Float"), the instance is sometimes blank. The workaround is to create a new JPanel around it each time. For some reason, this also not always works. For backward compatibility, dockables are required to implement the new interface DockableWindow in order to use the new behavior. Dockables that do not implement DockableWindow do not use the new behavior, and instead a new instance is created each time the docking is changed. A corresponding patch is also attached for the GlobalPlugin (global.patch), that uses this jEdit patch, so you can see the differences in code and behavior. ---------------------------------------------------------------------- >Comment By: Alan Ezust (ezust) Date: 2007-09-13 14:24 Message: Logged In: YES user_id=935841 Originator: NO I am testing this patch now... I noticed that when I try to float the ProjectViewer, now I get this exception: java.lang.UnsupportedOperationException: ProjectViewer does not support multiple instances per view. at projectviewer.ProjectViewer.<init>(ProjectViewer.java:421) at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method) at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:39) at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:27) at java.lang.reflect.Constructor.newInstance(Constructor.java:494) at bsh.Reflect.constructObject(Reflect.java:620) at bsh.BSHAllocationExpression.constructObject(BSHAllocationExpression.java:123) at bsh.BSHAllocationExpression.objectAllocation(BSHAllocationExpression.java:114) at bsh.BSHAllocationExpression.eval(BSHAllocationExpression.java:62) at bsh.BSHPrimaryExpression.eval(BSHPrimaryExpression.java:102) at bsh.BSHPrimaryExpression.eval(BSHPrimaryExpression.java:47) at bsh.Interpreter.eval(Interpreter.java:645) at bsh.Interpreter.eval(Interpreter.java:739) at bsh.Interpreter.eval(Interpreter.java:728) at org.gjt.sp.jedit.BeanShell._eval(BeanShell.java:432) at org.gjt.sp.jedit.BeanShell.eval(BeanShell.java:396) at org.gjt.sp.jedit.gui.DockableWindowFactory$Window.createDockableWindow(DockableWindowFactory.java:443) at org.gjt.sp.jedit.gui.DockableWindowManager.showDockableWindow(DockableWindowManager.java:336) at org.gjt.sp.jedit.gui.DockableWindowManager$3.actionPerformed(DockableWindowManager.java:676) at javax.swing.AbstractButton.fireActionPerformed(AbstractButton.java:1849) at javax.swing.AbstractButton$Handler.actionPerformed(AbstractButton.java:2169) at javax.swing.DefaultButtonModel.fireActionPerformed(DefaultButtonModel.java:420) at javax.swing.DefaultButtonModel.setPressed(DefaultButtonModel.java:258) at javax.swing.AbstractButton.doClick(AbstractButton.java:302) at javax.swing.plaf.basic.BasicMenuItemUI.doClick(BasicMenuItemUI.java:1000) at javax.swing.plaf.basic.BasicMenuItemUI$Handler.mouseReleased(BasicMenuItemUI.java:1041) at java.awt.Component.processMouseEvent(Component.java:5501) at javax.swing.JComponent.processMouseEvent(JComponent.java:3135) at java.awt.Component.processEvent(Component.java:5266) at java.awt.Container.processEvent(Container.java:1966) at java.awt.Component.dispatchEventImpl(Component.java:3968) at java.awt.Container.dispatchEventImpl(Container.java:2024) at java.awt.Component.dispatchEvent(Component.java:3803) at java.awt.LightweightDispatcher.retargetMouseEvent(Container.java:4212) at java.awt.LightweightDispatcher.processMouseEvent(Container.java:3892) at java.awt.LightweightDispatcher.dispatchEvent(Container.java:3822) at java.awt.Container.dispatchEventImpl(Container.java:2010) at java.awt.Window.dispatchEventImpl(Window.java:1778) at java.awt.Component.dispatchEvent(Component.java:3803) at java.awt.EventQueue.dispatchEvent(EventQueue.java:463) at java.awt.EventDispatchThread.pumpOneEventForHierarchy(EventDispatchThread.java:242) at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:163) at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:157) at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:149) at java.awt.EventDispatchThread.run(EventDispatchThread.java:110) PV does not define movable="TRUE"... Shouldn't it have the previous behavior for legacy plugins? ---------------------------------------------------------------------- Comment By: Shlomy Reinstein (shlomy) Date: 2007-09-09 13:27 Message: Logged In: YES user_id=1477607 Originator: YES I've updated the patch to include my update to the documentation and the DTD for the dockables.xml files. File Added: patch ---------------------------------------------------------------------- Comment By: Shlomy Reinstein (shlomy) Date: 2007-09-09 03:36 Message: Logged In: YES user_id=1477607 Originator: YES As for your other questions: 1. I removed the patch file for GlobalPlugin as it is no longer needed. The changes are already committed to SVN. I just removed the redundant code with the view->instance map and added the MOVABLE attribute in the dockables.xml. Just check-out the new version. Currently this means that until the jEdit patch is applied, a new instance of the GlobalPlugin dockables will be created whenever the docking position is changed - just like with the rest of the plugin dockables. If you apply the jEdit patch, the instances will be reused. 2. I forgot that the API docs should be updated together with the code. I'll update them and replace the attached patch with a new one with updated docs. 3. While at it, I noticed I didn't update the DTD of the dockables.xml. Is this DTD used at all? In the environment where I tried it, it works fine without changing the DTD. If the DTD is obsolete, it should be removed. If not, I will update it and include the update in the new patch file. 4. Regarding other features in the DockableWindowManager interface: There is one more feature I want to add, which we discussed before - an additional attribute in the XML that specifies whether multiple instances of the dockable are allowed per view. This attribute will, for now, tell jEdit whether it should provide the "New floating instance" action for the dockable. But this is much less important than the one I added in this patch. The one in this patch makes it easy to instantly fix the behavior for many plugin dockables (e.g. Console, ClassBrowser, Sidekick etc). ---------------------------------------------------------------------- Comment By: Shlomy Reinstein (shlomy) Date: 2007-09-09 00:56 Message: Logged In: YES user_id=1477607 Originator: YES As for the question why we need the MOVABLE attribute, it's simply a matter of convenience: 1. You don't have to modify plugin code in order to make dockables movable. You just need to update dockables.xml, and you get the behavior you want. Otherwise, you need to modify the classes implementing these dockables to implement the DockableWindow interface. Imagine now going through all plugins, modifying their code and rebuilding them just for this new behavior. 2. Most dockables do not care where they are positioned, so most "move" methods will just be empty. Why require them to define an empty method? I think it's much better to have this "movable" attribute. Another (future) benefit is that we may make this attribute true by default, but we cannot make it implement an interface by default. ---------------------------------------------------------------------- Comment By: Alan Ezust (ezust) Date: 2007-09-08 17:58 Message: Logged In: YES user_id=935841 Originator: NO Also, are there any other features (methods in the interface) you may need to have in the DockableWindow interface for an enhanced dockable window manager, such as FlexDock? ---------------------------------------------------------------------- Comment By: Alan Ezust (ezust) Date: 2007-09-08 17:58 Message: Logged In: YES user_id=935841 Originator: NO Also, are there any other features (methods in the interface) you may need to have in the DockableWindow interface for an enhanced dockable window manager, such as FlexDock? ---------------------------------------------------------------------- Comment By: Alan Ezust (ezust) Date: 2007-09-08 17:57 Message: Logged In: YES user_id=935841 Originator: NO Thinking about it some more, why do we need a MOVABLE="true" attribute anyway? We can always use rtti to determine if the dockable is movable. The additional attribute seems redundant to me. If the thing doesn't implement DockableWindow, we don't call the move() method, and instead create a new one. ---------------------------------------------------------------------- Comment By: Alan Ezust (ezust) Date: 2007-09-08 17:56 Message: Logged In: YES user_id=935841 Originator: NO Thinking about it some more, why do we need a MOVABLE="true" attribute anyway? We can always use rtti to determine if the dockable is movable. The additional attribute seems redundant to me. If the thing doesn't implement DockableWindow, we don't call the move() method, and instead create a new one. ---------------------------------------------------------------------- Comment By: Alan Ezust (ezust) Date: 2007-09-08 17:48 Message: Logged In: YES user_id=935841 Originator: NO The GlobalPlugin patch is no longer there. I'd still like to try it out first. Please re-add? Also, there needs to be more API docs, which I'm adding to my version, if I end up being the one to commit this. ---------------------------------------------------------------------- Comment By: Alan Ezust (ezust) Date: 2007-09-08 17:48 Message: Logged In: YES user_id=935841 Originator: NO The GlobalPlugin patch is no longer there. I'd still like to try it out first. Please re-add? Also, there needs to be more API docs, which I'm adding to my version, if I end up being the one to commit this. ---------------------------------------------------------------------- Comment By: Shlomy Reinstein (shlomy) Date: 2007-09-08 06:11 Message: Logged In: YES user_id=1477607 Originator: YES Attaching a new patch which does not require a change in the code, only an additional MOVABLE="TRUE" attribute in the dockable.xml in order to make a dockable instance move instead of recreated when the docking position is changed. The DockableWindow interface is not required in order to use the new behavior. If a dockable that has MOVABLE="TRUE" implements this interface, it is notified about the change in docking position using this interface, otherwise it is moved without being notified. The GlobalPlugin version in SVN is already adapted to the new behavior. File Added: patch ---------------------------------------------------------------------- Comment By: Shlomy Reinstein (shlomy) Date: 2007-09-02 12:59 Message: Logged In: YES user_id=1477607 Originator: YES Reviving this patch because, as it turns out, the DockableWindowManager API cannot be used to get the existing dockable instance for the view when the docking position is changed. The DWM gets rid of the instance before it invokes the BeanShell code to get an instance for the new docking position. The problems with this patch will be handled one by one. But this patch already improves the existing state. ---------------------------------------------------------------------- Comment By: Shlomy Reinstein (shlomy) Date: 2007-08-27 22:22 Message: Logged In: YES user_id=1477607 Originator: YES Closing this because there is no real need in this patch. A plugin can use the DockableWindowManager API to get the existing instance of the dockable for the view, if exists, so it can easily supporting preserving state when moving between docking areas. The attached patch suffers from some problems that can be easily avoided by correctly handling dockable movements in the plugin. Hence, I am replacing this patch with an updated section in the plugin development section of the jEdit user guide. ---------------------------------------------------------------------- Comment By: Shlomy Reinstein (shlomy) Date: 2007-08-26 12:29 Message: Logged In: YES user_id=1477607 Originator: YES File Added: global.patch ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=300588&aid=1782091&group_id=588 |