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.
The DataLoader.setAction( SystemAction[] ) method did not reflect the move of actions from SystemActions to javax.swing.Actions. This results in lot of wrapping code in the IDE. It would be nice to allow the method to take array ofjavax.swing.Actions as parameter.
Fixing Summary
I've just faced the first problem. DataLoader.setActions(SystemAction[]) stores the actions persistently. Which is ok, as it is enough to just know the action's classname. But what should be done with javax.swing.Action? First thought: If not serializable, skip them. If serializable serialize them. Does not work well, as javax.swing.AbstractAction is serializable and thus most of the actions are serializable by default. That would lead to problems. That is for sure.
Created attachment 15831 [details] Current version - everything works but the serialization
Created attachment 16132 [details] Declarative approach that uses layers
I guess that this second approach might solve all the troubles. The actions are read from a folder on disk which name is provided by String DataLoader.actionsContext() which defaults in case of java to org-openide-loaders/org-netbeans-modules-java-JavaDataLoader/Actions but can of course be overriden to point anywhere. Basically your modules (java, projects, refactoring) need to create all the actions in layer and do not even touch DataLoader.setActions. More or less this is ready to be commited. The only obsticles that may cause problems and I am aware of are backward compatibility with existing loaders (right now I just create all the files in actionsContext() folder as soon as getActions() is called based on the content returned from defaultActions() method) and the UI in Tools/Options that allows customizations. It would likely need a bit of rewrite after this change (otherwise I am affraid user would not be able to manipulate with non SystemActions). I am reassigning this to Hrebejk to give it a try and evaluate the usefulness of implementing this.
As Yarda pointed out there are better ways how to improve the performance of the Project actions. So maybe we should not integrated this in the D time frame. Good candidate for the next release though.
Created attachment 16593 [details] Declarative approach that fully keeps backward compatibility
After discussions with Hrebejk and others we have concluded that it makes sence to provide a way how loaders can read their actions from a certain location in a layer. However we decided that this should be an extension behaviour, not being turned on by default. That is why there is new protected method String actionsContext() that by default return null, but can be overriden to instruct the loader to take its actions from the layers. If not overriden everything is supposed to behave as it did until now. I am confident about quality of my code (well, two-three days of bug fixing may be needed), so I am ready to commit immediatelly, but as Hrebejk's problem may have another fix, Matula keeps his silence and there would be some work needed by the java module (well override actionsContext and define its actions there ;-), I now plan to integrate this for promo-E and I am no longer threating this as a defect.
The hack implemented in JavaProjectModule is now in WebProjectModule too where JspDO and HtmlDO functionality is extended. I agree we should do it for promo-E. (don't we miss API keyword here?)
Cool! Probably also need to rewrite the action customizer set by DataLoaderBeanInfo and used in the Object Types subnodes in the Options dialog, which assumes SystemAction's. Also compare David Strupl's openidex/enode which solves this kind of problem more generally for any node incl. actions, lookup, and icons. Of course Looks would solve it even more generally, but we have to stop fantasizing at some point and actually put something workable into a real release. :-)
No need to compare with enode, I have stolen its tests when I started. My goal was to solve the setAction problem as simply as possible, I did not want to get into the complexities of rewriting completely everything. However, I kept in mind the future compatibility of my solution and I believe that if we switch to enode or looks in future we can maintain the action registration contract because both looks and enode support reading of actions from layer, we just need to keep the same locations.
While testing this patch I found that loader using this declarative approach is serialized. The DataLdrActions (folder instance) calls DataLoader.setSwingActions and here property change event is fired that causes later serialization. This probably should be fixed before integration.
Created attachment 18781 [details] New patch with changes to openide,core,java and refactoring modules
I have find out this issue has not been reviewed yet. So let's start the review. The previous diff is a good way to show how the thing should work. There is just one problem I am aware of and that is FormDataLoader having different actions than JavaDataLoader while subclassing it. I think the immediate solution is to write: String JavaDataLoader.actionsContext () { if (getClass () == JavaDataLoader.class) { return "Loaders/text/x-java/Actions"; } else { return null; } } which will retain the compatibility for FormDataLoader till all the actions registered in the layer continue to be SystemActions (are now) if that changes (and that is the goal) the FormDataLoader needs to be rewritten and its "enhancing" code moved to FormDataNode. To delegate to getActions(boolean) and "enhance" the actions array there. Or the form can overide actionsContext() and let any other module to register specific actions to its own context - completely free itself from depence on java actions with all the pros and cons. Anyway I believe the patch is ready to be applied (works the same way as 4.0 in Tools/Option/Object Types), before integration I plan to add bunch of arch-*.xml documentations to describe all the new layer APIs that are being introduced. Please review.
I agree the "enhancing" code in form editor could be moved to FormDataNode. Don't think the second option - providing its own context - is usable. I just think we should be sure that customizing the actions in FormDataNode really works (should not be a problem to rewrite this) before integrating the patch.
Created attachment 18792 [details] Improvements for the FormDataNode, however the code is still as fragile as it was - e.g. it continues to make the same dangerous assumptions about the result of javaLoader.getActions()
Looks good, thanks.
I'll integrate this tommorow.
~/work$ cvs -q ci -m "#45137: DataLoader actions can be read from a layer" openide core java refactoring projects Checking in openide/arch/arch-openide-datasystems.xml; /cvs/openide/arch/arch-openide-datasystems.xml,v <-- arch-openide-datasystems.xml new revision: 1.23; previous revision: 1.22 done Processing log script arguments... More commits to come... Checking in openide/loaders/manifest.mf; /cvs/openide/loaders/manifest.mf,v <-- manifest.mf new revision: 1.17; previous revision: 1.16 done Processing log script arguments... More commits to come... Checking in openide/loaders/api/apichanges.xml; /cvs/openide/loaders/api/apichanges.xml,v <-- apichanges.xml new revision: 1.11; previous revision: 1.10 done Processing log script arguments... More commits to come... RCS file: /cvs/openide/loaders/src/org/openide/loaders/DataLdrActions.java,v done Checking in openide/loaders/src/org/openide/loaders/DataLdrActions.java; /cvs/openide/loaders/src/org/openide/loaders/DataLdrActions.java,v <-- DataLdrActions.java initial revision: 1.1 done Checking in openide/loaders/src/org/openide/loaders/DataLoader.java; /cvs/openide/loaders/src/org/openide/loaders/DataLoader.java,v <-- DataLoader.java new revision: 1.7; previous revision: 1.6 done Checking in openide/loaders/src/org/openide/loaders/DataLoaderPool.java; /cvs/openide/loaders/src/org/openide/loaders/DataLoaderPool.java,v <-- DataLoaderPool.java new revision: 1.16; previous revision: 1.15 done Checking in openide/loaders/src/org/openide/loaders/DataNode.java; /cvs/openide/loaders/src/org/openide/loaders/DataNode.java,v <-- DataNode.java new revision: 1.17; previous revision: 1.16 done Processing log script arguments... More commits to come... Checking in openide/src/org/openide/explorer/propertysheet/PropertySheet.java; /cvs/openide/src/org/openide/explorer/propertysheet/PropertySheet.java,v <-- PropertySheet.java new revision: 1.166; previous revision: 1.165 done Processing log script arguments... More commits to come... RCS file: /cvs/openide/test/unit/src/org/openide/loaders/DataLoaderGetActionsCompatibilityTest.java,v done Checking in openide/test/unit/src/org/openide/loaders/DataLoaderGetActionsCompatibilityTest.java; /cvs/openide/test/unit/src/org/openide/loaders/DataLoaderGetActionsCompatibilityTest.java,v <-- DataLoaderGetActionsCompatibilityTest.java initial revision: 1.1 done RCS file: /cvs/openide/test/unit/src/org/openide/loaders/DataLoaderGetActionsTest.java,v done Checking in openide/test/unit/src/org/openide/loaders/DataLoaderGetActionsTest.java; /cvs/openide/test/unit/src/org/openide/loaders/DataLoaderGetActionsTest.java,v <-- DataLoaderGetActionsTest.java initial revision: 1.1 done Processing log script arguments... More commits to come... Checking in core/src/org/netbeans/core/LoaderPoolNode.java; /cvs/core/src/org/netbeans/core/LoaderPoolNode.java,v <-- LoaderPoolNode.java new revision: 1.77; previous revision: 1.76 done Processing log script arguments... More commits to come... RCS file: /cvs/core/test/unit/src/org/netbeans/core/LoaderPoolNodeTest.java,v done Checking in core/test/unit/src/org/netbeans/core/LoaderPoolNodeTest.java; /cvs/core/test/unit/src/org/netbeans/core/LoaderPoolNodeTest.java,v <-- LoaderPoolNodeTest.java initial revision: 1.1 done Processing log script arguments... More commits to come... Checking in core/ui/src/org/netbeans/core/ui/resources/layer.xml; /cvs/core/ui/src/org/netbeans/core/ui/resources/layer.xml,v <-- layer.xml new revision: 1.85; previous revision: 1.84 done Processing log script arguments... More commits to come... Checking in java/manifest.mf; /cvs/java/manifest.mf,v <-- manifest.mf new revision: 1.82; previous revision: 1.81 done Processing log script arguments... More commits to come... Checking in java/arch/arch-java.xml; /cvs/java/arch/arch-java.xml,v <-- arch-java.xml new revision: 1.7; previous revision: 1.6 done Processing log script arguments... More commits to come... Checking in java/nbproject/project.xml; /cvs/java/nbproject/project.xml,v <-- project.xml new revision: 1.9; previous revision: 1.8 done Processing log script arguments... More commits to come... Checking in java/project/manifest.mf; /cvs/java/project/manifest.mf,v <-- manifest.mf new revision: 1.10; previous revision: 1.9 done Processing log script arguments... More commits to come... Checking in java/project/src/org/netbeans/modules/java/project/JavaProjectModule.java; /cvs/java/project/src/org/netbeans/modules/java/project/JavaProjectModule.java,v <-- JavaProjectModule.java new revision: 1.5; previous revision: 1.4 done Checking in java/project/src/org/netbeans/modules/java/project/layer.xml; /cvs/java/project/src/org/netbeans/modules/java/project/layer.xml,v <-- layer.xml new revision: 1.14; previous revision: 1.13 done Processing log script arguments... More commits to come... Checking in java/src/org/netbeans/modules/java/JavaDataLoader.java; /cvs/java/src/org/netbeans/modules/java/JavaDataLoader.java,v <-- JavaDataLoader.java new revision: 1.73; previous revision: 1.72 done Processing log script arguments... More commits to come... Checking in java/src/org/netbeans/modules/java/resources/mf-layer.xml; /cvs/java/src/org/netbeans/modules/java/resources/mf-layer.xml,v <-- mf-layer.xml new revision: 1.52; previous revision: 1.51 done Processing log script arguments... More commits to come... Checking in refactoring/arch/arch-refactoring.xml; /cvs/refactoring/arch/arch-refactoring.xml,v <-- arch-refactoring.xml new revision: 1.6; previous revision: 1.5 done Processing log script arguments... More commits to come... Checking in refactoring/nbproject/project.xml; /cvs/refactoring/nbproject/project.xml,v <-- project.xml new revision: 1.9; previous revision: 1.8 done Processing log script arguments... More commits to come... Checking in refactoring/src/org/netbeans/modules/refactoring/RefactoringModule.java; /cvs/refactoring/src/org/netbeans/modules/refactoring/RefactoringModule.java,v <-- RefactoringModule.java new revision: 1.5; previous revision: 1.4 done Processing log script arguments... More commits to come... Checking in refactoring/src/org/netbeans/modules/refactoring/resources/mf-layer.xml; /cvs/refactoring/src/org/netbeans/modules/refactoring/resources/mf-layer.xml,v <-- mf-layer.xml new revision: 1.16; previous revision: 1.15 done Processing log script arguments... More commits to come... Checking in projects/projectui/arch.xml; /cvs/projects/projectui/arch.xml,v <-- arch.xml new revision: 1.4; previous revision: 1.3 done Processing log script arguments... More commits to come... Checking in projects/projectui/src/org/netbeans/modules/project/ui/Hacks.java; /cvs/projects/projectui/src/org/netbeans/modules/project/ui/Hacks.java,v <-- Hacks.java new revision: 1.16; previous revision: 1.15 done Processing log script arguments... More commits to come... Checking in projects/projectui/src/org/netbeans/modules/project/ui/resources/layer.xml; /cvs/projects/projectui/src/org/netbeans/modules/project/ui/resources/layer.xml,v <-- layer.xml new revision: 1.44; previous revision: 1.43 done
Tomas, please integrate the change in form, so Hrebejk can stop using SystemActions for java files.
Is there some issue filed for refactoring actions to be properly registered in the layer now?
To Jesse: I think it is covered by issue 50317.
Guys, I guess that I fixed the refactoring actions in my commit. Check the log.
You are right. Thanks!
*** Issue 53615 has been marked as a duplicate of this issue. ***
*** Issue 56214 has been marked as a duplicate of this issue. ***