Quick fix for adding a space after the colon #1629

nburnwal09 · 2025-02-27T07:17:35Z

This is to provide a quick fix for situations when no space is added after the colon in the MANIFEST.MF file, and the value is entered by the user.

For example, in line 12, there is no space after the colon before the value 'test'. To address this, we are providing a quick fix to add a space there, as shown below:

Attaching the screenshot before and after applying the quick fix.

github-actions · 2025-02-27T07:44:49Z

Test Results

765 files + 147 765 suites +147 56m 46s ⏱️ + 5m 52s
3 611 tests ± 0 3 557 ✅ + 1 54 💤 ± 0 0 ❌ - 1
10 833 runs +2 650 10 670 ✅ +2 627 163 💤 +24 0 ❌ - 1

Results for commit 0b900f2. ± Comparison against base commit 09b7f61.

♻️ This comment has been updated with latest results.

nburnwal09 · 2025-04-24T12:57:19Z

Can anyone please review this PR?
It is a small quick fix, but quite useful for developers.

merks

It generally looks quite fine to me. I've not had time to actually try it in the IDE. Maybe later today or tomorrow. Sorry no one has looked that is so far.

merks · 2025-04-24T14:27:15Z

ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/builders/PDEMarkerFactory.java

 	public static final int M_EXEC_ENV_TOO_LOW = 0x1029; // other problem
 	public static final int M_CONFLICTING_AUTOMATIC_MODULE = 0x1030; // other
 																		// problem
+	public static final int M_NO_SPACE_AFTER_COLON = 0x1032; // other problem


I wonder why this is 1032 and not 1031?

I have raised PR for a few other quick fixes too. Still in review.
Adding the 1031 PR here: https://github.com/eclipse-pde/eclipse.pde/pull/1625/files

nburnwal09 · 2025-05-12T09:46:17Z

Hi @merks ,
Could you please review this PR's when you have some time.

eclipse-pde-bot · 2025-06-05T08:40:42Z

This pull request changes some projects for the first time in this development cycle.
Therefore the following files need a version increment:

ui/org.eclipse.pde.core/META-INF/MANIFEST.MF
ui/org.eclipse.pde.ui/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 321f59708ea10a530d5ea2635a322a89313ca37c Mon Sep 17 00:00:00 2001
From: Eclipse PDE Bot <[email protected]>
Date: Thu, 5 Jun 2025 08:40:23 +0000
Subject: [PATCH] Version bump(s) for 4.37 stream


diff --git a/ui/org.eclipse.pde.core/META-INF/MANIFEST.MF b/ui/org.eclipse.pde.core/META-INF/MANIFEST.MF
index 0d96f8a730..61b5e3c6e3 100644
--- a/ui/org.eclipse.pde.core/META-INF/MANIFEST.MF
+++ b/ui/org.eclipse.pde.core/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %name
 Bundle-SymbolicName: org.eclipse.pde.core; singleton:=true
-Bundle-Version: 3.20.200.qualifier
+Bundle-Version: 3.20.300.qualifier
 Bundle-Activator: org.eclipse.pde.internal.core.PDECore
 Bundle-Vendor: %provider-name
 Bundle-Localization: plugin
diff --git a/ui/org.eclipse.pde.ui/META-INF/MANIFEST.MF b/ui/org.eclipse.pde.ui/META-INF/MANIFEST.MF
index 645e581c4d..69ac002d3d 100644
--- a/ui/org.eclipse.pde.ui/META-INF/MANIFEST.MF
+++ b/ui/org.eclipse.pde.ui/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %name
 Bundle-SymbolicName: org.eclipse.pde.ui; singleton:=true
-Bundle-Version: 3.16.100.qualifier
+Bundle-Version: 3.16.200.qualifier
 Bundle-Activator: org.eclipse.pde.internal.ui.PDEPlugin
 Bundle-Vendor: %provider-name
 Bundle-Localization: plugin
-- 
2.49.0

Further information are available in Common Build Issues - Missing version increments.

merks · 2025-06-09T07:40:32Z

...rg.eclipse.pde.core/src/org/eclipse/pde/internal/core/builders/JarManifestErrorReporter.java

+					return;
+				}
 				if (line.length() < colon + 2 || line.charAt(colon + 1) != ' ') {
 					report(PDECoreMessages.BundleErrorReporter_noSpaceValue, lineNumber, CompilerFlags.ERROR, PDEMarkerFactory.CAT_FATAL);


Does BundleErrorReporter_noSpaceValue become unused as a result of this change?

It's defined as

BundleErrorReporter_noSpaceValue = Single space and a value is required after the header name

But the new property doesn't sound like a description of the error but rare a description of what the quick fix will do:

AddSpaceAfterColon_add=Add a space after the colon

merks · 2025-06-09T07:41:31Z

...rg.eclipse.pde.core/src/org/eclipse/pde/internal/core/builders/JarManifestErrorReporter.java

 					report(PDECoreMessages.BundleErrorReporter_invalidHeaderName, lineNumber, CompilerFlags.ERROR, PDEMarkerFactory.CAT_FATAL);
 					return;
 				}
+				if (line.length() >= colon + 2 && line.charAt(colon + 1) != ' ') {


It's not clear why you changing the condition under which an error is generated.

This condition is added to check if there is a space after the ':' and just before the value part.
If not, error is thrown saying 'Single space is required after the
colon' via PDECoreMessages.BundleErrorReporter_noSpaceAfterColon

Ah, silly me. It's an addition. Sorry.

merks · 2025-06-09T08:50:28Z

ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/correction/AddSpaceBeforeValue.java

+			}
+
+		} catch (Exception e) {
+


I'm not sure it's a good idea to just catch any exception and then just ignore it. What exception might be of concern here? I guess BadLocationException, which shouldn't be possible given the guards, but better that's more clear.

merks · 2025-06-10T09:46:55Z

ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/correction/AddSpaceBeforeValue.java

+		try {
+			IDocument doc = model.getDocument();
+			int lineNum;
+			lineNum = (int) marker.getAttribute(IMarker.LINE_NUMBER);


I was trying to figure out where the CoreException comes, and it's here. Probably better to use org.eclipse.core.resources.IMarker.getAttribute(String, int)` which doesn't throw an exception and returns an int.

I'm also not sure why you split the variable declaration from the variable initialization. That doesn't look so nice.

merks · 2025-06-10T09:49:27Z

ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/correction/AddSpaceBeforeValue.java

+				doc.replace(offset, colonInd + 1, userHeader);
+			}
+		} catch (BadLocationException e) {
+			e.printStackTrace();


Stack trace printing is almost always a bad idea because unless one monitoring the running IDE with a console, you want see this and then the operation quietly does nothing with no logged details about why.

merks · 2025-06-10T10:00:21Z

BTW, what I find often very useful is to search the SDK workspace for similar things.

I have a feeling from all the failures that this need to be rebased.

If you properly test that the location cannot be bad, then it's okay to have just a comment saying the location has been checked to ensure that it cannot be a bad location.

nburnwal09 · 2025-06-10T12:00:21Z

@merks
| have addressed all the suggestions. Please have a look.

nburnwal09 · 2025-06-17T06:03:43Z

Hi @merks
I have addressed all your suggestion. Can you please review it?

merks · 2025-06-17T06:18:58Z

ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/correction/AddSpaceBeforeValue.java

+		} catch (BadLocationException e) {
+			// the location has been checked to ensure that it cannot be a bad
+			// location
+			e.printStackTrace();


Either this can never happen because guards or it should be logged, but not just printed to the console.

nburnwal09 · 2025-06-23T09:30:20Z

Hi @merks
Can you review the PR? I have made all the necessary changes

nburnwal09 · 2025-08-08T10:43:52Z

@merks kindly review this PR.

merks

Mostly looks good. Needs the copyright/license header for the new file.

merks · 2025-08-08T13:21:10Z

ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/correction/AddSpaceBeforeValue.java

@@ -0,0 +1,50 @@
+package org.eclipse.pde.internal.ui.correction;


Missing copyright/license header.

merks

It generally looks fine, but I didn't have time to test it.

merks reviewed Apr 24, 2025

View reviewed changes

nburnwal09 force-pushed the QuickFix_addSpace branch from bc5e11b to c006664 Compare May 27, 2025 10:19

nburnwal09 force-pushed the QuickFix_addSpace branch from e115ec2 to baddcfa Compare June 9, 2025 07:25

merks reviewed Jun 9, 2025

View reviewed changes

merks requested a review from HannesWell June 9, 2025 07:43

nburnwal09 requested a review from merks June 9, 2025 08:33

merks reviewed Jun 9, 2025

View reviewed changes

nburnwal09 force-pushed the QuickFix_addSpace branch 2 times, most recently from ea36cfd to a8f0994 Compare June 10, 2025 09:33

nburnwal09 requested a review from merks June 10, 2025 09:34

merks requested changes Jun 10, 2025

View reviewed changes

nburnwal09 force-pushed the QuickFix_addSpace branch from a8f0994 to fc5b9fd Compare June 10, 2025 11:58

nburnwal09 requested a review from merks June 10, 2025 17:30

nburnwal09 force-pushed the QuickFix_addSpace branch from 815db17 to af0ca90 Compare June 13, 2025 06:59

merks requested changes Jun 17, 2025

View reviewed changes

nburnwal09 force-pushed the QuickFix_addSpace branch from ba943e3 to bd68815 Compare June 23, 2025 09:29

nburnwal09 requested a review from merks June 23, 2025 09:30

nburnwal09 force-pushed the QuickFix_addSpace branch from ca6863f to 7aedfb6 Compare July 3, 2025 06:49

nburnwal09 force-pushed the QuickFix_addSpace branch 2 times, most recently from 03a64a3 to f51785a Compare July 31, 2025 06:00

nburnwal09 force-pushed the QuickFix_addSpace branch 2 times, most recently from 60ea554 to 6f46999 Compare August 5, 2025 08:20

nburnwal09 force-pushed the QuickFix_addSpace branch 3 times, most recently from ee3bde3 to 011febc Compare August 8, 2025 08:47

merks requested changes Aug 8, 2025

View reviewed changes

nburnwal09 force-pushed the QuickFix_addSpace branch 2 times, most recently from ee3bde3 to fc9cc6b Compare August 13, 2025 00:47

Quick fix for adding a space after the header colon

0b900f2

nburnwal09 force-pushed the QuickFix_addSpace branch from fc9cc6b to 0b900f2 Compare August 14, 2025 09:18

nburnwal09 requested a review from merks August 19, 2025 08:26

merks approved these changes Aug 20, 2025

View reviewed changes

gireeshpunathil approved these changes Sep 17, 2025

View reviewed changes

gireeshpunathil merged commit 9213517 into eclipse-pde:master Sep 17, 2025
19 checks passed

		@@ -0,0 +1,50 @@
		package org.eclipse.pde.internal.ui.correction;

Quick fix for adding a space after the colon #1629

Quick fix for adding a space after the colon #1629

Uh oh!

Conversation

nburnwal09 commented Feb 27, 2025

Uh oh!

github-actions bot commented Feb 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

nburnwal09 commented Apr 24, 2025

Uh oh!

merks left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nburnwal09 commented May 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eclipse-pde-bot commented Jun 5, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

merks commented Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nburnwal09 commented Jun 10, 2025

Uh oh!

nburnwal09 commented Jun 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nburnwal09 commented Jun 23, 2025

Uh oh!

nburnwal09 commented Aug 8, 2025

Uh oh!

merks left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

merks left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Feb 27, 2025 •

edited

Loading

nburnwal09 commented May 12, 2025 •

edited

Loading

merks commented Jun 10, 2025 •

edited

Loading