Skip to content

Use ILog.of in IDEApplication version-file logger#17

Merged
vogella merged 3 commits into
masterfrom
cleanup/status-util-misc
Apr 27, 2026
Merged

Use ILog.of in IDEApplication version-file logger#17
vogella merged 3 commits into
masterfrom
cleanup/status-util-misc

Conversation

@vogella
Copy link
Copy Markdown
Owner

@vogella vogella commented Apr 22, 2026

Migrates the single cross-bundle StatusUtil.newError(e) in IDEApplication#writeVersion to ILog.of(IDEApplication.class).error(message, throwable). The logged plug-in id moves from "org.eclipse.ui.ide" to "org.eclipse.ui.ide.application" — the bundle that actually emits the version-file write failure — and the previous double-log emitted by IDEWorkbenchPlugin.log(message, status) collapses to a single coherent entry. As a side effect the dependency on IOException.getLocalizedMessage() (potentially null) goes away.

Tracked in vogellacompany/tasks#1837. The larger org.eclipse.ui.workbench migration is still blocked on the open plug-in-id decision and is not in this PR.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request removes internal StatusUtil dependencies in IDEApplication and WorkbenchActivitySupport, replacing them with standard Status or plugin-specific logging. A suggestion was made to simplify the logging in IDEApplication by passing the context message directly to Status.error to handle potential null localized messages and maintain the correct plugin ID.

Comment on lines +838 to +839
IDEWorkbenchPlugin.log("Could not write version file", //$NON-NLS-1$
StatusUtil.newError(e));
Status.error(e.getLocalizedMessage(), e));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The logging call can be simplified by passing the context message directly to Status.error(...). This ensures the message is correctly associated with the status and avoids potential issues if e.getLocalizedMessage() returns null. Furthermore, using the log(IStatus) overload ensures that the plug-in ID set by Status.error (the application bundle) is preserved, which aligns with the stated goal of relocating the logged plug-in ID.

            IDEWorkbenchPlugin.log(Status.error("Could not write version file", e)); //$NON-NLS-1$

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Applied with one adjustment: IDEWorkbenchPlugin doesn't expose a log(IStatus) single-arg overload (only log(String), log(String, Throwable), log(Class, String, Throwable), log(String, IStatus)), so the literal suggestion wouldn't compile. I went with the equivalent that satisfies the same intent and matches the recent b51da6b9eca migration:

ILog.of(IDEApplication.class).error("Could not write version file", e); //$NON-NLS-1$

This produces one log entry (instead of the two emitted by the previous IDEWorkbenchPlugin.log(message, status) helper), preserves the context message regardless of e.getLocalizedMessage(), and keeps the plug-in id at org.eclipse.ui.ide.application.

@vogella vogella force-pushed the cleanup/status-util-misc branch from 3de8563 to 3d7b588 Compare April 23, 2026 05:29
@vogella vogella changed the title StatusUtil cleanup: IDEApplication + WorkbenchActivitySupport discarded-status fix Use ILog.of in IDEApplication version-file logger Apr 23, 2026
vogella added 3 commits April 23, 2026 16:00
IDEApplication.start set the splash shell's taskbar title via
ChooseWorkspaceDialog.getWindowTitle(). That helper is scoped to the
workspace chooser dialog: it returns Platform.getProduct().getName(),
and falls back to an IDEWorkbenchMessages string whose name
(ChooseWorkspaceDialog_defaultProductName) is tied to the chooser
dialog's presentation.

Using it for the splash shell is a layering inversion. The splash is
a property of the application, not of the chooser dialog, and the
dialog-specific NLS fallback is not appropriate for the splash
taskbar entry.

Read Platform.getProduct().getName() directly in IDEApplication. If
no product is configured, leave the launcher's default title in place
rather than substituting a chooser-dialog placeholder.
ChooseWorkspaceDialog.getWindowTitle() is retained as an internal
helper for the dialog itself.

No user-visible change for Eclipse IDE products, which always have a
product name configured.
Replaces the cross-bundle call to org.eclipse.ui.internal.ide.StatusUtil
.newError(...) in IDEApplication#writeVersion with ILog.of(IDEApplication
.class).error(message, throwable), and drops the now-unused StatusUtil
import.

Behavior changes:
- Plug-in id moves from "org.eclipse.ui.ide" (IDE_WORKBENCH constant in
  the called StatusUtil) to "org.eclipse.ui.ide.application", which is
  the bundle that actually emits the version-file write failure.
- The previous IDEWorkbenchPlugin.log(message, status) helper emitted
  two separate log entries (one for the message, one for the status);
  this now produces a single coherent entry whose status carries both
  the context message and the throwable, removing the dependency on the
  potentially-null e.getLocalizedMessage().
Give the preference dialog's filter field the full set of search-related
style bits (SWT.SEARCH | SWT.ICON_SEARCH | SWT.ICON_CANCEL) so it is
rendered as a native search field with a magnifier and cancel icon.

Also override setInitialText to expose the hint via Text#setMessage
only, instead of stuffing it into the field's content. The dialog
auto-focuses the filter field on open, so the old initial-text fallback
was never actually shown as a placeholder and forced users to delete
the hint before typing.
@vogella vogella force-pushed the cleanup/status-util-misc branch from 3d7b588 to ba64dcd Compare April 24, 2026 04:03
@vogella vogella merged commit ba64dcd into master Apr 27, 2026
4 checks passed
@vogella vogella deleted the cleanup/status-util-misc branch April 27, 2026 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant