Community
Participate
Working Groups
Eventually, SubsystemConfiguration, SubSystem and releated classes should move from UI to Core. There are multiple steps to be done: * Get rid of unnecessary methods requiring UI * Move NLS Strings from UI to Core * Refactor interfaces that require UI
Created attachment 69527 [details] Patch for stage 1: Removing unnecessary UI dependencies Attached patch does stage 1 by - Removing unnecessary UI dependencies from SubsystemConfiguration, which are just introduced by obsolete methods - Update Javadoc to refer to "subsystem configuration" rather than "factory" Following obsolete methods are removed from SubsystemConfiguration, or moved into SubsystemConfigurationAdapter; where moved, a corresponding delegate method did already exist in the Adapter, it was just not completely migrated yet. * ServiceSubSystemConfiguration.getAdditionalFilterActions(ISystemFilter, Shell) -- moved code into SubSystemConfigurationAdapter; was called from there only * SubSystemConfiguration.getImage(), getLiveImage() * SubSystemConfiguration.getGraphicsImage(), getGraphicsLiveImage() * SubSystemConfiguration.imageTable -- moved code into SubSystemConfigurationAdapter; was called from there only * SubSystemConfiguration.getServerLauncherForm() * SubSystemConfiguration.getSubSystemPropertyPageCoreFrom() * SubSystemConfiguration.getSubSystemPropertyPages() * SubSystemConfiguration.getUserIdValidator() -- moved code into SubSystemConfigurationAdapter; was called from there only * SubSystemConfiguration.supportsUserDefinedActions(ISelection) -- Removed obsolete code that was nowhere called
Comment on attachment 69527 [details] Patch for stage 1: Removing unnecessary UI dependencies Dave's and Kushal - can you please review and comment on attached patch. IMHO this is a totally riskless change, preparing for moving SubSystemConfiguration into non-UI. Most changes are actually Javadoc fixes or moving stuff from the config into the adapter where it has been called from the adapter only before. I'd like to get this done for 2.0RC2 -- in preparation for finishing this work for 3.0 with fewer API changes.
I'm all for moving the subsystem configuration to core if it can be done.
I'm checking in the changes for stage 1 because I'm very confident that this is what we want. This also gives you the chance to review in Team Synchronize View. If you disagree, I can back it out again. It just affects 3 files: [190231][api] Remove unnecessary UI dependencies from SubSystemConfiguration SubSystemConfiguration SubSystemConfigurationAdapter ServiceSubSystemConfiguration
This looks like a lot less of a problem than I anticipated. Go for it.
Two more items for stage 1a: - Remove SubSystemConfiguration.configureNewFilterAction() --> This was apparently moved to ISubSystemConfigurationAdapter already, but forgotten to take out of SubSystemConfiguration - Refactor:move ISubSystemPropertiesWizardPage from rse.ui.wizards to rse.core.subsystems Committed: [190231][api] Move ISubSystemPropertiesWizardPage from UI to Core And one more optionals item (stage 1b): - Refactor:move ISystemNewConnectionWizardPage from rse.core.model to rse.core.subsystems I think it would better fit there because it is closely tied to subsystem configurations and as such extends the plain model objects This one is not yet committed, what do you think?
Stage 2 (safe) would be adding a non-UI proxy to SystemPreferenceManager.getShowFilterPools(): Basically, add methods RSECorePlugin.setShowFilterPools() RSECorePlugin.getShowFilterPools() and install a Preference Change listener in UI such that the UI Preference is set in the Core plugin Stage 3 (need discussion!) would be to decide if it's OK to not show a SystemMessageDialog on two occasions: - getSystemFilterPoolForBrokenReference() --> Currently shows a dlg but also logs and stores messages in brokenReferenceWarningsIssued, so clients could get the warnings from there - Exception in getFilterPoolManager() --> Currently shows a dialog, we could just log the exception instead Stage 4 (need discussion!) would be moving Logging and NLS Strings from UI to Core. Currently, SubSystemConfiguration uses the following from UI: ISystemMessages.MSG_LOADING_PROFILE_SHOULDBE_ACTIVATED ISystemMessages.MSG_CONNECTWITHPORT_PROGRESS ISystemMessages.MSG_CONNECT_PROGRESS ISystemMessages.MSG_DISCONNECTWITHPORT_PROGRESS ISystemMessages.MSG_DISCONNECT_PROGRESS SystemPropertyResources.RESID_PROPERTY_FILTERTYPE_VALUE SystemResources.RESID_NEWFILTER_PAGE2_PROFILE_LABEL SystemResources.RESID_NEWFILTER_PAGE2_PROFILE_TOOLTIP SystemResources.RESID_NEWFILTER_PAGE2_PROFILE_VERBIAGE I'm not sure if the NLS Strings can be moved or duplicated in Core. If we REALLY want this, one option would be to have the UI set the corresponding NL Strings in the Core when it starts up. This would complete the work on SubSystemConfiguration. I need your comments on Stage 1b, Stage 3 and Stage 4. Stage 1b - Should we move ISystemNewConnectionWizardPage from rse.core.model to rse.core.subsystems? Stage 3 - DaveM: Can we safely get rid of the messae dialogs and only log the errors? Stage 4 - DaveD: Can we move NLS Strings out of SystemMessages and into Core Resources? Or should we wait with this for 3.0? Or should we inject them from UI into Core?
(In reply to comment #7) > I need your comments on Stage 1b, Stage 3 and Stage 4. > Stage 1b - Should we move ISystemNewConnectionWizardPage > from rse.core.model to rse.core.subsystems? I don't think it belongs in the model, as long as we're reorganizing it should go in the right spot. > Stage 4 - DaveD: Can we move NLS Strings out of SystemMessages and into > Core Resources? Or should we wait with this for 3.0? Or should > we inject them from UI into Core? Move the strings. I'll catch heat for it, but I don't like the idea of injection.
(In reply to comment #7) I would like to move forward with this change for 3.0M6. Attached Patch 1 was already committed, but there is more work pending according to comment #7. Since it involves NLS String Relocations, it looks like it should be complete before the PII Freeze on Feb.25. Stage (1b) seems not applicable any more, but please look at stages 3 and 4: Stage 3 - DaveD, DaveM: Can we safely get rid of the message dialogs and only log the errors? Stage 4 - DaveD, DaveM: Can we move NLS Strings out of SystemMessages and into Core Resources? How does this relate to the message ID's? Can we just make normal NLS strings out of the messages, or should we create proxies of the UI-Messages as a copy in non-UI like we've been doing for the messages in RemoteFileCancelledException and similar?
(In reply to comment #9) > (In reply to comment #7) > I would like to move forward with this change for 3.0M6. Attached Patch 1 was > already committed, but there is more work pending according to comment #7. > Since it involves NLS String Relocations, it looks like it should be complete > before the PII Freeze on Feb.25. > > Stage (1b) seems not applicable any more, but please look at stages 3 and 4: > > Stage 3 - DaveD, DaveM: Can we safely get rid of the message dialogs and only > log the errors? > > Stage 4 - DaveD, DaveM: Can we move NLS Strings out of SystemMessages and into > Core Resources? How does this relate to the message ID's? Can we > just make normal NLS strings out of the messages, or should we create > proxies of the UI-Messages as a copy in non-UI like we've been doing > for the messages in RemoteFileCancelledException and similar? > Regarding 1b. I'd really like to move the wizard pages out of core back to the UI. I think the subsystem creation in the registry and subsystem configuration objects should be working off properties objects of some sort rather that directly accessing the pages. The pages could adapt to some sort of properties object. I believe this gets rid of one of the last UI dependencies in the core plugin. Regarding 3. Instead of just logging errors we might want to have a listener that can listen for them or perhaps better, just return IStatus objects that have these SystemMessages (or some equivalent) as data.
Created attachment 95785 [details] Patch for stage 2: Adding an IRSEInteractionProvider This patch brings in the following new API: org.eclipse.rse.core.IRSEInteractionProvider org.eclipse.rse.core.IRSERunnableWithProgress RSECorePlugin#setDefaultInteractionProvider() RSECorePlugin#getDefaultInteractionProvider() SubSystem#setInteractionProvider() SubSystem#getInteractonProvider() As well as the following new internal impl: org.eclipse.rse.internal.ui.DefaultUIInteractionProvider Committing this API as "experimental" for 3.0M6, to be refined in M7.
Committed: [190231] Prepare API for UI/Non-UI Splitting
Not for M7
Didn't get to this in RC1
Hopefully the APIs are in place already for making this non-breaking in 3.1 -- package names are correct already, so if implementation classes move to Core, that should be sufficient (pending the question whether we'd want to reexport the Core package in the UI plugin -- we'd probably have to.
