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.
separate issue from #42451, will require changes in multiview API and possibly openide. http://openide.netbeans.org/tutorial/reviews/opinions_41085.html#tcr-life Factory method to create plain TopComponent
Created attachment 14643 [details] API diff
Created attachment 14644 [details] total diff for openide and core
I would like to introduce a new interface to TopComponent, TopComponent.Cloneable2 which will allow instance based decision about cloneability. This is required because of the multiview topcomponent implementation. That one delegates to the inner elements for most of the functionality. It needs to extend CloneableTopComponent, among other things because if any of the inner elements is an editor, it requires the multiview tc to be a CloneableEditorSupport.Pane instance. Without this change I would not be able to either create a non-cloneable multiview component or embed an editor view into the non-cloneable multiview component. None of the options is satisfactory. Existing TopComponent.Cloneable instances are untouched by this change. Please see attached diffs (coming with suggested apichanges.xml diff as well)
I have to admit that TopComponent.Cloneable mixes API and SPI and that may be the root source of the problem. If TC.C was SPI, I would support your suggestion to create TC.C2, but as it is also API, I am going to object. If I understand correctly, there is one module (core/multiview) that needs to implement contract of TopComponent and CloneableTopComponent and instead of implementing those contracts twice it is going to enjoy the power of the owner of the whole interface and simplify its work by introducing new interface Cloneable2. This has a direct effect for everyone who is dealing directly with TopComponent.Cloneable or CloneableTopComponent. I wish the multiview team found another way around this issue. I believe this is not the right way to go, as a first step I am removing the fasttrack keyword and turning this into real review. I hope the core/multiview team will find appropriate way how to solve the cloneability and will close this request themselves.
TC.C is not much of an API - its only likely client is the window system impl. IMHO it is almost entirely SPI.
I agree with Jesse, It's more SPI than API. What are the other possible clients wanting to clone arbitrary TopComponent?
I have to admit that I am not aware of any caller of the interface outside of windows system. But adding new interface just for communication between core/windows and core/multiview seems to me overkill. Especially when the interface suggests that the lifecycle of the component could change - e.g. for a while it could be cloneable, for a while not. I really vote for much simpler solution: methods in MultiViewFactory TopComponent createMultiComponent (...); CloneableTopComponent createCloneableMultiComponent and possibly, if needed CloneableEditorSupport.Pane createMultiEditorPane (...); If we find out that we need the new interface we can always add it later, but it is not necessary to "chodit s kanonem na vrabce".
the cloneability shall not change during the lifecycle of the component. if the name suggests that, we might change the name to a more obvious one. your suggestion looks ok, 2 drawbacks though: 1. code duplication as I will havbe to facricate 2 exactly same classes, each extending a different component (TopComponent or CloneableTopComponent). 2. and more importantly: the non-cloneable multiview component could not embed any editors. It's because the multiview component has to implement CLoneableEditorSupport.Pane interface.
Created attachment 14682 [details] new api diff
Created attachment 14683 [details] new complete changes diff.
changes according to yarda's comments attached. factory now has the createCloneableMultiView() methods that mirror functionality of createMultiView() limitations expressed earlier apply.
If the api changes are just in MultiViewFactory look like: public static TopComponent createMultiView public static CloneableTopComponent createCloneableMultiView I am fine with the change.
done as suggested
Unresolved issues resolved. Making fasttrack again.