Modernize to Java 17 switch expression. #2816

deepika-u · 2025-02-25T12:51:28Z

Update legacy switch block with Java 17 switch expression in org.eclipse.ui.workbench.texteditor package.

iloveeclipse · 2025-02-25T12:57:28Z

...workbench.texteditor/src/org/eclipse/ui/internal/texteditor/rulers/RulerColumnPlacement.java

+				case BEFORE -> true;
+				default -> {
+					childHelper.fail(RulerColumnMessages.RulerColumnPlacement_illegal_child_msg);
+					yield false; // Required because default must return a value.


This looks wrong. Is this autogenerated or manually changed?
Original code would continue for loop without adding anything to constraints.

I had a doubt only in this context. So here it cannot be written in newer syntax does it mean?

No idea, have no time for this.

can this loop be like this to achieve the loop?

boolean before = switch (name) { case AFTER -> false; case BEFORE -> true; default -> { childHelper.fail(RulerColumnMessages.RulerColumnPlacement_illegal_child_msg); yield false; } }; continue;

Let me check on it further.

Reverting this to original switch, thanks for the suggestion.

iloveeclipse · 2025-02-25T12:58:56Z

I'm not sure the change improves readability.
Is this PR manually crafted of is the result of automated quick fix?

deepika-u · 2025-02-25T13:01:02Z

I'm not sure the change improves readability. Is this PR manually crafted of is the result of automated quick fix?

I have did it manually.

iloveeclipse · 2025-02-25T13:02:34Z

I have did it manually

Why not with a cleanup? Or there is nothing available?

deepika-u · 2025-02-25T13:08:10Z

I have did it manually

Why not with a cleanup? Or there is nothing available?

Any specific context of "cleanup" in this code are you referring to?

iloveeclipse

You can perform changes via Source->Cleanup... and the cleanup of your choice.

The proposed change breaks code.

iloveeclipse · 2025-02-25T13:13:56Z

...workbench.texteditor/src/org/eclipse/ui/internal/texteditor/rulers/RulerColumnPlacement.java

+				case BEFORE -> true;
+				default -> {
+					childHelper.fail(RulerColumnMessages.RulerColumnPlacement_illegal_child_msg);
+					yield false; // Required because default must return a value.


github-actions · 2025-02-25T13:33:20Z

Test Results

1 821 files ±0 1 821 suites ±0 1h 34m 22s ⏱️ + 3m 5s
7 900 tests ±0 7 671 ✅ - 1 228 💤 ±0 1 ❌ +1
23 787 runs ±0 23 038 ✅ - 1 748 💤 ±0 1 ❌ +1

For more details on these failures, see this check.

Results for commit 78f65c3. ± Comparison against base commit 60b8674.

♻️ This comment has been updated with latest results.

deepika-u · 2025-02-26T10:20:50Z

You can perform changes via Source->Cleanup... and the cleanup of your choice.

Thanks for the suggestion, even via this that file is not getting picked and updated. So i have reverted that file change(switch to original code).
Can you review now please?

iloveeclipse · 2025-02-26T13:03:19Z

even via this that file is not getting picked and updated

That is the sign that the quick fix doesn't have an automated solution for conversion (yet).

Can you review now please?

The new change shows some new warning, shows files with whitespace only changes. That shouldn't happen.

I only briefly checked original PR whether the proposed change make sense, found immediately a problem and beside the problem nothing interesting in the change itself. Approving a PR means I agree with the change, which is not the case here.

I would propose to close this PR, and if you are eager to apply this particular refactoring on this particular bundle, do this with the automated cleanup and only with that.

For the new PR please find someone who is interested in such kind of reviews and let review it. I'm definitely not interested in this particular change.

eclipse-platform-bot · 2025-03-06T15:30:22Z

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

bundles/org.eclipse.ui.workbench.texteditor/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 30eecdfa074bfd90c87cf2ae676f84d44a636f92 Mon Sep 17 00:00:00 2001
From: Eclipse Platform Bot <[email protected]>
Date: Thu, 6 Mar 2025 15:30:01 +0000
Subject: [PATCH] Version bump(s) for 4.36 stream


diff --git a/bundles/org.eclipse.ui.workbench.texteditor/META-INF/MANIFEST.MF b/bundles/org.eclipse.ui.workbench.texteditor/META-INF/MANIFEST.MF
index 2e7b7c5380..1115872cdf 100644
--- a/bundles/org.eclipse.ui.workbench.texteditor/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.ui.workbench.texteditor/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %pluginName
 Bundle-SymbolicName: org.eclipse.ui.workbench.texteditor; singleton:=true
-Bundle-Version: 3.19.100.qualifier
+Bundle-Version: 3.19.200.qualifier
 Bundle-Activator: org.eclipse.ui.internal.texteditor.TextEditorPlugin
 Bundle-ActivationPolicy: lazy
 Bundle-Vendor: %providerName
-- 
2.48.1

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

iloveeclipse reviewed Feb 25, 2025

View reviewed changes

iloveeclipse requested changes Feb 25, 2025

View reviewed changes

deepika-u force-pushed the Modernize_with_java17_switch_expression branch from 4773319 to ef4610d Compare February 26, 2025 08:06

deepika-u force-pushed the Modernize_with_java17_switch_expression branch from ef4610d to 8bb2889 Compare February 27, 2025 09:40

Modernize to Java 17 switch expression.Added default case.

2411bcb

akurtakov force-pushed the Modernize_with_java17_switch_expression branch from 8bb2889 to 2411bcb Compare March 6, 2025 15:25

Version bump(s) for 4.36 stream

78f65c3

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modernize to Java 17 switch expression. #2816

Modernize to Java 17 switch expression. #2816

deepika-u commented Feb 25, 2025

iloveeclipse Feb 25, 2025

deepika-u Feb 25, 2025

iloveeclipse Feb 25, 2025

deepika-u Feb 25, 2025

iloveeclipse Feb 25, 2025

deepika-u Feb 25, 2025

deepika-u Feb 26, 2025 •

edited

Loading

iloveeclipse commented Feb 25, 2025

deepika-u commented Feb 25, 2025

iloveeclipse commented Feb 25, 2025

deepika-u commented Feb 25, 2025

iloveeclipse left a comment

iloveeclipse Feb 25, 2025

github-actions bot commented Feb 25, 2025 •

edited

Loading

deepika-u commented Feb 26, 2025 •

edited

Loading

iloveeclipse commented Feb 26, 2025

eclipse-platform-bot commented Mar 6, 2025

Modernize to Java 17 switch expression. #2816

Are you sure you want to change the base?

Modernize to Java 17 switch expression. #2816

Conversation

deepika-u commented Feb 25, 2025

iloveeclipse Feb 25, 2025

Choose a reason for hiding this comment

deepika-u Feb 25, 2025

Choose a reason for hiding this comment

iloveeclipse Feb 25, 2025

Choose a reason for hiding this comment

deepika-u Feb 25, 2025

Choose a reason for hiding this comment

iloveeclipse Feb 25, 2025

Choose a reason for hiding this comment

deepika-u Feb 25, 2025

Choose a reason for hiding this comment

deepika-u Feb 26, 2025 • edited Loading

Choose a reason for hiding this comment

iloveeclipse commented Feb 25, 2025

deepika-u commented Feb 25, 2025

iloveeclipse commented Feb 25, 2025

deepika-u commented Feb 25, 2025

iloveeclipse left a comment

Choose a reason for hiding this comment

iloveeclipse Feb 25, 2025

Choose a reason for hiding this comment

github-actions bot commented Feb 25, 2025 • edited Loading

Test Results

deepika-u commented Feb 26, 2025 • edited Loading

iloveeclipse commented Feb 26, 2025

eclipse-platform-bot commented Mar 6, 2025

deepika-u Feb 26, 2025 •

edited

Loading

github-actions bot commented Feb 25, 2025 •

edited

Loading

deepika-u commented Feb 26, 2025 •

edited

Loading