Reverting some message logging changes in JavaToolMonitor

[unv-unv]

No description provided.

[unv-unv]

@claude review pr with inline comments

[github-actions]

Claude finished @unv-unv's task in 3m 16s —— View job

---

PR Review: Reverting some message logging changes in JavaToolMonitor

• Read the diff and the full file
• Analyze the refactoring
• Post inline review comment

I left an inline comment on the change. Summary below.

Overall

The refactoring is solid. Collapsing logInfo / logError / logWarning into a single logMessage(category, …) helper removes the triplicated builder code, and routing through newMessage(category, …).optionalUrl(fileUrl) correctly preserves the original behavior — the URI → VirtualFile resolution is kept, and optionalUrl gracefully handles the null case when the file can't be resolved. 👍

One actionable finding — dead code

With the three log* methods no longer calling new MessageBuilderWrapper(...), the inner class MessageBuilderWrapper (lines 171–205) is now unused. I confirmed there are no remaining new MessageBuilderWrapper(...) call sites anywhere in the repo, and its only logic (URI resolution via VirtualFileUtil.findFileByURL) is now inlined into logMessage. It can be removed in this PR.

Note: lines 171–205 fall outside the diff hunks, so GitHub wouldn't let me attach a one-click suggestion block there — the class needs to be deleted manually.

Remove the dead class →

Minor notes (non-blocking)

• position((int) lineNumber, (int) columnNumber) narrows long → int. This is pre-existing (unchanged by this PR), so no action needed here — just noting it.
• catch (Exception ignored) swallows URI/URL parsing failures silently, which matches the prior behavior, so it's fine to keep.

Nothing blocking — once the dead MessageBuilderWrapper is removed, this looks good to merge.
· branch java-tool-monitor-update

[github-actions]

Extracting a shared logMessage(...) helper is a nice cleanup, and the URI→VirtualFile resolution is correctly preserved here (now feeding optionalUrl, which tolerates a null).

One follow-up: this new method inlines exactly what MessageBuilderWrapper.url(...) used to do. With logInfo/logError/logWarning no longer constructing it, the inner MessageBuilderWrapper class (lines 171–205) now has no remaining new MessageBuilderWrapper(...) call sites anywhere in the codebase — it is dead code and can be deleted in this PR.

(Those lines are outside this diff hunk, so a one-click suggestion cannot be attached; please remove the class manually.)

[unv-unv]

@claude review pr with inline comments

[github-actions]

Claude finished @unv-unv's task in 1m 53s —— View job

---

PR Review: Reverting some message logging changes in JavaToolMonitor

• Read the diff and the full file
• Verify the MessageBuilderWrapper dead-code concern from the prior review
• Check imports for orphans after the refactor
• Post inline review comment

I left an inline comment on the change.

Overall

The refactoring is clean. Collapsing logInfo / logError / logWarning into one logMessage(category, …) removes the triplicated builder code, and routing through newMessage(category, …).optionalUrl(fileUrl) preserves the original behavior — the URI → VirtualFile resolution is kept and optionalUrl tolerates the null case when the file can't be resolved. 👍

On the prior review's "dead code" note

The earlier review flagged MessageBuilderWrapper as leftover dead code. That's already resolved in this diff — the class is fully deleted (replaced by logMessage), and grep confirms no MessageBuilderWrapper references remain anywhere in the repo. No action needed.

One actionable finding — unused import

MessageBuilderWrapper.navigatable(Navigatable) was the only user of import consulo.navigation.Navigatable; (line 12). With the wrapper gone, that import is now unused and can be removed. All other imports (VirtualFile, VirtualFileUtil, URI) are still used in logMessage.

The import line is outside the diff hunks, so GitHub wouldn't let me attach a one-click suggestion block there — it needs to be deleted manually. Remove it →

Minor note (non-blocking)

• position((int) lineNumber, (int) columnNumber) narrows long → int. Pre-existing, unchanged by this PR — just noting it.

Nothing blocking — once the unused Navigatable import is removed, this looks good to merge.
· branch java-tool-monitor-update

[github-actions]

Nice cleanup — collapsing logInfo/logError/logWarning into a single logMessage(category, …) and routing through newMessage(category, …).optionalUrl(fileUrl) preserves the original URI→VirtualFile resolution while removing the triplicated builder code. The MessageBuilderWrapper class is fully removed in this diff, so the dead-code concern is already handled. 👍

One small leftover: now that MessageBuilderWrapper.navigatable(Navigatable) is gone, the import consulo.navigation.Navigatable; (line 12) is unused and can be removed — it has no other reference in this file. (That line is outside the diff hunks, so a one-click suggestion block cannot be attached here.)