Community
Participate
Working Groups
This is a spawn-off of bug 259411, comment 0, bullet 4. From bug 259411, comment 3: "The patch removes constructor of CompilationUnitEditorAdapter and makes the adapter create flow be more like a flow of a standalone editor ie the editor part is initialised first then the UI is created. So far in Java Compare it happened the other way round, a source viewer was created and set on the editor, then editor was initialised with the given input. Now, created source viewer is not set on the editor as long as the editor is not init'ed with an input. Then the createPartControl(SourceViewer) method is called which sets the viewer and configures it. This fixes the issue with the preference store being explicitly set, also it simplifies the code imo." I'm not an Editor/UI expert so I would be delighted to hear if the above makes any sense.
Created attachment 131400 [details] Patch v01
The patch causes tons of NPEs because createPartControl(Composite parent) is never called ;-) The general idea listed in comment 0 is not too bad but you need to ensure that createPartControl(Composite parent) gets called and you should not create the viewer in JavaMergeViewer.getSourceViewerConfiguration(SourceViewer, IEditorInput).
Created attachment 131915 [details] Patch v02 I'm sorry Dani but I don't understand why calling createPartControl(Composite) is obligatory. My idea was to call createPartControl(SourceViewer) instead just after the editor had been initialized. The new method doesn't create the viewer (it's created in createSourceViewer(Composite, int) method. The new createPartControl method only sets the viewer and configures it (a small part of what the original does). I don't see any NPEs but maybe I missed something in the previous patch so I send an updated one (with changes made in bug 271757). One more thing I found is the way we're dealing with JavaSourceViewerConfiguration in getSourceViewerConfiguration when an editor input is available. Just after the editor is init'ed and actions are created we instantiate a new object of JavaSourceViewerConfiguration. This could be avoided by simply getting the source viewer configuration from the editor (JavaEditor sets JavaSourceViewerConfiguration during editor's initialization).
Created attachment 131916 [details] mylyn/context/zip
>I'm sorry Dani but I don't understand why calling createPartControl(Composite) >is obligatory. Mmh, maybe because lots of things are initialized in there? Where do you do it now? Copy all the code? >I don't see any NPEs Did you work with it or just launch once?
Sorry that I didn't post the NPEs. I can't reproduce now because the patch no longer applies. Oh-oh: I now see that createPartControl(Composite parent) is not calling super. This explains why features like auto-closing brackets is not working. Also, if we called super and override createJavaSourceViewer(...) we could get rid of the ugly setSourceViewer(...) method that uses reflection. Tomasz, do you know why we don't do this? Regarding the latest patch: I still don't like to create the editor/viewer inside getSourceViewerConfiguration(...) ;-)
(In reply to comment #6) > Sorry that I didn't post the NPEs. I can't reproduce now because the patch no > longer applies. Please try the updated version of the patch (Patch v02). > Oh-oh: I now see that createPartControl(Composite parent) is not calling super. > This explains why features like auto-closing brackets is not working. Also, if > we called super and override createJavaSourceViewer(...) we could get rid of the > ugly setSourceViewer(...) method that uses reflection. > > Tomasz, do you know why we don't do this? Compare Editor first asks for a source viewer, then when the input is available we set it to our editor adapter, set the viewer on the editor and configure it. If we called createPartControl(Composite parent) and super when asked for a viewer we would have to do it without a meaningful input. > Regarding the latest patch: I still don't like to create the editor/viewer > inside getSourceViewerConfiguration(...) ;-) It's just a name :) What happens in the createPartControl method (no matter if it's the old one with Composite as parameter or the one from the patch, with SourceViewer) has nothing to do with creating editor nor viewer. As mentioned in comment 3, "the method only sets the viewer and configures it (a small part of what the original does)". I know the name is confusing ;)
>Please try the updated version of the patch (Patch v02). No NPE there so far. >If we called createPartControl(Composite parent) and super when asked for a >viewer we would have to do it without a meaningful input. Since we call createPartControl we could probably fix this, couldn't we? >It's just a name :) Then change it ;-)
Created attachment 132601 [details] Patch v03 An attempt to call createPartControl from the Java Editor when asked for a source viewer. A change has been made in TextMergeViewer so the input is prepared just before building the UI. Unfortunately this doesn't work as expected, the Compare Editor opens but the Java Merge Viewer is blank. This may be caused by the fact that I'm trying to embed composites containing a little bit more than a single viewer.
Created attachment 132602 [details] Embedding Java Editor This is a dirty patch that shows how the above could look like. This is an example of an editor made of two Java Editors.
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. As such, we're closing this bug. If you have further information on the current state of the bug, please add it and reopen this bug. 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. -- The automated Eclipse Genie.
