Skip to content

Refactor NLS messages #612

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
awisniew90 wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Refactor NLS messages #612

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
26a0cd9
Update message props structure
awisniew90 Jan 5, 2026
c72ba15
Remove duplicate trace messages
awisniew90 Jan 5, 2026
218fc5d
Remove hardcoded messages
awisniew90 Jan 5, 2026
f6ff615
Refactor messages
awisniew90 Jan 5, 2026
474a11f
Fix typos and duplicate messages
awisniew90 Jan 5, 2026
10a2825
Remove unnecessary null checks
awisniew90 Jan 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions bundles/io.openliberty.tools.eclipse.ui/src/io/openliberty/tools/eclipse/CommandBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,9 @@ private String getCommand() throws CommandBuilder.CommandNotFoundException {
}

if (isMaven) {
ErrorHandler.processPreferenceErrorMessage(NLS.bind(Messages.maven_exec_not_found, null), true);
ErrorHandler.processPreferenceErrorMessage(Messages.getMessage("maven_exec_not_found"), true);
} else {
ErrorHandler.processPreferenceErrorMessage(NLS.bind(Messages.gradle_exec_not_found, null), true);
ErrorHandler.processPreferenceErrorMessage(Messages.getMessage("gradle_exec_not_found"), true);
}

throw new CommandNotFoundException(errorMsg);
Expand Down
91 changes: 45 additions & 46 deletions ...s/io.openliberty.tools.eclipse.ui/src/io/openliberty/tools/eclipse/DevModeOperations.java
View file
Open in desktop

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions bundles/io.openliberty.tools.eclipse.ui/src/io/openliberty/tools/eclipse/Project.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ public List<Project> filterJavaProjects(Set<Project> projects) {
javaProjecs.add(child);
}
} catch (CoreException e) {
ErrorHandler.processWarningMessage(NLS.bind(Messages.determine_java_project_error, child.getName()), e, false);
ErrorHandler.processWarningMessage(Messages.getMessage("determine_java_project_error", child.getName()), e, false);
}
}
return javaProjecs;
Expand Down Expand Up @@ -269,7 +269,7 @@ public void classifyAsServerModule() {
libertyServerModule = false;
}
} catch (Exception e) {
ErrorHandler.processWarningMessage(NLS.bind(Messages.liberty_nature_add_error, null), e, false);
ErrorHandler.processWarningMessage(Messages.getMessage("liberty_nature_add_error"), e, false);
}
}

Expand All @@ -292,7 +292,7 @@ public void classifyAsLibertyNature() {
}
}
} catch (Exception e) {
ErrorHandler.processWarningMessage(NLS.bind(Messages.liberty_nature_add_error, null), e, false);
ErrorHandler.processWarningMessage(Messages.getMessage("liberty_nature_add_error"), e, false);
}
}

Expand Down
2 changes: 1 addition & 1 deletion ...openliberty.tools.eclipse.ui/src/io/openliberty/tools/eclipse/WorkspaceProjectsModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ private void buildMultiProjectModel(List<IProject> projectsToScan, boolean class
if (Trace.isEnabled()) {
Trace.getTracer().trace(Trace.TRACE_TOOLS, msg + " Workspace projects: " + projectsByLocation.values(), e);
}
ErrorHandler.processWarningMessage(NLS.bind(Messages.project_analyze_error, null), e, false);
ErrorHandler.processWarningMessage(Messages.getMessage("project_analyze_error"), e, false);
}

if (Trace.isEnabled()) {
Expand Down
21 changes: 8 additions & 13 deletions ...openliberty.tools.eclipse.ui/src/io/openliberty/tools/eclipse/debug/DebugModeHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
import io.openliberty.tools.eclipse.Project;
import io.openliberty.tools.eclipse.Project.BuildType;
import io.openliberty.tools.eclipse.logging.Trace;
import io.openliberty.tools.eclipse.messages.Messages;
import io.openliberty.tools.eclipse.ui.dashboard.DashboardView;
import io.openliberty.tools.eclipse.utils.ErrorHandler;
import io.openliberty.tools.eclipse.utils.Utils;
Expand Down Expand Up @@ -133,8 +134,7 @@ public String addDebugDataToStartParms(Project project, String debugPort, String
addendum = GRADLE_DEVMODE_DEBUG_PORT_PARM + "=" + debugPort;
}
} else {
throw new Exception("Unexpected project build type: " + buildType + ". Project" + project.getIProject().getName()
+ "does not appear to be a Maven or Gradle built project.");
throw new Exception(Messages.getMessage("unexpected_build_type", buildType, project.getIProject().getName()));
}
}

Expand Down Expand Up @@ -175,8 +175,7 @@ public String calculateDebugPort(Project project, String inputParms) throws Exce
} else if (buildType == BuildType.GRADLE) {
searchKey = GRADLE_DEVMODE_DEBUG_PORT_PARM;
} else {
throw new Exception("Unexpected project build type: " + buildType + ". Project " + project.getIProject().getName()
+ "does not appear to be a Maven or Gradle built project.");
throw new Exception(Messages.getMessage("unexpected_build_type", buildType, project.getIProject().getName()));
}

if (inputParms.contains(searchKey)) {
Expand Down Expand Up @@ -218,7 +217,7 @@ public String calculateDebugPort(Project project, String inputParms) throws Exce
public void startDebugAttacher(Project project, ILaunch launch, String port) {
String projectName = project.getIProject().getName();

Job job = new Job("Attaching Debugger to JVM...") {
Job job = new Job(Messages.getMessage("attaching_debugger_job")) {
@Override
protected IStatus run(IProgressMonitor monitor) {
try {
Expand Down Expand Up @@ -254,7 +253,7 @@ protected IStatus run(IProgressMonitor monitor) {
if (debugPort == null) {
// We failed to read the debug port. Throw an exception. This will be caught by the outer
// catch block and the job will return with an error.
String errorMessage = "Failed to read debug port from server.env file";
String errorMessage = Messages.getMessage("debug_port_read_error");
if (ex[0] != null) {
// Add the last exception we got.
throw new Exception(errorMessage, ex[0]);
Expand Down Expand Up @@ -282,7 +281,7 @@ protected IStatus run(IProgressMonitor monitor) {

} catch (Exception e) {
return new Status(IStatus.ERROR, LibertyDevPlugin.PLUGIN_ID, JOB_STATUS_DEBUGGER_CONN_ERROR,
"An error was detected while attaching the debugger to the JVM.", e);
Messages.getMessage("debugger_attach_error"), e);
}

return Status.OK_STATUS;
Expand Down Expand Up @@ -413,7 +412,7 @@ private VirtualMachine attachJVM(String hostName, int port, AttachingConnector c
} catch (IOException e) {
if (Trace.isEnabled()) {
Trace.getTracer().trace(Trace.TRACE_UI,
"Error occured while trying to connect to the remote virtual machine " + e.getMessage(), e);
Messages.getMessage("debugger_connect_error", e.getMessage()), e);
}
} catch (TimeoutException e2) {
// do nothing
Expand Down Expand Up @@ -486,8 +485,6 @@ private void openDebugPerspective() {
*/
private Path getServerEnvFile(Project project) throws Exception {



Path libertyPluginConfigXmlPath = devModeOps.getLibertyPluginConfigXmlPath(project);

// Read server.env path from liberty-plugin-config.xml
Expand Down Expand Up @@ -593,11 +590,9 @@ private String waitForSocketActivation(Project project, String host, String port
}
}

throw new Exception("Timed out trying to attach the debugger to JVM on host: " + host + " and port: " + port
+ ". If the server starts later you might try to manually connect the debugger from the launch in the Debug view You can confirm the debug port used in the console output looking for a message like 'Liberty debug port: [ 63624 ]'.");
throw new Exception(Messages.getMessage("debugger_timeout_error", host, port));
}


private class DataHolder {
boolean started;
}
Expand Down
4 changes: 2 additions & 2 deletions ...tools.eclipse.ui/src/io/openliberty/tools/eclipse/debug/LibertyDebugReconnectHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,11 @@ public Object execute(ExecutionEvent event) throws ExecutionException {
try {
projectName = launch.getLaunchConfiguration().getAttribute(StartTab.PROJECT_NAME, "");
} catch (CoreException e) {
String msg = "An error was detected during debugger reconnect";
String msg = Messages.getMessage("project_name_error");
if (Trace.isEnabled()) {
Trace.getTracer().trace(Trace.TRACE_UI, msg, e);
}
ErrorHandler.processErrorMessage(NLS.bind(Messages.project_name_error, null), e, true);
ErrorHandler.processErrorMessage(msg, e, true);
}

if (projectName != null && !projectName.isBlank()) {
Expand Down
3 changes: 2 additions & 1 deletion ...nliberty.tools.eclipse.ui/src/io/openliberty/tools/eclipse/handlers/DashboardHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import org.eclipse.ui.handlers.HandlerUtil;

import io.openliberty.tools.eclipse.logging.Trace;
import io.openliberty.tools.eclipse.messages.Messages;

public class DashboardHandler extends AbstractHandler {

Expand All @@ -26,7 +27,7 @@ public Object execute(ExecutionEvent event) throws ExecutionException {
try {
HandlerUtil.getActiveWorkbenchWindow(event).getActivePage().showView("io.openliberty.tools.eclipse.views.liberty.devmode.dashboard");
} catch (Exception e) {
String msg = "Unable to open the Liberty dashboard view";
String msg = Messages.getMessage("dashboard_open_error");
if (Trace.isEnabled()) {
Trace.getTracer().trace(Trace.TRACE_HANDLERS, msg, e);
}
Expand Down
8 changes: 4 additions & 4 deletions ...berty.tools.eclipse.ui/src/io/openliberty/tools/eclipse/handlers/ExplorerMenuHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public Object execute(ExecutionEvent event) throws ExecutionException {
Trace.getTracer().trace(Trace.TRACE_HANDLERS, msg);
}

ErrorHandler.processErrorMessage(NLS.bind(Messages.project_not_valid, null), true);
ErrorHandler.processErrorMessage(Messages.getMessage("project_not_valid"), true);

return null;
}
Expand All @@ -68,12 +68,12 @@ public Object execute(ExecutionEvent event) throws ExecutionException {
try {
commandName = command.getName();
} catch (Exception e) {
String msg = "Unable to retrieve menu command.";
String msg = Messages.getMessage("menu_command_retrieve_error");
if (Trace.isEnabled()) {
Trace.getTracer().trace(Trace.TRACE_HANDLERS, msg, e);
}

ErrorHandler.processErrorMessage(NLS.bind(Messages.menu_command_retrieve_error, null), e, true);
ErrorHandler.processErrorMessage(msg, e, true);
return null;
}

Expand Down Expand Up @@ -106,7 +106,7 @@ public Object execute(ExecutionEvent event) throws ExecutionException {
}

ErrorHandler.processErrorMessage(
NLS.bind(Messages.menu_command_process_error, new String[] { commandName, iProject.getName() }), e);
Messages.getMessage("menu_command_process_error", commandName, iProject.getName()), e);
}
}

Expand Down
123 changes: 45 additions & 78 deletions ...s/io.openliberty.tools.eclipse.ui/src/io/openliberty/tools/eclipse/messages/Messages.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,89 +10,56 @@
*******************************************************************************/
package io.openliberty.tools.eclipse.messages;

import org.eclipse.osgi.util.NLS;
import java.text.MessageFormat;
import java.util.Locale;
import java.util.ResourceBundle;

/**
* Translated messages.
* Provides access to NLS Messages.
*/
public class Messages extends NLS {
public class Messages {

/** DebugModeHandler */
public static String multiple_server_env;

/** DevModeOperations */
public static String start_no_project_found;
public static String start_already_issued;
public static String start_general_error;

public static String start_container_no_project_found;
public static String start_container_already_issued;
public static String start_container_general_error;

public static String stop_no_project_found;
public static String stop_already_issued;
public static String stop_general_error;

public static String run_tests_no_project_found;
public static String run_tests_no_prior_start;
public static String run_tests_general_error;

public static String mvn_int_test_report_no_project_found;
public static String mvn_int_test_report_none_found;
public static String mvn_int_test_report_general_error;

public static String mvn_unit_test_report_no_project_found;
public static String mvn_unit_test_report_none_found;
public static String mvn_unit_test_report_general_error;

public static String gradle_test_report_no_project_found;
public static String gradle_test_report_none_found;
public static String gradle_test_report_general_error;

public static String issue_stop_prompt;
public static String plugin_stop_issue_error;
public static String plugin_stop_timeout;
public static String plugin_stop_failed;
public static String plugin_stop_general_error;

/** WorkspaceProjectsModel */
public static String project_analyze_error;

/** DashboardView */
public static String project_not_gradle_or_maven;
public static String image_descriptions_error;
public static String action_general_error;
public static String dashboard_refresh_error;

/** JRETab */
public static String java_default_set_error;
public static String java_resolution_error;

/** LaunchConfigurationDelegateLauncher */
public static String launch_config_error;

/** StartTab */
public static String start_parm_retrieve_error;
public static String project_name_error;
public static String run_config_initialize_error;

/** All *Action classes */
public static String launch_shortcut_error;

/** ExplorerMenuHandler */
public static String project_not_valid;
public static String menu_command_retrieve_error;
public static String menu_command_process_error;

/** CommandBuilder */
public static String maven_exec_not_found;
public static String gradle_exec_not_found;
/** Message Resource bundle */
private static ResourceBundle NLS_BUNDLE;
static {
try {
NLS_BUNDLE = ResourceBundle.getBundle("io.openliberty.tools.eclipse.messages.Messages", Locale.getDefault());
} catch (Exception e) {
NLS_BUNDLE = ResourceBundle.getBundle("io.openliberty.tools.eclipse.messages.Messages");
Copy link
Contributor

@ajm01 ajm01 Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a chance that both of these inti's could fail? at that point NLS_BUNDLE would be null - should we initialize it to an empty ResourceBundle?

Copy link
Contributor Author

@awisniew90 awisniew90 Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, there shouldnt be a chance of that since we are using the fully qualified classname for this class. If the class is running within the bundle, we should be able to lookup the bundle based on the class.

}
}

/** Project */
public static String determine_java_project_error;
public static String liberty_nature_add_error;
/**
* Returns a translated message with no arguments.
*
* @param key The message Key.
*
* @return The translated message without arguments.
*/
public static String getMessage(String key) {
return getMessage(key, (Object) null);
}

static {
NLS.initializeMessages("io.openliberty.tools.eclipse.messages.Messages", Messages.class);
/**
* Returns a translated message with arguments.
*
* @param key The message key.
* @param args The arguments associated with the message.
*
* @return A translated message with arguments.
*/
public static String getMessage(String key, Object... args) {
String msg = null;

try {
msg = NLS_BUNDLE.getString(key);
if (msg != null && args != null) {
msg = MessageFormat.format(msg, args);
}
} catch (Exception e) {
Copy link
Contributor

@ajm01 ajm01 Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we catch more specific exceptions here to give a finer grained reason for failure?

Copy link
Contributor Author

@awisniew90 awisniew90 Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think in all cases, the reason is pretty much that there is an issue with that particular message property. Either the key being used is wrong, or the value in the properties file is wrong. In either case, it's a bug in our code and the failed message lookup should be enough to indicate that.

msg = key;
}

return msg;
Copy link
Contributor

@ajm01 ajm01 Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this could be null - if msg = NLS_BUNDLE.getString(key); on line 55 gets a null back - could that cause an issue in the caller?

Copy link
Contributor Author

@awisniew90 awisniew90 Jan 8, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the NLS_BUNDLE.getString() method, we actually should never get a null back. If the String object is null, that method throws an exception which we will catch and set the msg to the provided key. Given that, we probably dont need the null check afterwards either.

}
}
Loading
Loading