简化列表式管理页面相关代码 [Gemini 3.1 Pro] by CiiLu · Pull Request #6470 · HMCL-dev/HMCL

CiiLu · 2026-07-21T15:55:39Z

迁移重复逻辑到 ToolbarListPageSkin
同时为世界管理、原理图管理添加搜索功能

gemini-code-assist

Code Review

This pull request refactors several list page skins (such as Java, Theme Pack, Data Pack, Game, Installer, Mod, Resource Pack, Schematics, World Backups, and World list pages) to inherit from a newly introduced common base class ToolbarListPageSkin. This centralizes toolbar, search, and selection logic, reducing code duplication. Feedback on the changes highlights a logic bug in createSelectAllButton where the button is incorrectly enabled when the list is empty, potential NullPointerException risks in the search predicates of DataPackListPageSkin and WorldListPage due to missing null checks, and performance inefficiencies in WorldListPage, SchematicsPage, and GameListPage where toLowerCase is repeatedly called inside lambda filters.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

gemini-code-assist · 2026-07-21T15:58:12Z

+    protected JFXButton createSelectAllButton() {
+        JFXButton selectAll = createToolbarButton2(i18n("button.select_all"), SVG.SELECT_ALL, () -> listView.getSelectionModel().selectRange(0, listView.getItems().size()));
+        ListChangeListener<Object> listener = change -> {
+            selectAll.setDisable(!listView.getItems().isEmpty()
+                    && listView.getSelectionModel().getSelectedItems().size() == listView.getItems().size());
+        };
+        listView.getSelectionModel().getSelectedItems().addListener(listener);
+        listView.getItems().addListener(listener);
+        return selectAll;
+    }


在 createSelectAllButton 中，存在两个可以改进的地方：\n1. 逻辑漏洞：当列表为空时，!listView.getItems().isEmpty() 为 false，导致整个表达式计算为 false，从而使 selectAll 按钮被启用（setDisable(false)）。实际上，当列表为空时，应该禁用“全选”按钮。建议将逻辑改为 listView.getItems().isEmpty() || ...。\n2. 初始状态未更新：在创建按钮并添加监听器后，没有立即触发一次状态更新。如果列表初始为空或已全选，按钮的禁用状态不会正确反映，直到下一次选择或列表内容发生变化。\n\n建议重构该方法，提取更新逻辑并进行初始调用。

protected JFXButton createSelectAllButton() {\n JFXButton selectAll = createToolbarButton2(i18n("button.select_all"), SVG.SELECT_ALL, () -> listView.getSelectionModel().selectRange(0, listView.getItems().size()));\n Runnable updateDisableState = () -> {\n selectAll.setDisable(listView.getItems().isEmpty()\n || listView.getSelectionModel().getSelectedItems().size() == listView.getItems().size());\n };\n ListChangeListener<Object> listener = change -> updateDisableState.run();\n listView.getSelectionModel().getSelectedItems().addListener(listener);\n listView.getItems().addListener(listener);\n updateDisableState.run();\n return selectAll;\n }

gemini-code-assist · 2026-07-21T15:58:12Z

+        final Predicate<String> stringPredicate;
+        if (queryString.startsWith("regex:")) {
+            try {
+                Pattern pattern = Pattern.compile(StringUtils.substringAfter(queryString, "regex:"));
+                stringPredicate = s -> pattern.matcher(s).find();
+            } catch (Exception e) {
+                return dataPack -> false;
            }
+        } else {
+            String lowerCaseFilter = queryString.toLowerCase(Locale.ROOT);
+            stringPredicate = s -> s.toLowerCase(Locale.ROOT).contains(lowerCaseFilter);
        }


在重构 updateSearchPredicate 时，删除了原先对 s != null 的防御性检查。如果 id 或 description 为 null，在执行 stringPredicate.test(...) 时会抛出 NullPointerException。为了保证代码的健壮性，建议恢复 s != null 的空值检查。

final Predicate<String> stringPredicate;\n if (queryString.startsWith("regex:")) {\n try {\n Pattern pattern = Pattern.compile(StringUtils.substringAfter(queryString, "regex:"));\n stringPredicate = s -> s != null && pattern.matcher(s).find();\n } catch (Exception e) {\n return dataPack -> false;\n }\n } else {\n String lowerCaseFilter = queryString.toLowerCase(Locale.ROOT);\n stringPredicate = s -> s != null && s.toLowerCase(Locale.ROOT).contains(lowerCaseFilter);\n }

gemini-code-assist · 2026-07-21T15:58:12Z

        @Override
-        protected List<Node> initializeToolbar(WorldListPage skinnable) {
-            JFXCheckBox chkShowAll = new JFXCheckBox(i18n("world.show_all"));
-            chkShowAll.selectedProperty().bindBidirectional(skinnable.showAllProperty());
-
-            return Arrays.asList(
-                    chkShowAll,
-                    createToolbarButton2(i18n("button.refresh"), SVG.REFRESH, skinnable::refresh),
-                    createToolbarButton2(i18n("world.add"), SVG.ADD, skinnable::add),
-                    createToolbarButton2(i18n("world.download"), SVG.DOWNLOAD, skinnable::download)
-            );
+        protected Predicate<World> updateSearchPredicate(String searchText) {
+            if (searchText == null || searchText.isEmpty()) return item -> true;
+            if (searchText.startsWith("regex:")) {
+                String regex = searchText.substring("regex:".length());
+                try {
+                    Pattern pattern = Pattern.compile(regex, Pattern.CASE_INSENSITIVE);
+                    return item -> pattern.matcher(item.getWorldName()).find();
+                } catch (PatternSyntaxException e) {
+                    return item -> false;
+                }
+            } else {
+                return item -> item.getWorldName().toLowerCase(Locale.ROOT).contains(searchText.toLowerCase(Locale.ROOT));
+            }
        }


在 updateSearchPredicate 中存在两个问题：\n1. 空指针异常风险：item.getWorldName() 可能返回 null（如第 355 行代码所示，存在对 world.getWorldName() != null 的显式判断）。如果为 null，调用 item.getWorldName().toLowerCase(...) 或 pattern.matcher(...) 将抛出 NullPointerException。\n2. 性能开销：在非正则搜索分支中，searchText.toLowerCase(Locale.ROOT) 在过滤每个元素时都会被重复调用。应当将其提取到 Lambda 表达式外部，只计算一次。

@Override\n protected Predicate<World> updateSearchPredicate(String searchText) {\n if (searchText == null || searchText.isEmpty()) return item -> true;\n if (searchText.startsWith("regex:")) {\n String regex = searchText.substring("regex:".length());\n try {\n Pattern pattern = Pattern.compile(regex, Pattern.CASE_INSENSITIVE);\n return item -> {\n String name = item.getWorldName();\n return name != null && pattern.matcher(name).find();\n };\n } catch (PatternSyntaxException e) {\n return item -> false;\n }\n } else {\n String lowerCaseFilter = searchText.toLowerCase(Locale.ROOT);\n return item -> {\n String name = item.getWorldName();\n return name != null && name.toLowerCase(Locale.ROOT).contains(lowerCaseFilter);\n };\n }\n }

gemini-code-assist · 2026-07-21T15:58:12Z

        @Override
-        protected ListCell<Item> createListCell(JFXListView<Item> listView) {
-            return new Cell();
+        protected Predicate<SchematicsPage.Item> updateSearchPredicate(String searchText) {
+            if (searchText == null || searchText.isEmpty()) return item -> true;
+            if (searchText.startsWith("regex:")) {
+                String regex = searchText.substring("regex:".length());
+                try {
+                    Pattern pattern = Pattern.compile(regex, Pattern.CASE_INSENSITIVE);
+                    return item -> pattern.matcher(item.getName() + item.getDescription()).find();
+                } catch (PatternSyntaxException e) {
+                    return item -> false;
+                }
+            } else {
+                return item -> (item.getName() + item.getDescription()).toLowerCase(Locale.ROOT).contains(searchText.toLowerCase(Locale.ROOT));
+            }
        }


在非正则搜索分支中，searchText.toLowerCase(Locale.ROOT) 在过滤每个元素时都会被重复调用。建议将其提取到 Lambda 表达式外部计算，以提升过滤大列表时的性能。

@Override\n protected Predicate<SchematicsPage.Item> updateSearchPredicate(String searchText) {\n if (searchText == null || searchText.isEmpty()) return item -> true;\n if (searchText.startsWith("regex:")) {\n String regex = searchText.substring("regex:".length());\n try {\n Pattern pattern = Pattern.compile(regex, Pattern.CASE_INSENSITIVE);\n return item -> pattern.matcher(item.getName() + item.getDescription()).find();\n } catch (PatternSyntaxException e) {\n return item -> false;\n }\n } else {\n String lowerCaseFilter = searchText.toLowerCase(Locale.ROOT);\n return item -> (item.getName() + item.getDescription()).toLowerCase(Locale.ROOT).contains(lowerCaseFilter);\n }\n }

gemini-code-assist · 2026-07-21T15:58:12Z

+            @Override
+            protected Predicate<GameListItem> updateSearchPredicate(String searchText) {
+                if (searchText == null || searchText.isEmpty()) return item -> true;
+                if (searchText.startsWith("regex:")) {
+                    String regex = searchText.substring("regex:".length());
+                    try {
+                        Pattern pattern = Pattern.compile(regex, Pattern.CASE_INSENSITIVE);
+                        return item -> pattern.matcher(item.id).find();
+                    } catch (PatternSyntaxException e) {
+                        return item -> false;
                    }
+                } else {
+                    return item -> item.id.toLowerCase(Locale.ROOT).contains(searchText.toLowerCase(Locale.ROOT));
                }
            }


在非正则搜索分支中，searchText.toLowerCase(Locale.ROOT) 在过滤每个元素时都会被重复调用。建议将其提取到 Lambda 表达式外部计算，以提升过滤大列表时的性能。

@Override\n protected Predicate<GameListItem> updateSearchPredicate(String searchText) {\n if (searchText == null || searchText.isEmpty()) return item -> true;\n if (searchText.startsWith("regex:")) {\n String regex = searchText.substring("regex:".length());\n try {\n Pattern pattern = Pattern.compile(regex, Pattern.CASE_INSENSITIVE);\n return item -> pattern.matcher(item.id).find();\n } catch (PatternSyntaxException e) {\n return item -> false;\n }\n } else {\n String lowerCaseFilter = searchText.toLowerCase(Locale.ROOT);\n return item -> item.id.toLowerCase(Locale.ROOT).contains(lowerCaseFilter);\n }\n }

(22) 楼船侧峰如趋亭如冲

cd609e7

github-actions Bot added the 1000+ label Jul 21, 2026

gemini-code-assist Bot reviewed Jul 21, 2026

View reviewed changes

(23) 直驶向青史之下哪一宗

45c9241

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

简化列表式管理页面相关代码 [Gemini 3.1 Pro]#6470

简化列表式管理页面相关代码 [Gemini 3.1 Pro]#6470
CiiLu wants to merge 2 commits into
HMCL-dev:mainfrom
CiiLu:abc

CiiLu commented Jul 21, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Jul 21, 2026

Uh oh!

gemini-code-assist Bot Jul 21, 2026

Uh oh!

gemini-code-assist Bot Jul 21, 2026

Uh oh!

gemini-code-assist Bot Jul 21, 2026

Uh oh!

gemini-code-assist Bot Jul 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

CiiLu commented Jul 21, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant