Community
Participate
Working Groups
The activateEditor() method is not completely implemented in org.eclipse.ui.part.AbstractMultiEditor: public void activateEditor(IEditorPart part) { activeEditorIndex = getIndex(part); E4Util.unsupported("We need to request an activation of this part"); } This problem is causing all editors that subclass from AbstractMultiEditor to fail.
It looks like this functionality requires making the AME MPart contain inner MParts, and then insure we deal fairly with the inner MPart in the rest of the workbench. I don't feel we can address this in 4.2.2, so I have deferred it to 4.3 PW
I can take a look at it if no one else is already doing that.
First, I thought that adding something like MCompoundPart to the model as an extension of MPart with the getInnerParts() method is a good option. But on second thought, since we don't want to pollute e4 workbench model, this change could be too big and we should try to implement it somehow using the existing model.
Szymon, you're on the right track here but there's no need for a 'getInnerParts' method; all we have to do is create a new element MCompoundPart which is both an MPart and an MPartSashContainer. Then you can use the EModelService's 'findElements' method to get the 'inner' parts... I expect to add this right after we release M6 so that I have a chance of getting 'split' parts working in Kepler at least to demo them. Also note that while doing the split parts work I may introduce a new CompoundPartImplementation class which would be an implementation class that knows enough to manage its children's state (i.e. it will listen to its children's 'dirty' state and keep itself up to date accordingly, forward on @focus calls...)
While working on that I found another possible solution. Maybe instead of a new MCompoundPart we could make MPart extend MPartSashContainer and allow all parts to have inner parts? Then, only the parts that actually have any inner parts like multi-editors would use it. Eric, what's your opinion?
(In reply to comment #5) > While working on that I found another possible solution. Maybe instead of a > new MCompoundPart we could make MPart extend MPartSashContainer and allow > all parts to have inner parts? Then, only the parts that actually have any > inner parts like multi-editors would use it. The rationale behind this change is that we first create MPart and then appropriate CompatibilityEditor, so at the time of MPart creation we don't know if the editor will be a multi-editor or not. If any MPart could contain other MParts, we wouldn't need to worry about it. Otherwise, before MPart creation we would need to check what will be "kept" in that MPart and then create either a simple MPart or the new MCompoundPart. This additional checking sounds like a step backwards to me. For reference see org.eclipse.ui.internal.WorkbenchPage.busyOpenEditor(IEditorInput, String, boolean, int, IMemento, boolean) line 3122.
Szymon, The main problem with allowing *all* parts to be sash containers is that it means that our tree has no definable 'leaf' nodes. Is there a real-world scenario where you wouldn't know at the time of creating the 'root' part whether or not it would contain child parts ? Even if true wouldn't you have to change the root part's implementation when it finally determines that is does indeed have child parts (this will likely be necessary to handle both activation and the save cycle) ? It seems that we should be able to determine from the editor whether it needs inner parts *before* creating the MPart no ? In any case it should be possible to add a new method to the model service that will turn an existing MPart into an MCompoundPart. This is what I expected to do when implementing the 'split part' code. We'd originally have a simple MPart and the 'split me' code would replace the original MPart with an MCompoundPart whose implementation is capable of managing the children. This would act very much like the current 'insert' method that does the model changes necessary to split a stack into two stacks (eg. when dragging a view to split a stack).
(In reply to comment #7) > Szymon, The main problem with allowing *all* parts to be sash containers is > that it means that our tree has no definable 'leaf' nodes. Thanks for your prompt response, Eric. Yes, I thought that a part with no children could be considered as 'leaf'. Is there any side effect of no definable 'leaf' nodes? > Is there a real-world scenario where you wouldn't know at the time of > creating the 'root' part whether or not it would contain child parts ? I think this is what is happening in org.eclipse.ui.internal.WorkbenchPage.busyOpenEditor(IEditorInput, String, boolean, int, IMemento, boolean) line 3122. We know only the editorId, so we don't know if it is a multi-editor until we instantiate it. And that happens later in line 3125 when we call partService.showPart(editor, PartState.VISIBLE). > Even > if true wouldn't you have to change the root part's implementation when it > finally determines that is does indeed have child parts (this will likely be > necessary to handle both activation and the save cycle) ? It seems that we > should be able to determine from the editor whether it needs inner parts > *before* creating the MPart no ? To determine if an editor needs inner parts we need to instantiate the editor or editor input and check it with instanceof. Checking editor can be done in CompatibilityEditor which is after we already have an MPart. Checking against editor input can be done earlier (WorkbenchPage.busyOpenEditor line 3124) but we need EditorReference for that which in turn needs the MPart. So the question is how can we create EditorReference if it requires MPart and the type of MPart (compound or not) requires output from EditorReference? > In any case it should be possible to add a new method to the model service > that will turn an existing MPart into an MCompoundPart. This is what I > expected to do when implementing the 'split part' code. We'd originally have > a simple MPart and the 'split me' code would replace the original MPart with > an MCompoundPart whose implementation is capable of managing the children. > This would act very much like the current 'insert' method that does the > model changes necessary to split a stack into two stacks (eg. when dragging > a view to split a stack). Coverting existing MPart into an MCompundPart sounds promising. I will need to check how it works.
Hmm, I guess I need some coffee... I can see now that we can create/convert relevant MPart based on the editor input which is already available there.
Szymon, thanks for your excellent analysis of the ordering issues with openEditor...in Luna we should take a look to see whether we can re-organize the flow so that we can at least determine 'early' whether or not the result is going to be compound or not. Note that we have the opportunity to adjust the model right up to the point where we add the MPart (compound or not) into the model, likely an MPartStack but from what you show I suspect that we should be able to make this somewhat more ordered. In order to actually get a 'proper' implementation of MPE's we'll likely have to extend our current concept of 'active part' since it may now be a chain. How this affects operations such as 'activatePart'; if the part is be one of the 'pages' in the MPE then there may also be a necessity of 'activating' the outer 'root' part. Not sure how much of a problem this will represent, it's just something to keep in mind...
(In reply to comment #10) > Szymon, thanks for your excellent analysis of the ordering issues with > openEditor...in Luna we should take a look to see whether we can re-organize > the flow so that we can at least determine 'early' whether or not the result > is going to be compound or not. I think I already have that part: MPart editor = ((PartServiceImpl) partService).createPart( CompatibilityEditor.MODEL_ELEMENT_ID, input instanceof MultiEditorInput); where the createPart method has additional param isCompound to create either MPart or MCompoundPart. It's simple and works for now. > Note that we have the opportunity to adjust the model right up to the point > where we add the MPart (compound or not) into the model, likely an > MPartStack but from what you show I suspect that we should be able to make > this somewhat more ordered. > > In order to actually get a 'proper' implementation of MPE's we'll likely > have to extend our current concept of 'active part' since it may now be a > chain. How this affects operations such as 'activatePart'; if the part is be > one of the 'pages' in the MPE then there may also be a necessity of > 'activating' the outer 'root' part. Not sure how much of a problem this will > represent, it's just something to keep in mind... For now I'm trying the simplest case so that at least MultiEditorTestSuite is working. Then I will switch to some real editor implementation to fix remaining issues. At the moment I'm trying to understand why partOpened(...) is called before CompatibilityEditor.createPartControl(...) is finished. Is it really fine to subscribe event handlers before createPartControl is called? Or maybe there is some 'rule' that model cannot be modified inside createPartControl?
(In reply to comment #11) > For now I'm trying the simplest case so that at least MultiEditorTestSuite > is working. It seems there might be something wrong with these tests. See bug 366522 comment 3. I will do some cleanup and share what I've got so far.
Created attachment 230640 [details] Patch v.0.1 First attempt to handle multieditors.
Patch v.0.1 gives an idea how it can look like. At the moment the inner editors are rendered twice and on the wrong composite, probably because of wrong usage of context.
Created attachment 234032 [details] Patch v.0.2 Updated patch to use the new model element.
Will this bug be fixed in the next eclipse release?
We should review how far we got in Mars (4.5) PW
Created attachment 249623 [details] Patch v.0.3 I updated the previous patch to solve the duplicate inner editors problem present in previous patches.
Created attachment 249624 [details] Patch v.0.3
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. If the bug is still relevant, please remove the stalebug whiteboard tag.
