Add support for Eclipse-BundleShape in Manifest-Editor #949

alshamams · 2023-11-29T10:36:32Z

Three modes are supported: jar, dir and default.
First two are self-explanatory, third option removes the header which is the default behaviour.
Fixes: #864

github-actions · 2023-11-29T10:57:28Z

Test Results

291 files ±0 291 suites ±0 53m 56s ⏱️ + 1m 5s
3 526 tests ±0 3 468 ✅ ±0 58 💤 ±0 0 ❌ ±0
10 875 runs ±0 10 698 ✅ ±0 177 💤 ±0 0 ❌ ±0

Results for commit fe94e3d. ± Comparison against base commit 530e7a7.

♻️ This comment has been updated with latest results.

laeubi · 2023-11-29T20:50:07Z

@alshamams can you share a screenshot how it looks like?

alshamams · 2023-11-30T06:07:48Z

Hi @laeubi ,
This is an image of the overview tab where the options jar, dir and default are available.

This is an image of the updated Manifest, if for example jar is chosen.

laeubi · 2023-11-30T06:12:17Z

@alshamams thanks for sharing the screen, the radio buttons look a bit stretched, can you make it that there is no large space between them?

alshamams · 2023-12-01T07:01:18Z

I have adjusted the spacing between the radio buttons. Kindly review and let me know. @laeubi

alshamams · 2023-12-01T07:34:23Z

Thanks a lot @laeubi . Do you think this qualifies as a noteworthy feature?

laeubi · 2023-12-01T07:37:02Z

Thanks a lot @laeubi . Do you think this qualifies as a noteworthy feature?

I think anything new can be added to N&N if you like.

merks · 2023-12-01T07:45:13Z

Just a general comment. It generally looks nice to have a significant number of N&N entries for each release. It makes it look like we're doing lots of good things for the community...

laeubi · 2023-12-01T07:50:46Z

It makes it look like we're doing lots of good things for the community...

It makes ~~it look like~~ visible that we're doing lots of good things for the community...

HannesWell

Thank you @alshamams for working on this.
I just have a coarse look, but in general it looks good.
Will make a more detailed review tomorrow.

ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/editor/plugin/GeneralInfoSection.java

ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/pderesources.properties

alshamams · 2023-12-05T06:29:05Z

Hi @HannesWell ,
I have made the suggested changes. Do let me know if any other edits are required.

HannesWell · 2023-12-05T07:24:51Z

Hi @HannesWell , I have made the suggested changes. Do let me know if any other edits are required.

Thank you. Sorry I didn't find the time at the weekend. But will do a (hopefully final) review this evening.

From a quick look at this image, I would say that default should be the first option, then jar and then directory.

HannesWell

I have reviewed this in detail and added some remarks below and pushed another commit on top yours that applies the remarks and also does a few more minor simplifications.

What do you think about them?

ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/editor/plugin/GeneralInfoSection.java

alshamams · 2023-12-06T07:23:49Z

Thanks @HannesWell for your meticulous review. All the proposed changes look good to me.
Let me know if I could squash the commits. (Not sure if squashing will cause your attribution to be lost?)

HannesWell

Thanks @HannesWell for your meticulous review. All the proposed changes look good to me. Let me know if I could squash the commits. (Not sure if squashing will cause your attribution to be lost?)

Your welcome and I'm glad you like the suggestions. Yes please squash the commits.
You can attribute me by mentioning me in a footer in the sense of
Co-authored-by: Some Bodyelse <somebodyelse@nowhere.com>
using my Name+Mail from the commit (see https://www.eclipse.org/projects/handbook/#resources-commit for details).

I just rebased and slightly updated the PR to further unify the button creation.
The points that are currently open:

Externalization of radio button labels (default, jar and directory should be externalized just like the main message, we should not reuse the constants for the labels although they have the same value by default, but in other languages other labels might be appropriate).
We could add a more elaborative tool-tip to each choice or do you all think the current suggestion is clear enough?

HannesWell · 2023-12-06T23:45:05Z

...g.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/editor/plugin/PluginGeneralInfoSection.java

@@ -74,6 +74,7 @@ protected void createSpecificControls(Composite parent, FormToolkit toolkit, IAc
 		if (isBundle() && (formEditor instanceof ManifestEditor)) {
 			createLazyStart(parent, toolkit, actionBars);
 			createSingleton(parent, toolkit, actionBars, PDEUIMessages.PluginGeneralInfoSection_singleton);
+			createBundleShape(parent, toolkit, PDEUIMessages.PluginGeneralInfoSection_bundleshape);


By calling createBundleShape() here the Bundle-Shape buttons are only available for non-fragment bundles, but fragments can also have an explicit Bundle-Shape.
So this should called for all kind of Plug-in projects in a way that it is still at the right position.
So probably in GeneralInfoSection.createClient() after createSpecificControls() has been called.

Hi @HannesWell ,
I have addressed all the review comments. Please have a look and let me know. (I am unable to select the checkboxes in the above comment.)

Thank you. I'm a bit short in time today, but will have a look at this ASAP.
But I think we can submit this soon. :)

Three modes are supported: jar, dir and default. First two are self-explanatory, third option removes the header which is the default behavior. Fixes: eclipse-pde#864 Co-authored-by: Hannes Wellmann <wellmann.hannes1@gmx.net>

HannesWell

Sorry for the delay.
I adjusted the tool-tips to describe the consequences of each possible option, similiar to how it is described in the doc:
https://help.eclipse.org/latest/index.jsp?topic=%2Forg.eclipse.platform.doc.isv%2Freference%2Fmisc%2Fbundle_manifest.html

With that this is ready for submission.
Thanks @alshamams for this contribution again.

HannesWell · 2024-02-11T20:00:23Z

ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/pderesources.properties

@@ -1510,6 +1510,7 @@ PluginWorkingSet_deselectAll_label=Dese&lect All
 PluginDevelopmentPage_presentation=Plug-in Manifest Editor Presentation
 PluginGeneralInfoSection_lazyStart=Activate this plug-in when one of its classes is loaded
 PluginGeneralInfoSection_singleton=This plug-in is a singleton
+PluginGeneralInfoSection_bundleshape=Shape after P2 installation:


In general we could re-think if this should be made more neutral, but this can be done in a follow-up.

Suggested change

PluginGeneralInfoSection_bundleshape=Shape after P2 installation:

PluginGeneralInfoSection_bundleshape=Shape after installation:

alshamams force-pushed the bund_shape_support branch from 8dbe9b5 to 949e6be Compare December 1, 2023 06:58

laeubi requested a review from HannesWell December 1, 2023 07:12

laeubi approved these changes Dec 1, 2023

View reviewed changes

gireeshpunathil approved these changes Dec 1, 2023

View reviewed changes

HannesWell reviewed Dec 1, 2023

View reviewed changes

ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/editor/plugin/GeneralInfoSection.java Outdated Show resolved Hide resolved

ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/pderesources.properties Outdated Show resolved Hide resolved

alshamams force-pushed the bund_shape_support branch from dae3ac2 to fb73cd0 Compare December 5, 2023 06:27

HannesWell force-pushed the bund_shape_support branch from fb73cd0 to d48ac73 Compare December 5, 2023 22:59

HannesWell reviewed Dec 6, 2023

View reviewed changes

HannesWell force-pushed the bund_shape_support branch from 4f9ba17 to 3ee450a Compare December 6, 2023 23:26

HannesWell reviewed Dec 6, 2023

View reviewed changes

alshamams force-pushed the bund_shape_support branch from 3ee450a to a210808 Compare December 11, 2023 09:19

HannesWell force-pushed the bund_shape_support branch from a210808 to c99e249 Compare December 22, 2023 19:52

laeubi force-pushed the bund_shape_support branch from c99e249 to d187c13 Compare January 27, 2024 15:57

laeubi assigned HannesWell Jan 27, 2024

laeubi force-pushed the bund_shape_support branch from d187c13 to 5e4e410 Compare February 11, 2024 06:38

laeubi requested a review from HannesWell February 11, 2024 06:38

Add support for Eclipse-BundleShape in Manifest-Editor

fe94e3d

Three modes are supported: jar, dir and default. First two are self-explanatory, third option removes the header which is the default behavior. Fixes: eclipse-pde#864 Co-authored-by: Hannes Wellmann <wellmann.hannes1@gmx.net>

HannesWell force-pushed the bund_shape_support branch from 5e4e410 to fe94e3d Compare February 11, 2024 19:48

HannesWell approved these changes Feb 11, 2024

View reviewed changes

HannesWell reviewed Feb 11, 2024

View reviewed changes

HannesWell merged commit 43ec146 into eclipse-pde:master Feb 11, 2024
17 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for Eclipse-BundleShape in Manifest-Editor #949

Add support for Eclipse-BundleShape in Manifest-Editor #949

alshamams commented Nov 29, 2023

github-actions bot commented Nov 29, 2023 •

edited

laeubi commented Nov 29, 2023

alshamams commented Nov 30, 2023

laeubi commented Nov 30, 2023

alshamams commented Dec 1, 2023

alshamams commented Dec 1, 2023

laeubi commented Dec 1, 2023

merks commented Dec 1, 2023

laeubi commented Dec 1, 2023

HannesWell left a comment

alshamams commented Dec 5, 2023

HannesWell commented Dec 5, 2023

HannesWell left a comment

alshamams commented Dec 6, 2023

HannesWell left a comment •

edited

HannesWell Dec 6, 2023

alshamams Dec 11, 2023

HannesWell Dec 11, 2023

HannesWell left a comment

HannesWell Feb 11, 2024

	PluginGeneralInfoSection_bundleshape=Shape after P2 installation:
	PluginGeneralInfoSection_bundleshape=Shape after installation:

Add support for Eclipse-BundleShape in Manifest-Editor #949

Add support for Eclipse-BundleShape in Manifest-Editor #949

Conversation

alshamams commented Nov 29, 2023

github-actions bot commented Nov 29, 2023 • edited

Test Results

laeubi commented Nov 29, 2023

alshamams commented Nov 30, 2023

laeubi commented Nov 30, 2023

alshamams commented Dec 1, 2023

alshamams commented Dec 1, 2023

laeubi commented Dec 1, 2023

merks commented Dec 1, 2023

laeubi commented Dec 1, 2023

HannesWell left a comment

Choose a reason for hiding this comment

alshamams commented Dec 5, 2023

HannesWell commented Dec 5, 2023

HannesWell left a comment

Choose a reason for hiding this comment

alshamams commented Dec 6, 2023

HannesWell left a comment • edited

Choose a reason for hiding this comment

HannesWell Dec 6, 2023

Choose a reason for hiding this comment

alshamams Dec 11, 2023

Choose a reason for hiding this comment

HannesWell Dec 11, 2023

Choose a reason for hiding this comment

HannesWell left a comment

Choose a reason for hiding this comment

HannesWell Feb 11, 2024

Choose a reason for hiding this comment

github-actions bot commented Nov 29, 2023 •

edited

HannesWell left a comment •

edited