feat: 支持非阻塞运行脚本 - #71
Conversation
Walkthrough新增 Changes阻塞运行流程
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ScriptSettingRootInterface
participant ChainRunner
participant launch_in_terminal
participant ScriptProcess
ScriptSettingRootInterface->>ChainRunner: 提交启用脚本规格
ChainRunner->>launch_in_terminal: 传递 block 并启动脚本
launch_in_terminal->>ScriptProcess: 创建终端子进程
ScriptProcess-->>ChainRunner: 返回执行状态
ChainRunner-->>ScriptSettingRootInterface: 回传成功数量和失败列表
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/script_chainer/gui/page/script_setting_cards.py (1)
161-168: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win不要在 GUI 线程中等待阻塞调试进程。
block=True会同步等待脚本退出,导致整个界面冻结;且脚本以非零状态退出后仍会显示“启动成功”。请将启动/等待移入工作线程,并在完成回调中根据returncode显示完成或失败状态。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/script_chainer/gui/page/script_setting_cards.py` around lines 161 - 168, Update the debug-launch flow around launch_in_terminal so it never waits synchronously on the GUI thread when self.config.block is true. Move process startup and waiting into a worker thread, then use the completion callback to inspect the process returncode and show a success message only for a zero exit status, otherwise show a failure status; keep non-blocking launches responsive and avoid reporting success before completion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/script_chainer/gui/page/script_setting_interface.py`:
- Around line 235-238: Update the specs construction in the relevant script
settings method to use cfg.script_display_name instead of cfg.display_name,
ensuring failure messages always include the script filename or fallback display
name while preserving the existing enabled-script filtering.
---
Outside diff comments:
In `@src/script_chainer/gui/page/script_setting_cards.py`:
- Around line 161-168: Update the debug-launch flow around launch_in_terminal so
it never waits synchronously on the GUI thread when self.config.block is true.
Move process startup and waiting into a worker thread, then use the completion
callback to inspect the process returncode and show a success message only for a
zero exit status, otherwise show a failure status; keep non-blocking launches
responsive and avoid reporting success before completion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 614dad46-33df-4c7e-9ca0-680fecdf5931
📒 Files selected for processing (5)
src/script_chainer/config/script_config.pysrc/script_chainer/gui/page/script_edit_interface.pysrc/script_chainer/gui/page/script_setting_cards.pysrc/script_chainer/gui/page/script_setting_interface.pysrc/script_chainer/utils/process_utils.py
| specs = [ | ||
| (i, cfg.block, cfg.display_name) | ||
| for i, cfg in enumerate(self.chosen_config.script_list) | ||
| if cfg.enabled |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
失败提示应使用始终可用的脚本显示名。
display_name 默认为空,默认脚本启动失败时会得到空白条目。请改用 cfg.script_display_name,以便错误提示包含文件名或回退名称。
- (i, cfg.block, cfg.display_name)
+ (i, cfg.block, cfg.script_display_name)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| specs = [ | |
| (i, cfg.block, cfg.display_name) | |
| for i, cfg in enumerate(self.chosen_config.script_list) | |
| if cfg.enabled | |
| specs = [ | |
| (i, cfg.block, cfg.script_display_name) | |
| for i, cfg in enumerate(self.chosen_config.script_list) | |
| if cfg.enabled |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/script_chainer/gui/page/script_setting_interface.py` around lines 235 -
238, Update the specs construction in the relevant script settings method to use
cfg.script_display_name instead of cfg.display_name, ensuring failure messages
always include the script filename or fallback display name while preserving the
existing enabled-script filtering.
|
应该得写进launch_script |
支持非阻塞运行脚本,使得可以后台运行部分脚本
Summary by CodeRabbit
新功能
改进