fix: add plugin sorting by name and improve search filtering logic#5559
fix: add plugin sorting by name and improve search filtering logic#5559Soulter merged 1 commit intoAstrBotDevs:masterfrom
Conversation
dosubot
bot
added
the
size:M
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求旨在通过引入插件名称排序功能来解决已安装插件界面中插件顺序不一致的问题。同时，它还改进了插件搜索过滤逻辑，确保无论是否进行搜索，插件列表都能保持按名称排序，从而提升用户体验和界面稳定性。 Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Related Documentation Checked 1 published document(s) in 1 knowledge base(s). No updates required. How did I do? Any feedback? |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我发现了 1 个问题，并给出了一些整体反馈：
- 在
filteredPlugins中，你已经在sortPluginsByName里创建了一个不变更原数组的排序副本，因此这里的展开操作sortPluginsByName([...filtered])是多余的；你可以直接传入filtered，以避免额外的数组分配。 - 由于
sortPluginsByName是一个不依赖组件状态的纯工具函数，建议将它移动到useExtensionPage之外（例如移动到一个共享的工具文件，或者模块作用域），这样可以避免在每次 hook 调用时重新定义该函数，并且在其他插件列表也需要同样的排序逻辑时能复用它。
给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- In `filteredPlugins`, you’re already creating a non-mutating sorted copy in `sortPluginsByName`, so the spread `sortPluginsByName([...filtered])` is redundant; you can pass `filtered` directly to avoid the extra array allocation.
- Since `sortPluginsByName` is a pure utility that doesn’t depend on component state, consider moving it outside `useExtensionPage` (e.g., to a shared utility file or at module scope) to avoid redefining the function on each hook invocation and to make it reusable if other plugin lists need the same ordering.
## Individual Comments
### Comment 1
<location path="dashboard/src/views/extension/useExtensionPage.js" line_range="330-339" />
<code_context>
return data;
});
+ const sortPluginsByName = (plugins) => {
+ return plugins
+ .map((plugin, index) => ({ plugin, index }))
+ .sort((a, b) => {
+ const nameA = String(a.plugin?.name ?? "");
+ const nameB = String(b.plugin?.name ?? "");
+ const nameCompare = nameA.localeCompare(nameB, undefined, {
+ sensitivity: "base",
+ });
+ if (nameCompare !== 0) {
+ return nameCompare;
+ }
+ return a.index - b.index;
+ })
+ .map((item) => item.plugin);
+ };
+
</code_context>
<issue_to_address>
**suggestion (performance):** Consider simplifying sorting logic by relying on stable `Array.prototype.sort` instead of mapping to index-tagged objects.
Modern JS engines (evergreen browsers and Node 12+) guarantee a stable `Array.prototype.sort`, so you can rely on that stability instead of carrying the original index through the sort. That lets you drop the pre/post `map` calls and index tracking, keeping just the `localeCompare`-based comparator and operating directly on the `plugins` array (or a shallow copy if you want to avoid mutation).
Suggested implementation:
```javascript
const sortPluginsByName = (plugins) => {
return [...plugins].sort((a, b) => {
const nameA = String(a?.name ?? "");
const nameB = String(b?.name ?? "");
return nameA.localeCompare(nameB, undefined, {
sensitivity: "base",
});
});
};
```
If callers of `sortPluginsByName` previously depended on it preserving the original array reference (i.e., mutating `plugins` in place), you can drop the spread and simply call `plugins.sort(...)` instead. In that case, update the body to `plugins.sort(...)` and optionally return `plugins` for chaining.
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进评审质量。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- In
filteredPlugins, you’re already creating a non-mutating sorted copy insortPluginsByName, so the spreadsortPluginsByName([...filtered])is redundant; you can passfiltereddirectly to avoid the extra array allocation. - Since
sortPluginsByNameis a pure utility that doesn’t depend on component state, consider moving it outsideuseExtensionPage(e.g., to a shared utility file or at module scope) to avoid redefining the function on each hook invocation and to make it reusable if other plugin lists need the same ordering.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `filteredPlugins`, you’re already creating a non-mutating sorted copy in `sortPluginsByName`, so the spread `sortPluginsByName([...filtered])` is redundant; you can pass `filtered` directly to avoid the extra array allocation.
- Since `sortPluginsByName` is a pure utility that doesn’t depend on component state, consider moving it outside `useExtensionPage` (e.g., to a shared utility file or at module scope) to avoid redefining the function on each hook invocation and to make it reusable if other plugin lists need the same ordering.
## Individual Comments
### Comment 1
<location path="dashboard/src/views/extension/useExtensionPage.js" line_range="330-339" />
<code_context>
return data;
});
+ const sortPluginsByName = (plugins) => {
+ return plugins
+ .map((plugin, index) => ({ plugin, index }))
+ .sort((a, b) => {
+ const nameA = String(a.plugin?.name ?? "");
+ const nameB = String(b.plugin?.name ?? "");
+ const nameCompare = nameA.localeCompare(nameB, undefined, {
+ sensitivity: "base",
+ });
+ if (nameCompare !== 0) {
+ return nameCompare;
+ }
+ return a.index - b.index;
+ })
+ .map((item) => item.plugin);
+ };
+
</code_context>
<issue_to_address>
**suggestion (performance):** Consider simplifying sorting logic by relying on stable `Array.prototype.sort` instead of mapping to index-tagged objects.
Modern JS engines (evergreen browsers and Node 12+) guarantee a stable `Array.prototype.sort`, so you can rely on that stability instead of carrying the original index through the sort. That lets you drop the pre/post `map` calls and index tracking, keeping just the `localeCompare`-based comparator and operating directly on the `plugins` array (or a shallow copy if you want to avoid mutation).
Suggested implementation:
```javascript
const sortPluginsByName = (plugins) => {
return [...plugins].sort((a, b) => {
const nameA = String(a?.name ?? "");
const nameB = String(b?.name ?? "");
return nameA.localeCompare(nameB, undefined, {
sensitivity: "base",
});
});
};
```
If callers of `sortPluginsByName` previously depended on it preserving the original array reference (i.e., mutating `plugins` in place), you can drop the spread and simply call `plugins.sort(...)` instead. In that case, update the body to `plugins.sort(...)` and optionally return `plugins` for chaining.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| const sortPluginsByName = (plugins) => { | ||
| return plugins | ||
| .map((plugin, index) => ({ plugin, index })) | ||
| .sort((a, b) => { | ||
| const nameA = String(a.plugin?.name ?? ""); | ||
| const nameB = String(b.plugin?.name ?? ""); | ||
| const nameCompare = nameA.localeCompare(nameB, undefined, { | ||
| sensitivity: "base", | ||
| }); | ||
| if (nameCompare !== 0) { |
There was a problem hiding this comment.
suggestion (performance): 考虑利用稳定的 Array.prototype.sort 来简化排序逻辑，而不是先映射成带索引的对象再排序。
现代 JS 引擎（主流浏览器和 Node 12+）都保证了 Array.prototype.sort 的稳定性，因此你可以依赖这种稳定性，而无需在排序过程中额外携带原始索引。这样就可以去掉排序前后的 map 调用和索引追踪，只保留基于 localeCompare 的比较器，并直接在 plugins 数组上操作（如果需要避免原地修改，可以先做一个浅拷贝）。
推荐实现：
const sortPluginsByName = (plugins) => {
return [...plugins].sort((a, b) => {
const nameA = String(a?.name ?? "");
const nameB = String(b?.name ?? "");
return nameA.localeCompare(nameB, undefined, {
sensitivity: "base",
});
});
};如果 sortPluginsByName 的调用方之前依赖它保留原数组引用（即就地修改 plugins），那你可以去掉展开操作，直接调用 plugins.sort(...)。这种情况下，将函数体更新为 plugins.sort(...)，并可选择返回 plugins 以便链式调用。
Original comment in English
suggestion (performance): Consider simplifying sorting logic by relying on stable Array.prototype.sort instead of mapping to index-tagged objects.
Modern JS engines (evergreen browsers and Node 12+) guarantee a stable Array.prototype.sort, so you can rely on that stability instead of carrying the original index through the sort. That lets you drop the pre/post map calls and index tracking, keeping just the localeCompare-based comparator and operating directly on the plugins array (or a shallow copy if you want to avoid mutation).
Suggested implementation:
const sortPluginsByName = (plugins) => {
return [...plugins].sort((a, b) => {
const nameA = String(a?.name ?? "");
const nameB = String(b?.name ?? "");
return nameA.localeCompare(nameB, undefined, {
sensitivity: "base",
});
});
};If callers of sortPluginsByName previously depended on it preserving the original array reference (i.e., mutating plugins in place), you can drop the spread and simply call plugins.sort(...) instead. In that case, update the body to plugins.sort(...) and optionally return plugins for chaining.
dosubot
bot
added
the
feature:plugin
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
| filtered = plugins.filter((plugin) => { | ||
| const pluginName = (plugin.name ?? "").toLowerCase(); | ||
| const pluginDesc = (plugin.desc ?? "").toLowerCase(); | ||
| const pluginAuthor = (plugin.author ?? "").toLowerCase(); | ||
| const supportPlatforms = Array.isArray(plugin.support_platforms) | ||
| ? plugin.support_platforms.join(" ").toLowerCase() | ||
| : ""; | ||
| const astrbotVersion = (plugin.astrbot_version ?? "").toLowerCase(); | ||
|
|
||
| return ( | ||
| pluginName.includes(search) || | ||
| pluginDesc.includes(search) || | ||
| pluginAuthor.includes(search) || | ||
| supportPlatforms.includes(search) || | ||
| astrbotVersion.includes(search) | ||
| ); | ||
| }); |
There was a problem hiding this comment.
为了提高代码的可读性和可维护性，可以考虑将过滤逻辑重构一下。通过将所有需要搜索的字段放入一个数组中，然后使用 some 方法来检查是否至少有一个字段匹配搜索条件，可以避免重复的 toLowerCase().includes() 调用，也让未来增删搜索字段变得更加容易。
filtered = plugins.filter((plugin) => {
const fieldsToSearch = [
plugin.name,
plugin.desc,
plugin.author,
Array.isArray(plugin.support_platforms)
? plugin.support_platforms.join(" ")
: "",
plugin.astrbot_version,
];
return fieldsToSearch.some((field) =>
(field ?? "").toLowerCase().includes(search)
);
});
|
Generated docs update PR (pending manual review): AI change summary:
Experimental bot notice:
|
2 participants
fixes: #2762
解决：在已安装插件的界面，对插件进行保存配置、重载等操作后被置于页面末尾的问题，在issue #2762有提及
Modifications / 改动点
仅修改dashboard/src/views/extension/useExtensionPage.js，未影响后端
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.Summary by Sourcery
在扩展控制台页面中，对已安装插件列表按名称排序，并增强插件搜索过滤功能。
New Features:
Enhancements:
Original summary in English
Summary by Sourcery
Sort the installed plugin list by name and enhance the plugin search filtering on the extensions dashboard page.
New Features:
Enhancements: