Bug 544838 - Add an option to automatically add required features/plugins upon lauching
Summary: Add an option to automatically add required features/plugins upon lauching
Status: VERIFIED FIXED
Alias: None
Product: PDE
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.11   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: 4.24 M1   Edit
Assignee: Hannes Wellmann CLA
QA Contact:
URL:
Whiteboard:
Keywords: noteworthy
Depends on: 351396 576860 576888 576889 576890
Blocks: 570760
  Show dependency tree
 
Reported: 2019-02-26 12:29 EST by Rolf Theunissen CLA
Modified: 2022-05-17 18:53 EDT (History)
4 users (show)

See Also:


Attachments
Option 1 (23.85 KB, image/png)
2022-03-27 07:20 EDT, Hannes Wellmann CLA
no flags Details
Option 2 (19.64 KB, image/png)
2022-03-27 07:20 EDT, Hannes Wellmann CLA
no flags Details
Option 3 (18.85 KB, image/png)
2022-03-27 07:21 EDT, Hannes Wellmann CLA
no flags Details
Option 4 (18.96 KB, image/png)
2022-03-27 07:21 EDT, Hannes Wellmann CLA
no flags Details
Option 5 (34.13 KB, image/png)
2022-03-27 07:21 EDT, Hannes Wellmann CLA
no flags Details
Option 5-2 (57.71 KB, image/png)
2022-03-28 16:50 EDT, Hannes Wellmann CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Rolf Theunissen CLA 2019-02-26 12:29:30 EST
Currently, to have a correct launch configuration, all required features/plugins must be selected.

Often, defining a launch configuration starts by selection a few plug-ins and/or features. After that all required plugins/features are added (automatically by pushing the "Add required plug-ins/Select Required" buttons.

When over time the required plugins/features change, the launch configuration must be adapted to incorporate the changes as well.

Enhancement: Add an option to add required features/plugins automatically when a target is launched. These added requirements should not be persisted in the launch target. In this way, only a few plugins/features are selected, and still the minimal number of required plugins are added.

This would be similar to the "plugin" and "feature" properties as defined by the LcDSL (https://github.com/mduft/lcdsl). Note that also supporting the "ignore" property would have even more added value, in some use-cases.
Comment 1 Julian Honnen CLA 2021-07-28 02:09:37 EDT
As discussed in bug 570760, Hannes plans to implement this option.

The option would be available for
* plugin-based launches (off by default) and
* feature-based launches (on by default) where it would essentially keep the
  current behavior of automatically including all requirements.
  Disabling that option for feature-based launches would make the launch
  configuration strictly include only the features' contents.

TBD: should feature imports ("Dependencies") be ignored in this strict mode?
Comment 2 Hannes Wellmann CLA 2021-07-28 03:54:44 EDT
Thanks Julian for starting the discussion here and summarizing the other one.

(In reply to Julian Honnen from comment #1)
> As discussed in bug 570760, Hannes plans to implement this option.
> 
> The option would be available for
> * plugin-based launches (off by default) and
> * feature-based launches (on by default) where it would essentially keep the
>   current behavior of automatically including all requirements.
>   Disabling that option for feature-based launches would make the launch
>   configuration strictly include only the features' contents.
> 
> TBD: should feature imports ("Dependencies") be ignored in this strict mode?

I tend to say, that in 'strict mode' it should only be included what is explicitly specified and dependencies should not be included automatically. This would be consistent to plug-in based launches.

Nevertheless I consider the content of a feature (plug-ins AND nested features) as part of it and therefore a feature's content should be included in strict mode as well.
Comment 3 Julian Honnen CLA 2021-07-28 03:59:08 EDT
(In reply to Hannes Wellmann from comment #2)
> I tend to say, that in 'strict mode' it should only be included what is
> explicitly specified and dependencies should not be included automatically.
> This would be consistent to plug-in based launches.
> 
> Nevertheless I consider the content of a feature (plug-ins AND nested
> features) as part of it and therefore a feature's content should be included
> in strict mode as well.
I think we agree: only the included plugins and features should be included in strict mode but neither the plugin nor feature imports (<requires> node).
Comment 4 Eclipse Genie CLA 2021-10-22 12:49:13 EDT
New Gerrit change created: https://git.eclipse.org/r/c/pde/eclipse.pde.ui/+/186833
Comment 5 Eclipse Genie CLA 2022-03-27 06:50:45 EDT
New Gerrit change created: https://git.eclipse.org/r/c/pde/eclipse.pde.ui/+/192272
Comment 6 Hannes Wellmann CLA 2022-03-27 07:19:23 EDT
The implementation is ready for me, but I'm not yet sure which is the best way to present the new option in the PLugin-Tab of the Launch config UI.

One way would be to add the check-box on the right hand side of the Plugins tab.
But unfortuonatly the horziontal space is not unlimitted and IIRC it is not possible to break labels in SWT is it?
I have created four variants for this way with different texts:
 1 - Add Requirements automatically
 2 - Add Requirements
 3 - Auto Add Required
 4 - Auto Add Requirements
A tool-tip with a more verbose description is added.
At the moment my favorit is option 4 because it is the most precise while requires only a little extra horizontal space. Option 3 is IMHO second best.
If line breaks where possible (or I could exclude the label from the parents width computation easaly so that the labels text is broken accorindly I would be in favour of option 1).

Another way would be to add the checkbox at the bottom (option 5) where more than enough horizontal space is available.
But this way the checbox is a bit far a way from the "Add Required Plugins" Button and therefore some kind of hidden.
And furthermore it takes 'one line' additional vertical space which is IMHO worse than a tiny bit of horizontal space.

Please find screenshots of all five options attached and have a look at the code if you are interested.
Comment 7 Hannes Wellmann CLA 2022-03-27 07:20:09 EDT
Created attachment 288298 [details]
Option 1
Comment 8 Hannes Wellmann CLA 2022-03-27 07:20:34 EDT
Created attachment 288299 [details]
Option 2
Comment 9 Hannes Wellmann CLA 2022-03-27 07:21:00 EDT
Created attachment 288300 [details]
Option 3
Comment 10 Hannes Wellmann CLA 2022-03-27 07:21:15 EDT
Created attachment 288301 [details]
Option 4
Comment 11 Hannes Wellmann CLA 2022-03-27 07:21:46 EDT
Created attachment 288302 [details]
Option 5
Comment 12 Rolf Theunissen CLA 2022-03-27 09:58:23 EDT
Plugin/bundle based the button is called 'Add Required plug-ins'
Feature based it is called 'Select Required' [features]

This by itself is inconsistent. Together with 'Select all'/'Deselect all', 'Select' would be the preferred label, though this eats more space.

The new selection would not select additional elements in the tree. i.e. I expect that the additional elements are not selected next time the launch configuration is used (is this correct?). Therefore I would prefer to call it something like 'Include Required [Plug-ins/Features]'.

Moreover, when included at the right hand side, I kind of expect that the 'Add/Select required plug-ins/[features]' would be disabled. But there might be cases that both functionalities are needed. When included at the bottom, this would be less confusing. That is, buttons on the right hand side operate on the selection in the tree. The selection in the bottom do additional configuration on top of what is selected in the tree.

For option 5 I would not call it 'Add missing required', in this case 'missing' refers to unselected which has IMHO a different meaning.

In the Plugin selection the option 'Include optional dependencies ...' is available. For consistency, also here either refer to 'dependencies' or to 'features/plug-ins'.
Also, when adding this new options to the right hand side, the 'include optional ...' should be moved to the right hand side, as this options has impact on the behavior of the 'Add Required Plug-ins'. And the new option too, I assume. That makes me wonder too, how about optional dependencies in the feature case?

I would suggest to use option 5 with this label:
  'Include required dependencies', or 
  'Include required dependencies automatically when launching'.
Or when the selection is saved:
  'Add required dependencies automatically prior to launching'

Finally, a 6th option would be to add entries to the 'Launch with:' drop down, e.g.:
  'features/plug-ins selected below, including dependencies'
(Also notice another inconsistency w.r.t. casing, here 'plug-ins' bottom labels 'Plug-ins', buttons use Camel Case).
Comment 13 Hannes Wellmann CLA 2022-03-27 16:51:23 EDT
> The new selection would not select additional elements in the tree. i.e. I
> expect that the additional elements are not selected next time the launch
> configuration is used (is this correct?). Therefore I would prefer to call
> it something like 'Include Required [Plug-ins/Features]'.

This is correct. If the check-box (currently) named "Auto Add Required" is checked the bundles/features required by the selected bundles/features, that are not selected, are added to the set of launched bundles while preparing the application to launch. But those added requirements are not selected in the tree.

Your suggestion to therefore name it 'include' sounds good to me. Nevertheless I think we should still have 'Auto' in the name to make it clear it happens automatically, combined it would be "Auto Include Required".


> Plugin/bundle based the button is called 'Add Required plug-ins'
> Feature based it is called 'Select Required' [features]
> 
> This by itself is inconsistent. Together with 'Select all'/'Deselect all',
> 'Select' would be the preferred label, though this eats more space.

That's right those label are indeed inconsistent and it should be changed to "Select Required". Without 'Plug-ins' it also does not require more space and is the same for features.
It should be mentioned that the "Select Required" in the Feature blog only adds required features, so it can happen that the validation still fails after that button is pressed because required bundles are not in a required/included feature.

Actually the label "The features selected below and all required plug-ins will be launched" can be removed. This is outdated (for example included and required features are considered as well) and with this new button the behaviour can be altered. And I think it should not be explained here how this works.

> 
> 
> Moreover, when included at the right hand side, I kind of expect that the
> 'Add/Select required plug-ins/[features]' would be disabled. But there might
> be cases that both functionalities are needed.

Exactly. For example one could want to really select the requirements. 

> When included at the bottom,
> this would be less confusing. That is, buttons on the right hand side
> operate on the selection in the tree. The selection in the bottom do
> additional configuration on top of what is selected in the tree.

This separation is unfortunately not done completely in this distinct way.
For example "Include optional dependencies when computing required Plug-ins" controls what happens when selecting requirements (as you mentioned below), while for example "Add new workspace Plug-ins ... " acts similarly to the new option. And for the Feature selection "Use features from workspace ..." also influences which features are selected during the launch of an application. So this is kind of similar.
So in order to separate the check-boxes more strictly "Include optional dependencies when computing required Plug-ins" should actually be moved to the right side. But I assume when this wizard was designed these considerations where not made and the checkboxes where just placed there below because then they can occupy more horizontal space.

> 
> For option 5 I would not call it 'Add missing required', in this case
> 'missing' refers to unselected which has IMHO a different meaning.

"Missing" could indeed be misleading and "Unselected" probably be a better name for this option.

> 
> In the Plugin selection the option 'Include optional dependencies ...' is
> available. For consistency, also here either refer to 'dependencies' or to
> 'features/plug-ins'.

That's right. But the new option also includes plug-ins required by the launched product/application. Therefore I wanted to use the broader term 'requirements' because everything that is required is included not only dependencies of features/plug-ins. But even in the scope of bundles/feature 'requirements' and 'dependencies' are interchangeable, at least they are often used as such and I could not tell an exact difference. If it should be consistent with the other check-box I would rater adjust that.

> Also, when adding this new options to the right hand side, the 'include
> optional ...' should be moved to the right hand side, as this options has
> impact on the behavior of the 'Add Required Plug-ins'. And the new option
> too, I assume.

'include optional ...' has an impact on the new option in case of a plug-in based launch, yes.
And I agree that 'include optional ...' would be better placed at the right, but I think it's hard to find a expressive and clear short name for that.

> That makes me wonder too, how about optional dependencies in
> the feature case?

In the case of features optional dependencies are not considered.
But if there is a demand it should be doable. However it would only affect the computation of required plug-ins contained in the included features because for features dependencies/requirements cannot be optional.
In general the intention with this and many of my previous changes is to make feature-based launches behave similar to the product that is build by Tycho. So it would be interesting to know how Tycho handles optional requirements?

@Chrstoph do you know that? (and maybe you are also interested in this change).


> 
> I would suggest to use option 5 with this label:
>   'Include required dependencies automatically when launching'.
Besides that I think that 'requirements' would be more suitable here as mentioned above this is a good suggestion for this option.

> 
> Finally, a 6th option would be to add entries to the 'Launch with:' drop
> down, e.g.:
>   'features/plug-ins selected below, including dependencies'

To be honest I'm not convinced this a better option than the others.

> (Also notice another inconsistency w.r.t. casing, here 'plug-ins' bottom
> labels 'Plug-ins', buttons use Camel Case).

The drop-down elements should indeed all start with an upper-case.

I have created Bug 579433 to track the corrections of the label inconsistencies you mentioned.
Comment 14 Julian Honnen CLA 2022-03-28 03:36:47 EDT
(In reply to Rolf Theunissen from comment #12)
> I would suggest to use option 5 with this label:
>   'Include required dependencies automatically when launching'.
+1
Comment 15 Hannes Wellmann CLA 2022-03-28 16:50:01 EDT
Created attachment 288306 [details]
Option 5-2
Comment 16 Hannes Wellmann CLA 2022-03-28 16:59:49 EDT
Since you both were in favour of Option 5 I updated the change accordingly and used for Plug-in based launches the label

"Include not selected but required Plug-ins automatically when launching"

respectively for Feature-based launches:

"Include not selected but required Features and Plug-ins automatically when launching"

For Equinox-Applications the same label is used with "Plug-ins" replaced by "Bundles".

I think this reflects quite well that it includes dependencies and application requirements.
Please see the attached new Screenshot for Option 5-2 (the term "while" in the screenshot was replaced by "when" in the meantime).
Comment 17 Julian Honnen CLA 2022-03-29 02:12:51 EDT
Looks OK to me, though at first glance I would have reduced it to "Include required * automatically..." - isn't "not included" obvious?
Comment 18 Vikas Chandra CLA 2022-03-29 02:47:20 EDT
(In reply to Julian Honnen from comment #17)
> Looks OK to me, though at first glance I would have reduced it to "Include
> required * automatically..." - isn't "not included" obvious?

Yes, the text is too complicated.

Suggestion from Sarika:

" Include all required * automatically while launching"

Maybe we can remove "all" as well from here as Julian has mentioned.
Comment 19 Hannes Wellmann CLA 2022-03-29 03:59:21 EDT
You are right.

Let's got for (and correspondingly for Bundles)

"Include required Plug-ins automatically when launching"
"Include required Features and Plug-ins automatically when launching"

As you said "all" is actually obvious.

What I wonder, should it be "when launching" or "while launching"?
I'm not a native English speaker so my decision could be bad.
Comment 20 Eclipse Genie CLA 2022-03-29 12:32:23 EDT
New Gerrit change created: https://git.eclipse.org/r/c/pde/eclipse.pde.ui/+/192328
Comment 21 Hannes Wellmann CLA 2022-03-29 12:48:22 EDT
(In reply to Hannes Wellmann from comment #19)
> What I wonder, should it be "when launching" or "while launching"?
> I'm not a native English speaker so my decision could be bad.

I asked the internet for the difference and as far as I understand it "while" should be used when something happens during a process that has a duration and "when" should be used if something happens at or due to a certain event at a certain point of time.
With this explanation both would IMO be valid since the launch can be seen as both an event or as a progress but I for now decided to see it as a process with a (short) duration and took "while".
If you disagree please let me know until tomorrow morning (CET). I would like to submit the associated changes then.
Comment 25 Vikas Chandra CLA 2022-04-05 03:32:39 EDT
Hannes, can you please provide a N&N entry for this?
Comment 26 Hannes Wellmann CLA 2022-04-06 12:33:37 EDT
(In reply to Vikas Chandra from comment #25)
> Hannes, can you please provide a N&N entry for this?

Yes I will.
I just wanted to complete Bug 570760 and Bug 325614 first and have a combined entry for them with a nice connected story.
Comment 27 Hannes Wellmann CLA 2022-05-17 01:49:21 EDT
Verified with Eclipse SDK

Version: 2022-06 (4.24)
Build id: I20220516-1800

I'm working on the N&N entry.
Comment 28 Hannes Wellmann CLA 2022-05-17 18:53:49 EDT
PR for the N&N entry:
https://github.com/eclipse-platform/www.eclipse.org-eclipse-news/pull/24