Bug 190271 - [api][breaking] Move ISystemViewInputProvider to Core and have SystemRegistry implement it
Summary: [api][breaking] Move ISystemViewInputProvider to Core and have SystemRegistry...
Status: RESOLVED FIXED
Alias: None
Product: Target Management
Classification: Tools
Component: RSE (show other bugs)
Version: 2.0   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: 2.0   Edit
Assignee: Martin Oberhuber CLA
QA Contact: Martin Oberhuber CLA
URL:
Whiteboard:
Keywords: api
Depends on:
Blocks:
 
Reported: 2007-05-31 12:35 EDT by Martin Oberhuber CLA
Modified: 2008-08-13 13:20 EDT (History)
4 users (show)

See Also:
dmcknigh: review+


Attachments
Patch moving ISystemViewInputProvider to Core (68.24 KB, patch)
2007-06-05 13:47 EDT, Martin Oberhuber CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Oberhuber CLA 2007-05-31 12:35:17 EDT
+++ This bug was initially created as a clone of Bug #190195 +++

We've seen some regressions due to splitting SystemRegistry vs. SystemRegistryUI. Particularly, some views compare event.getParent() against their input provider -- which is bound to fail due to the split.

The problem could be fixed by moving ISystemViewInputProvider to Core, and making SystemRegistry implement it. What's making this problematic is the getViewer() / getShell() methods on the ISystemViewInputProvider.

A quick fix could be done by changing their signature to get and pass java.lang.Object, but in the longer term these should be removed.

For details, see bug #190195 comment 2 and bug #190195 comment 5.

Build ID: Eclipse3.3 RSE2.0rc1

Steps To Reproduce:
1. enable the "show new connection in system view" preference
2. the new connection prompt will not get visible in the system view


More information:
to me it looks like the UI/NonUI splitting of the system registry caused this problem.
in get/hasChildren of SystemViewRootInputAdapter the new conenction prompt is only added when the provider is a ISystemRegistry.
in fact, when debugging this code, the provider that is used is always a ISystemRegistryUI.
changing this interface to ISystemRegistryUI re-enables the new connection prompt.
Comment 1 Martin Oberhuber CLA 2007-06-05 13:47:04 EDT
Created attachment 70182 [details]
Patch moving ISystemViewInputProvider to Core

Please review attached patch. It's a sizeable patch but rather simple:
* Change signature of set/getViewer/Shell() to "Object"
* Move it to Core
* Have ISystemRegistry implement it rather than ISystemRegistryUI
* Change the corresponding instanceof expressions

Although it's late for such a sizeable patch, I expect RSE to be more reliable with it because the original assumption of SystemRegistry and the InputProvider being the same is restored.
Comment 2 Martin Oberhuber CLA 2007-06-05 13:48:10 EDT
Dave can you please review this ASAP? - I'd like to see it in RC2 if possible.
Comment 3 David McKnight CLA 2007-06-05 15:13:03 EDT
I'm not sure I understand why the signature for getShell() is being changed to return an Object.  Could you explain that?  Is that what we want in the end or are we just changing the signature to get around something?
Comment 4 Martin Oberhuber CLA 2007-06-05 15:23:21 EDT
We need to move it to Core so that ISystemRegistry can implement it.
In Core, we do not know Shell or Viewer.
So, the signature is changed to Object for now.

All methods that have the "workaround" Object signature are deprecated so they should be removed eventually.
Comment 5 Martin Oberhuber CLA 2007-06-05 16:38:53 EDT
Dave do you think it's safe to commit this?
Comment 6 David McKnight CLA 2007-06-05 16:50:23 EDT
I didn't see the "workaround" Object signature.  Are you planning on adding that?   Anyway this looks safe for now.
Comment 7 Martin Oberhuber CLA 2007-06-05 16:57:36 EDT
The signature change is hard to see in the diffs, because the file was moved.

It used to be
   setShell(Shell shell)
   setViewer(Viewer viewer)
but now it is
   setShell(Object shell)
   setViewer(Object viewer)

The corresponding getters are also changed (return value). The getters are deprecated, since I'd like to encourage clients to use other methods for getting the viewer or shell.

Anyways, thanks for the review, this is the last change I'm accepting for RC2 so I can kick it off now.
Comment 8 Martin Oberhuber CLA 2007-06-05 17:02:00 EDT
Patch committed, changing 
   org.eclipse.rse.core
   org.eclipse.rse.subsystems.files.core
   org.eclipse.rse.ui

[190271][api][breaking] Move ISystemViewInputProvider to Core

Migration Docs:
---------------
Organize Imports. If there are compile errors on setShell() getShell(), setViewer() getViewer(), get rid of those methods since they are deprecated.
Comment 9 David McKnight CLA 2007-06-05 17:37:04 EDT
I saw the signature changes... I was just thinking there was supposed to be "workaround" keyword or something for the changed signatures.
Comment 10 Martin Oberhuber CLA 2008-08-13 13:20:31 EDT
[target cleanup] 2.0 RC2 was the original target milestone for this bug