feat(core): 支持JDK Tool ActionGroup #2130
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Close #2078 Signed-off-by: unknowIfGuestInDream <liang.tang.cx@gmail.com>
|
Thank you for following naming conventions! 😻 |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Their most recently public accepted PR is: #2129 |
Reviewer's GuideThis pull request introduces a new "JDK Tool" action group to the menubar by creating a dedicated Sequence diagram for JDK Tool menu creationsequenceDiagram
participant Provider as MenubarConfigProvider
participant JDKToolGroup as FxJDKToolActionGroup
participant ActionGroupFactory as FxActionGroup
Provider->>+JDKToolGroup: new FxJDKToolActionGroup()
Note right of JDKToolGroup: Finds tool paths, creates Actions (jmc, jconsole, etc.)
JDKToolGroup-->>-Provider: instance
Provider->>+JDKToolGroup: create()
JDKToolGroup->>+ActionGroupFactory: jdkTool(jmcAction, jconsoleAction, ...)
ActionGroupFactory-->>-JDKToolGroup: jdkToolGroup (ActionGroup)
JDKToolGroup-->>-Provider: jdkToolGroup
Provider->>+ActionGroupFactory: tool(jdkToolGroup, ..., otherTools)
ActionGroupFactory-->>-Provider: mainToolGroup (ActionGroup)
Note left of Provider: Adds mainToolGroup to menubar actions
Class diagram for JDK Tool ActionGroup featureclassDiagram
class FxActionGroup {
+static ActionGroup jdkTool(Action... actions)
}
class FxJDKToolActionGroup {
-Action jmc
-Action jconsole
-Action jvisualvm
-String javaHome
-String javaHomeEnv
-String jmcRelativePath
-String jconsoleRelativePath
-String jvisualvmRelativePath
+FxJDKToolActionGroup()
-void runProgram(String filePath)
+ActionGroup create()
}
note for FxJDKToolActionGroup "Handles finding and launching JDK tools (jmc, jconsole, jvisualvm). \nChecks java.home and JAVA_HOME."
class QeMenubarConfigrationProvider {
-ActionGroup jdkToolGroup
-Collection~Action~ actions
+QeMenubarConfigrationProvider()
}
note for QeMenubarConfigrationProvider "Integrates FxJDKToolActionGroup into the Tool menu."
class SmcMenubarConfigrationProvider {
-ActionGroup jdkToolGroup
-Collection~Action~ actions
+SmcMenubarConfigrationProvider()
}
note for SmcMenubarConfigrationProvider "Integrates FxJDKToolActionGroup into the Tool menu."
class ActionGroup
class Action
class OSUtil
class LayoutHelper
class ProcessBuilder
class ExceptionDialog
class StaticLog
QeMenubarConfigrationProvider --> FxJDKToolActionGroup : uses
SmcMenubarConfigrationProvider --> FxJDKToolActionGroup : uses
FxJDKToolActionGroup --> FxActionGroup : uses
FxJDKToolActionGroup --> Action : creates
FxJDKToolActionGroup --> OSUtil : uses
FxJDKToolActionGroup --> LayoutHelper : uses
FxJDKToolActionGroup --> ProcessBuilder : uses
FxJDKToolActionGroup --> ExceptionDialog : uses
FxJDKToolActionGroup --> StaticLog : uses
FxJDKToolActionGroup ..> ActionGroup : creates via create()
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
""" Walkthrough本次变更为应用的菜单栏新增了“JDK工具”功能组。通过引入新的 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MenuBar
participant FxJDKToolActionGroup
participant OS
participant JDK Tool
User->>MenuBar: 点击“JDK工具”菜单
MenuBar->>FxJDKToolActionGroup: 展示工具项（jmc, jconsole, jvisualvm）
User->>MenuBar: 选择某个 JDK 工具
MenuBar->>FxJDKToolActionGroup: 调用对应 Action
FxJDKToolActionGroup->>OS: 检查工具路径并尝试启动进程
OS-->>JDK Tool: 启动工具进程
JDK Tool-->>User: 展示工具界面
Assessment against linked issues
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (9)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @unknowIfGuestInDream - I've reviewed your changes - here's some feedback:
- Consider refactoring the repeated logic for finding JDK tool paths within
FxJDKToolActionGroupinto a reusable helper method.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
core/src/main/java/com/tlcsdm/core/javafx/controlsfx/FxJDKToolActionGroup.java
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
core/src/main/java/com/tlcsdm/core/javafx/controlsfx/FxJDKToolActionGroup.java (2)
45-54: 建议改进字段访问修饰符Action字段（jmc、jconsole、jvisualvm）当前使用的是默认（包级别）访问修饰符，但这些字段只在类内部使用。
建议将这些字段声明为私有：
- Action jmc; - Action jconsole; - Action jvisualvm; + private final Action jmc; + private final Action jconsole; + private final Action jvisualvm;
55-123: 构造函数复杂度较高，建议重构当前构造函数包含初始化三个工具操作的重复逻辑，导致方法较长且复杂。静态代码分析工具已标记此方法为"复杂方法"。
建议将初始化每个工具的逻辑提取到单独的方法中，简化构造函数：
public FxJDKToolActionGroup() { boolean isWin = OSUtil.getOS().equals(OSUtil.OS.WINDOWS); jmcRelativePath = "bin" + File.separator + (isWin ? "jmc.exe" : "jmc"); jconsoleRelativePath = "bin" + File.separator + (isWin ? "jconsole.exe" : "jconsole"); jvisualvmRelativePath = "bin" + File.separator + (isWin ? "jvisualvm.exe" : "jvisualvm"); - jmc = new Action("jmc", e -> { - File file = new File(javaHome, jmcRelativePath); - if (file.exists()) { - runProgram(file.getAbsolutePath()); - } else { - file = new File(javaHomeEnv, jmcRelativePath); - if (!file.exists()) { - StaticLog.warn("Not found jmc."); - return; - } - runProgram(file.getAbsolutePath()); - } - }); - jmc.setGraphic( - LayoutHelper.iconView(FxJDKToolActionGroup.class.getResource("/com/tlcsdm/core/static/icon/jmc.png"))); + jmc = createToolAction("jmc", jmcRelativePath, "/com/tlcsdm/core/static/icon/jmc.png"); - jconsole = new Action("jConsole", e -> { - File file = new File(javaHome, jconsoleRelativePath); - if (file.exists()) { - runProgram(file.getAbsolutePath()); - } else { - file = new File(javaHomeEnv, jconsoleRelativePath); - if (!file.exists()) { - StaticLog.warn("Not found jconsole."); - return; - } - runProgram(file.getAbsolutePath()); - } - }); - jconsole.setGraphic( - LayoutHelper.iconView(FxJDKToolActionGroup.class.getResource("/com/tlcsdm/core/static/icon/java.png"))); + jconsole = createToolAction("jConsole", jconsoleRelativePath, "/com/tlcsdm/core/static/icon/java.png"); - jvisualvm = new Action("jvisualvm", e -> { - File file = new File(javaHome, jvisualvmRelativePath); - if (file.exists()) { - runProgram(file.getAbsolutePath()); - } else { - file = new File(javaHomeEnv, jvisualvmRelativePath); - if (!file.exists()) { - StaticLog.warn("Not found jvisualvm."); - return; - } - runProgram(file.getAbsolutePath()); - } - }); - jvisualvm.setGraphic( - LayoutHelper.iconView( - FxJDKToolActionGroup.class.getResource("/com/tlcsdm/core/static/icon/jvisualvm.png"))); + jvisualvm = createToolAction("jvisualvm", jvisualvmRelativePath, "/com/tlcsdm/core/static/icon/jvisualvm.png"); File file1 = new File(javaHome, jmcRelativePath); File file2 = new File(javaHomeEnv, jmcRelativePath); if (!file1.exists() && !file2.exists()) { jmc.setDisabled(true); } file1 = new File(javaHome, jconsoleRelativePath); file2 = new File(javaHomeEnv, jconsoleRelativePath); if (!file1.exists() && !file2.exists()) { jconsole.setDisabled(true); } file1 = new File(javaHome, jvisualvmRelativePath); file2 = new File(javaHomeEnv, jvisualvmRelativePath); if (!file1.exists() && !file2.exists()) { jvisualvm.setDisabled(true); } } + + private Action createToolAction(String name, String relativePath, String iconPath) { + Action action = new Action(name, e -> { + File file = new File(javaHome, relativePath); + if (file.exists()) { + runProgram(file.getAbsolutePath()); + } else { + file = new File(javaHomeEnv, relativePath); + if (!file.exists()) { + StaticLog.warn("Not found " + name + "."); + return; + } + runProgram(file.getAbsolutePath()); + } + }); + action.setGraphic(LayoutHelper.iconView(FxJDKToolActionGroup.class.getResource(iconPath))); + return action; + }另外，验证工具可用性的重复代码也可以提取到单独的辅助方法中：
private boolean isToolAvailable(String relativePath) { File file1 = new File(javaHome, relativePath); File file2 = new File(javaHomeEnv, relativePath); return file1.exists() || file2.exists(); }🧰 Tools
🪛 GitHub Check: CodeFactor
[notice] 55-124: core/src/main/java/com/tlcsdm/core/javafx/controlsfx/FxJDKToolActionGroup.java#L55-L124
Complex Method
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
core/src/main/resources/com/tlcsdm/core/static/icon/jmc.pngis excluded by!**/*.pngcore/src/main/resources/com/tlcsdm/core/static/icon/jmc@2x.pngis excluded by!**/*.pngcore/src/main/resources/com/tlcsdm/core/static/icon/jvisualvm.pngis excluded by!**/*.pngcore/src/main/resources/com/tlcsdm/core/static/icon/jvisualvm@2x.pngis excluded by!**/*.png
📒 Files selected for processing (7)
core/src/main/java/com/tlcsdm/core/javafx/controlsfx/FxActionGroup.java(1 hunks)core/src/main/java/com/tlcsdm/core/javafx/controlsfx/FxJDKToolActionGroup.java(1 hunks)core/src/main/resources/com/tlcsdm/core/i18n/messages_en.properties(1 hunks)core/src/main/resources/com/tlcsdm/core/i18n/messages_ja.properties(1 hunks)core/src/main/resources/com/tlcsdm/core/i18n/messages_zh.properties(1 hunks)qe/src/main/java/com/tlcsdm/qe/provider/QeMenubarConfigrationProvider.java(3 hunks)smc/src/main/java/com/tlcsdm/smc/provider/SmcMenubarConfigrationProvider.java(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
qe/src/main/java/com/tlcsdm/qe/provider/QeMenubarConfigrationProvider.java (2)
core/src/main/java/com/tlcsdm/core/javafx/controlsfx/FxJDKToolActionGroup.java (1)
FxJDKToolActionGroup(45-137)core/src/main/java/com/tlcsdm/core/javafx/controlsfx/FxActionGroup.java (1)
FxActionGroup(43-119)
🪛 GitHub Check: CodeFactor
core/src/main/java/com/tlcsdm/core/javafx/controlsfx/FxJDKToolActionGroup.java
[notice] 55-124: core/src/main/java/com/tlcsdm/core/javafx/controlsfx/FxJDKToolActionGroup.java#L55-L124
Complex Method
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: build (21, windows-latest, false)
- GitHub Check: build (21, ubuntu-latest, false)
- GitHub Check: build (21, macos-latest, false)
- GitHub Check: build (17, macos-latest, false)
- GitHub Check: build (17, windows-latest, false)
- GitHub Check: build (17, ubuntu-latest, false)
- GitHub Check: qodana
- GitHub Check: StepSecurity Harden-Runner
- GitHub Check: Summary
🔇 Additional comments (12)
core/src/main/resources/com/tlcsdm/core/i18n/messages_zh.properties (1)
68-68: 很好的本地化实现！该变更正确地为新的"JDK 工具"菜单项添加了中文本地化支持，格式与现有条目保持一致。
core/src/main/resources/com/tlcsdm/core/i18n/messages_en.properties (1)
68-68: 英文本地化实现完美！该变更为"JDK Tool"菜单项添加了英文本地化支持，且格式规范，与整体风格保持一致。
core/src/main/resources/com/tlcsdm/core/i18n/messages_ja.properties (1)
68-68: 日文本地化实现完整！该变更为"JDKツール"菜单项添加了日文本地化支持，遵循了正确的格式规范，与其他本地化条目风格一致。
core/src/main/java/com/tlcsdm/core/javafx/controlsfx/FxActionGroup.java (1)
89-95: 新方法实现规范！
jdkTool方法实现符合类中已有的编码模式，正确地使用了I18n获取本地化文本和适当的图标资源。JavaDoc注释风格与现有代码保持一致，整体集成良好。smc/src/main/java/com/tlcsdm/smc/provider/SmcMenubarConfigrationProvider.java (3)
34-34: 已正确导入JDK工具组相关类导入
FxJDKToolActionGroup类以支持新增的JDK工具功能组，导入声明位置合理。
182-183: 成功初始化JDK工具组通过
FxJDKToolActionGroup().create()创建的JDK工具组实例被正确声明为私有的final字段，遵循了类中其他ActionGroup的声明风格。
196-196: JDK工具组已正确集成到工具菜单JDK工具组作为第一个元素添加到工具菜单中，并通过ACTION_SEPARATOR与其他工具选项分隔，保持了良好的菜单结构。
qe/src/main/java/com/tlcsdm/qe/provider/QeMenubarConfigrationProvider.java (3)
36-36: 已正确导入JDK工具组相关类导入
FxJDKToolActionGroup类以支持新增的JDK工具功能组，导入声明位置合理。
185-186: 成功初始化JDK工具组通过
FxJDKToolActionGroup().create()创建的JDK工具组实例被正确声明为私有的final字段，遵循了类中其他ActionGroup的声明风格。
197-197: JDK工具组已正确集成到工具菜单JDK工具组作为第一个元素添加到工具菜单中，并通过ACTION_SEPARATOR与其他工具选项分隔，保持了良好的菜单结构。与SmcMenubarConfigrationProvider中的实现保持一致，确保了应用不同模块间的UI一致性。
core/src/main/java/com/tlcsdm/core/javafx/controlsfx/FxJDKToolActionGroup.java (2)
1-44: JDK工具组类的版权和基本声明符合项目标准类声明、包结构、导入和JavaDoc注释都符合项目规范。
134-136: 创建方法实现正确create方法通过FxActionGroup.jdkTool静态方法正确创建了包含所有JDK工具的ActionGroup，实现简洁明了。
core/src/main/java/com/tlcsdm/core/javafx/controlsfx/FxJDKToolActionGroup.java
Outdated
Show resolved
Hide resolved
Qodana Community for JVMIt seems all right 👌 No new problems were found according to the checks applied 💡 Qodana analysis was run in the pull request mode: only the changed files were checked Contact Qodana teamContact us at qodana-support@jetbrains.com
|
Close #2078 Signed-off-by: unknowIfGuestInDream <liang.tang.cx@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
core/src/main/java/com/tlcsdm/core/javafx/controlsfx/FxJDKToolActionGroup.java (1)
152-159: 🛠️ Refactor suggestion增强进程启动的跨平台支持和错误处理
当前的
runProgram方法仅支持 Windows 系统（使用 "cmd", "/c", "start" 命令），但构造函数中已进行了操作系统判断以设置不同系统的可执行文件扩展名。建议改进 runProgram 方法，使其支持不同操作系统并增强错误处理：
private void runProgram(String filePath) { - ProcessBuilder builder = new ProcessBuilder("cmd", "/c", "start", filePath); + ProcessBuilder builder; + if (OSUtil.getOS().equals(OSUtil.OS.WINDOWS)) { + builder = new ProcessBuilder("cmd", "/c", "start", filePath); + } else { + builder = new ProcessBuilder(filePath); + } + // 设置工作目录，确保程序在正确的上下文中运行 + builder.directory(new File(System.getProperty("user.dir"))); try { builder.start(); } catch (IOException ex) { - new ExceptionDialog(ex).show(); + StaticLog.error("启动程序失败: " + filePath, ex); + new ExceptionDialog(ex, "启动JDK工具失败", "无法启动工具: " + new File(filePath).getName()).show(); } }
🧹 Nitpick comments (2)
core/src/main/java/com/tlcsdm/core/javafx/controlsfx/FxJDKToolActionGroup.java (2)
45-55: 建议将 Action 字段修改为私有访问控制当前 Action 字段（jmc, jconsole, jvisualvm, jshell）是包访问权限的。为遵循封装原则，建议将这些字段设置为私有（private）以防止外部直接访问和修改。
- Action jmc; - Action jconsole; - Action jvisualvm; - Action jshell; + private Action jmc; + private Action jconsole; + private Action jvisualvm; + private Action jshell;
1-164: 建议添加单元测试确保可靠性新增的
FxJDKToolActionGroup类缺少单元测试。为保证功能的可靠性，建议添加测试用例，覆盖以下场景：
- 检测不同操作系统下工具路径的正确生成
- 检测工具可用性判断的正确性
- 模拟工具启动过程并验证结果
您需要我为这个类提供单元测试示例吗？
🧰 Tools
🪛 GitHub Check: CodeFactor
[notice] 57-151: core/src/main/java/com/tlcsdm/core/javafx/controlsfx/FxJDKToolActionGroup.java#L57-L151
Complex Method
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
core/src/main/resources/com/tlcsdm/core/static/icon/jshell.pngis excluded by!**/*.pngcore/src/main/resources/com/tlcsdm/core/static/icon/jshell@2x.pngis excluded by!**/*.png
📒 Files selected for processing (1)
core/src/main/java/com/tlcsdm/core/javafx/controlsfx/FxJDKToolActionGroup.java(1 hunks)
🧰 Additional context used
🪛 GitHub Check: CodeFactor
core/src/main/java/com/tlcsdm/core/javafx/controlsfx/FxJDKToolActionGroup.java
[notice] 57-151: core/src/main/java/com/tlcsdm/core/javafx/controlsfx/FxJDKToolActionGroup.java#L57-L151
Complex Method
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: build (21, ubuntu-latest, false)
- GitHub Check: build (17, macos-latest, false)
- GitHub Check: build (21, macos-latest, false)
- GitHub Check: build (21, windows-latest, false)
- GitHub Check: build (17, windows-latest, false)
- GitHub Check: build (17, ubuntu-latest, false)
- GitHub Check: qodana
- GitHub Check: Summary
core/src/main/java/com/tlcsdm/core/javafx/controlsfx/FxJDKToolActionGroup.java
Outdated
Show resolved
Hide resolved
Close #2078 Signed-off-by: unknowIfGuestInDream <liang.tang.cx@gmail.com>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Close #2078
Fixes #
Proposed Changes
Readiness Checklist
Author/Contributor
Reviewing Maintainer
enhancement,bug,documentationordependenciesSummary by Sourcery
Add JDK Tool support to the application's menubar, introducing a new action group for JDK-related tools
New Features:
Enhancements:
Chores:
Summary by CodeRabbit