Added a button to show launch bundles

[nburnwal09]

Added a button to show launch bundles: #1850
It has filter option and shows number of bundles selected(attached screenshot)

[github-actions]

Test Results

   798 files  ±0     798 suites  ±0   59m 14s ⏱️ +19s
 3 787 tests ±0   3 733 ✅ ±0   54 💤 ±0  0 ❌ ±0 
11 127 runs  ±0  10 964 ✅ ±0  163 💤 ±0  0 ❌ ±0 

Results for commit e2b5cdd. ± Comparison against base commit 85d8c94.

♻️ This comment has been updated with latest results.

[nburnwal09]

@laeubi kindly review this PR

[gireeshpunathil]

your screen shot is however showing "Show plug-ins", how is that possible? was this modified later?

[nburnwal09]

ohh yes I renamed it later. Thanks for pointing that out
I will update the screenshot now

[HannesWell]

Thanks for this proposal.

I have a few remarks below, but there are two more points:

1. The button should also be available for Eclipse-apps based on features (at least personally I'm hardly using bundle-based apps). This can be done easily by repeating the same code of the AbstractPluginBlock in the FeatureBlock, around here:
   

   eclipse.pde/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/launcher/FeatureBlock.java

   Lines 931 to 933 in 66a95fd

   	fAutoValidate.addSelectionListener(fListener);
   	Composite rightAlignComp = SWTFactory.createComposite(validatecomp, 1, 1, SWT.NONE, 0, 0);
   	rightAlignComp.setLayoutData(new GridData(SWT.RIGHT, SWT.BOTTOM, true, true));

   
   Ideally features would then also be listed (in addition to bundles), but I assume the currently used dialog is not capable of that.
2. While using the PluginSelectionDialog is certainly a quick solution, I'm not sure if it's really suitable. At least the title of the dialog should be changed to something more meaningful in this use-case.
   What I'm currently missing is the ability to copy the list of plugins (ideally just the selected sub-set) and being able to past it e.g. into a text- or diff-editor. This way I often try to figure what has changed in the full application. But that's not working at the moment.
   For #1850 I initially thought about a separate tab or something like that.
   Maybe we could instead also re-use the selection tree in the center and just have a check-box on the right to list all included (features and) bundles. Of course selecting them should then not be possible.

But yes that would probably be more work and I'm not sure if that's overall much better in the end. It's currently just an idea.

[HannesWell]

As the button is used in only one method, this field is not necessary and the button can just be a local variable.

Suggested change

private Button fShowPlugin;

[nburnwal09]

For #1850 I initially thought about a separate tab or something like that.

thanks @HannesWell . I have these questions:

1. What would be an added benefit of having a separate tab as opposed to the current checkbox? is it to accommodate more scenarios than just the bundles view, or to improve the user experience, or something else?

In case we add a new separate tab:

2. are you suggesting to have a single view (in the new tab) for both features and plugins? and the tab embedding the tree view / list view in itself, as opposed to a popup dialog? and in the tree / list view, selection of one ore more features in the left shows the associated plugins? (with a default of all features and all plugins in the launch configuration displayed)
3. Can you please clarify on what you mean by re using the selection tree in the centre? if we want a feature-to-plugin mapping to be displayed, shouldn't the features be in the left, as opposed to be in the center?
4. What would happen about the unbounded plug-ins which doesn't correspond to any features? should we list them as is
5. > Maybe we could instead also re-use the selection tree in the center and just have a check-box on the right to list all included (features and) bundles. Of course selecting them should then not be possible. - can you pls clarify why selecting them should not be possible?

[nburnwal09]

@HannesWell

1. The button should also be available for Eclipse-apps based on features (at least personally I'm hardly using bundle-based apps). This can be done easily by repeating the same code of the AbstractPluginBlock in the FeatureBlock, around here:

I added the same for Eclipse-apps based on features. Also, with current changes, if we select one/more features and click on 'show launch bundles', it is showing the list of plug-ins(direct and transitive dependent plug-ins both) respective to the features selected.

2. While using the PluginSelectionDialog is certainly a quick solution, I'm not sure if it's really suitable. At least the title of the dialog should be changed to something more meaningful in this use-case.

Yeah I missed that. I have changed the title to 'Launch Plug-ins list'

What I'm currently missing is the ability to copy the list of plugins (ideally just the selected sub-set) and being able to past it e.g. into a text- or diff-editor. This way I often try to figure what has changed in the full application. But that's not working at the moment.

Yeah I have that in my mind(based on our conversation in #1850 (comment)). I will be taking the copy/paste option in another PR.

[laeubi]

Ideally features would then also be listed (in addition to bundles), but I assume the currently used dialog is not capable of that.

Features are exploded and do not remain in the final runtime, so I would say this is not really useful here. One might want to show what a feature pulls in but then we need a tree to represent the data and it would possibly complicate things a lot.

So I would vote for starting simple and improve on that iteratively.

[nburnwal09]

@laeubi

Features are exploded and do not remain in the final runtime, so I would say this is not really useful here. One might want to show what a feature pulls in but then we need a tree to represent the data and it would possibly complicate things a lot.

With my changes, we will still be able to see what a feature pulls in(the direct and indirect dependent plug-in).

I added the same for Eclipse-apps based on features. Also, with current changes, if we select one/more features and click on 'show launch bundles', it is showing the list of plug-ins(direct and transitive dependent plug-ins both) respective to the features selected.

[laeubi]

While this can be interesting I don't think the selection should matter and would expect to always see the final result of what will happen when I click Run

[nburnwal09]

Hi @HannesWell
Please have a look at this PR whenever you get sometime.

[nburnwal09]

Hi @HannesWell
Kindly have a look at this PR.

[gireeshpunathil]

@HannesWell - do you still have outstanding suggestions on this?

[HannesWell]

@HannesWell - do you still have outstanding suggestions on this?

Sorry for the delay. I'll look into the new state tomorrow.

[HannesWell]

For #1850 I initially thought about a separate tab or something like that.

1. What would be an added benefit of having a separate tab as opposed to the current checkbox? is it to accommodate more scenarios than just the bundles view, or to improve the user experience, or something else?

Good question. I cannot answer this clearly, but I would say that it would allow to support more scenarios or 'tools' (later).
This could also be a general UI/UX questions: Why are things located in tabs and not just a wizard or preferences like page that opens when you click a button. I would say it 'feels' better from what I'm used to, but I'd need to ask an UX expert for a more clear and scientific answer.

But to come back to the current proposal and as my counter-suggestion from #1922 (review), is indeed much more complex, what about re-using the approach used for the Show Command Line button?
This button just uses a not editable, but selectable text block/field to show the resolved command line:

If we use that to show the list of resolved bundles in CSV style (if we want to have more then the name) and also add a search field to for the text, then we would have, what would be provided with your proposal plus the ability to copy (parts) of the list.
Plus you could easily add start-level infos or the file-system path to the jar using a CSV style.

To feed the content of that text field you could actually use what's generated into the <configuration-path>/org.eclipse.equinox.simpleconfigurator/bundles.info nowadays (if the app contains P2). Otherwise you could use the value of the osgi.bundles field in the <configuration-path>/config.ini. Or you just use the current approach and add the suggested additional information from it (this would avoid the need to check two possible files, with different format.

Overall I think it should still be not too complicated to achieve and should provide more information than today.

What do you think?

[nburnwal09]

But to come back to the current proposal and as my counter-suggestion from #1922 (review), is indeed much more complex, what about re-using the approach used for the Show Command Line button? This button just uses a not editable, but selectable text block/field to show the resolved command line:

If we use that to show the list of resolved bundles in CSV style (if we want to have more then the name) and also add a search field to for the text, then we would have, what would be provided with your proposal plus the ability to copy (parts) of the list. Plus you could easily add start-level infos or the file-system path to the jar using a CSV style.

To feed the content of that text field you could actually use what's generated into the <configuration-path>/org.eclipse.equinox.simpleconfigurator/bundles.info nowadays (if the app contains P2). Otherwise you could use the value of the osgi.bundles field in the <configuration-path>/config.ini. Or you just use the current approach and add the suggested additional information from it (this would avoid the need to check two possible files, with different format.

Overall I think it should still be not too complicated to achieve and should provide more information than today.

What do you think?

Sure Hannes, that sounds better given that we will have selected text block/field. I will check this.

[nburnwal09]

If we use that to show the list of resolved bundles in CSV style (if we want to have more then the name) and also add a search field to for the text, then we would have, what would be provided with your proposal plus the ability to copy (parts) of the list. Plus you could easily add start-level infos or the file-system path to the jar using a CSV style.

@HannesWell
Sorry for the delay, I got busy with other things and could not proceed with this.

I am re-using the approach used for the Show Command Line button
I have one question -
Did you really want me to show comma-separated row text (fields) for easier copy-paste, or did you expect me to put it in a grouped-column format like in the plug-ins view?

[HannesWell]

Did you really want me to show comma-separated row text (fields) for easier copy-paste, or did you expect me to put it in a grouped-column format like in the plug-ins view?

If you can make it fully copy-able where one gets the bundle-symbolic name and the version (and maybe even the file-system path) speparated by space or semicolon or similar copied into the clipboard on copy, a table like UI would definitively be nicer.
But as it would probably be more complex I would also be fine with a copyable, read-only text field, maybe just as a first step.
As mentioned in #1922 (comment), I personally use the simple-configurator file for the designated purpose of the new button and it gives me everything I need.
Although I agree that it feels quite low-level.

[nburnwal09]

If you can make it fully copy-able where one gets the bundle-symbolic name and the version (and maybe even the file-system path) speparated by space or semicolon or similar copied into the clipboard on copy, a table like UI would definitively be nicer. But as it would probably be more complex I would also be fine with a copyable, read-only text field, maybe just as a first step. As mentioned in #1922 (comment), I personally use the simple-configurator file for the designated purpose of the new button and it gives me everything I need. Although I agree that it feels quite low-level.

@HannesWell
Yeah, but if I use simple-configurator - bundles.info, I would need to filter all the selected options from the plugins list.

Earlier you had mentioned this to fetch all the details which gives the list of selected plug-ins.
I will check if I can get the bundles details along with start level and file-system path to the jar from here. If not, I would filter and get it from bundles.info

[nburnwal09]

@HannesWell
Please have a look at this commit. I have made necessary changes which allows selecting and copying the bundle values.
This is the view now:

[nburnwal09]

Hi @HannesWell
Could you please look into the changes whenever you get sometime?

[nburnwal09]

Hi @HannesWell
Kindly check this PR whenever you get time.

[gireeshpunathil]

this is ready to land, but would benefit from a final review by @HannesWell . @HannesWell - pls have a look?

[HannesWell]

this is ready to land, but would benefit from a final review by @HannesWell . @HannesWell - pls have a look?

Very sorry for the delay! I'll try to have a look tomorrow.

[HannesWell]

I just looked into this and in general it looks good. Thanks for your work on this.

But I have multiple minor points that should be addressed.

• The pop-up listing all bundles should be wider or at least should not break lines and instead show a horizontal scroll-bar
• In the headline of the pop-up, i.e. above the text field, there should be an extra line explaining the format of the text below.
• For OSGi Framwork launch configs the formatting seems off and the code in createControl() should be stream-lined, but that's not related to this PR and I'll provide a dedicated one this evening.

I'll provide more details later today as I have to leave now.

[HannesWell]

Please see the details comments below.
In general please always format new code and add copy-right headers to new files and don't format code that hasn't been touched.

Besides the points mentioned below, it would be good to have an explanation about the displayed values respectively their structure at the top of the dialog.

List of all bundles contained in this launch:
<Symbolic Name>, <Version>, <Start level>, <File Path>

The text is just a first attempt and probably should be improved.
Do you have suggestions for that?

[nburnwal09]

@HannesWell
I have addressed all your suggestions. Kindly review this.

[HannesWell]

Thank you @nburnwal09 for the update.

I made a few more adjustments, mostly minor and the most relevant one was to move the explanation for the new dialog into a separate label above the list of bundles.
With that this is ready for submission from my side, too.