eclipse-platform
[#1668] API types to simplify work with launch configuration attributes
- identify launch attribute
- connect it with preference metadata (to supply defaults/label/description)
- read attribute from configuration
- write attribute to configuration working copy
- demonstrate usage for ExternalTools
This pull request changes some projects for the first time in this development cycle. Therefore the following files need a version increment:
ant/org.eclipse.ant.launching/META-INF/MANIFEST.MF
ant/org.eclipse.ant.ui/META-INF/MANIFEST.MF
debug/org.eclipse.ui.externaltools/META-INF/MANIFEST.MF
An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch.
Git patch
From 9d9fc2199cde44fd761137da4d6184cafbefd898 Mon Sep 17 00:00:00 2001
From: Eclipse Platform Bot <[email protected]>
Date: Mon, 10 Mar 2025 11:24:15 +0000
Subject: [PATCH] Version bump(s) for 4.36 stream
diff --git a/ant/org.eclipse.ant.launching/META-INF/MANIFEST.MF b/ant/org.eclipse.ant.launching/META-INF/MANIFEST.MF
index 701a5b50f8..9c9a10f1a4 100644
--- a/ant/org.eclipse.ant.launching/META-INF/MANIFEST.MF
+++ b/ant/org.eclipse.ant.launching/META-INF/MANIFEST.MF
@@ -3,7 +3,7 @@ Bundle-ManifestVersion: 2
Bundle-Localization: plugin
Bundle-Name: %pluginName
Bundle-SymbolicName: org.eclipse.ant.launching;singleton:=true
-Bundle-Version: 1.4.700.qualifier
+Bundle-Version: 1.4.800.qualifier
Bundle-Activator: org.eclipse.ant.internal.launching.AntLaunching
Require-Bundle: org.eclipse.core.runtime;bundle-version="[3.29.0,4.0.0)",
org.eclipse.debug.core;bundle-version="[3.12.0,4.0.0)",
diff --git a/ant/org.eclipse.ant.ui/META-INF/MANIFEST.MF b/ant/org.eclipse.ant.ui/META-INF/MANIFEST.MF
index 14d67d91bc..d9d8d92c9a 100644
--- a/ant/org.eclipse.ant.ui/META-INF/MANIFEST.MF
+++ b/ant/org.eclipse.ant.ui/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: %pluginName
Bundle-SymbolicName: org.eclipse.ant.ui; singleton:=true
-Bundle-Version: 3.10.0.qualifier
+Bundle-Version: 3.10.100.qualifier
Bundle-Activator: org.eclipse.ant.internal.ui.AntUIPlugin
Bundle-Vendor: %providerName
Bundle-Localization: plugin
diff --git a/debug/org.eclipse.ui.externaltools/META-INF/MANIFEST.MF b/debug/org.eclipse.ui.externaltools/META-INF/MANIFEST.MF
index 3d7bc00b7c..40216b2a14 100644
--- a/debug/org.eclipse.ui.externaltools/META-INF/MANIFEST.MF
+++ b/debug/org.eclipse.ui.externaltools/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: %Plugin.name
Bundle-SymbolicName: org.eclipse.ui.externaltools; singleton:=true
-Bundle-Version: 3.6.500.qualifier
+Bundle-Version: 3.6.600.qualifier
Bundle-Activator: org.eclipse.ui.externaltools.internal.model.ExternalToolsPlugin
Bundle-Vendor: %Plugin.providerName
Bundle-Localization: plugin
--
2.48.1
Further information are available in Common Build Issues - Missing version increments.
Test Results
1 947 files 1 947 suites 1h 37m 35s ⏱️ 4 720 tests 4 696 ✅ 24 💤 0 ❌ 14 160 runs 13 993 ✅ 167 💤 0 ❌
Results for commit e1b3b609.
:recycle: This comment has been updated with latest results.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
As mentioned in the call, some example using this API proposal would be helpful. There is probably some content in org.eclipse.ui.externaltools/External Tools Base/org/eclipse/ui/externaltools/internal/launchConfigurations you can tweak to leverage this API.
I'm not sure why it is is failing @HannesWell
11:59:41.178 [ERROR] Failed to execute goal org.eclipse.tycho:tycho-apitools-plugin:4.0.11-SNAPSHOT:verify (verify) on project org.eclipse.debug.core:
Execute ApiApplication failed: InvocationTargetException: EmptyStackException -> [Help 1]
I'm not sure why it is is failing @HannesWell
11:59:41.178 [ERROR] Failed to execute goal org.eclipse.tycho:tycho-apitools-plugin:4.0.11-SNAPSHOT:verify (verify) on project org.eclipse.debug.core: Execute ApiApplication failed: InvocationTargetException: EmptyStackException -> [Help 1]
It looks like your change is breaking API-tools^^
I have replayed the Jenkins build the -e set to get an more informative stack-trace:
https://ci.eclipse.org/platform/job/eclipse.platform/job/PR-1669/7/
Thank you @HannesWell for your assistance.
It looks like your change is breaking API-tools^^
WOW! But that wasn't part of my plan. It looks like this issue was already reported. Interesting, may be I can have a deeper look to it soon.
And still interested in your feedback regarding PR content.
After supporting
recordfor API Tools everything is green @HannesWell
Thank you for that fix/enhancement of API-tools! It's much appreciated.
And still interested in your feedback regarding PR content.
Sorry for not responding. It has been some busy days recently. I have not forgotten this, just didn't have the time for it. I try my best to have look at it in the next days.
Thank you for your feedback @HannesWell
PR was reimplemented according to your comments. I'm not sure why we are fighting for reducing number of API types with adding more API methods and more implementation code the the interface compilation unit, but the result looks much more closer to the old good Eclipse 2.0 style of Debug subsystem.
I'm not sure why we are fighting for reducing number of API types with adding more API methods and more implementation code the the interface compilation unit, but the result looks much more closer to the old good Eclipse 2.0 style of Debug subsystem.
I don't consider it as a fight, more as a search for (ideally) the best solution. Of course what is best is also a matter of opinion, but at least my personal opinion is that in general I like more compact APIs better. However can you please explain to me what the advantage is of defining in types what could be methods respectively why exactly this is Eclipse-2.0 style and why it's bad (I haven't been around back then)?
With the suggestions made we could still define typed launch-attributes using LaunchAttribute(-Defined/Definition) as static constant and read/write with it from/to a LaunchConfiguration in a more convenient way, couldn't we?
But of course others can have other opinions about it and I invite everyone to share theirs.
You are right @HannesWell , it's definitely a matter of personal opinion. For me the old good Eclipse 2.0 style means a lot of static definitions, favoring of primitive types in API and a lot of setters to reuse one instance as long as one can. All this was actual 25 years ago for ancient JVMs. Nowadays we have modern JVM and very dynamic environment where non-replaceable static definition can cause problems and saving memory due to mutability of objects does not justify the cost of (much more complex) code support.
And even if I've stopped naming interfaces with "I" for new code and prefer simple immutable objects with very simple interfaces, I've done a lot following your comments to make this API improvement better suited to the surrounding code.
Now the question is: what else should be improved in this PR? I still can see your "change requested" veto but your latest comment doesn't content any direct change requests. Please clarify.
@HannesWell I changed it to a state that we discussed on a meeting:
- remove
ILaunchAttributeIdentity - minimize API surface
@HannesWell do you have more comments on this one?
I didn't had the time yet to look into this, but definitely want to. Try to respond until the end of this week and will also look how to use this for PDE.
@HannesWell do you have more comments on this one?
I didn't had the time yet to look into this, but definitely want to. Try to respond until the end of this week and will also look how to use this for PDE.
I only had the time for a short look at this change and in general I think this looks very good now. But I'd like to do an in depth review and try my best to do it tomorrow or Wednesday.