Import stdexec and httplib and refactor example widgets#18
Import stdexec and httplib and refactor example widgets#18creeper5820 wants to merge 12 commits intomainfrom
Conversation
Walkthrough新增 AssetCenter 小部件(PIMPL)并实现基于 stdexec 的异步下载流水线;新增下载任务、代理解析、Qt 调度器、httplib 兼容层、工具与字符串常量;调整 snackbar/text API、示例构建依赖、CI 安装项及 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as 用户
participant UI as AssetCenter (UI 线程)
participant QtSched as QtScheduler
participant Worker as WorkerThread
participant HTTP as cpp-httplib
participant FS as 文件系统
User->>UI: 点击“下载”
UI->>QtSched: 启动 stdexec 异步工作流(schedule)
QtSched->>Worker: 派发下载任务到后台线程
Worker->>UI: 请求保存路径(通过 QtScheduler 回到 UI 线程)
UI-->>Worker: 返回保存路径
Worker->>HTTP: 创建 client 并调用 configure_client(超时、代理、重定向)
Worker->>HTTP: 发起 GET(注册进度回调)
loop 下载进行中
HTTP->>Worker: 进度回调
Worker->>QtSched: 转发进度更新
QtSched->>UI: 更新进度显示
end
alt 成功 (HTTP 200)
Worker->>FS: 写入并 flush 文件
Worker->>QtSched: 通知完成
QtSched->>UI: 更新状态“下载完成”
else 错误/超时
Worker->>QtSched: 传递异常/错误
QtSched->>UI: 更新状态“下载失败”
end
User->>UI: 点击“打开文件”
UI->>FS: 打开保存目录
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 分钟 需要额外关注的点:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-12-17T14:36:12.565ZApplied to files:
🧬 Code graph analysis (1)example/widgets/components/asset-center.cc (1)
🔇 Additional comments (1)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (9)
creeper-qt/widget/text.hh (1)
36-36: 考虑统一参数传递方式以提高一致性。
Color属性使用const auto& v传递QColor,而新增的属性(如TextFormat、OpenExternalLinks)使用auto v传递小类型。QColor是一个较小的类型(通常约 16 字节),按值传递在现代 C++ 中是可接受的,可以考虑统一为按值传递以提高代码一致性。不过,现有代码中
Alignment(第 41 行)和TextInteractionFlags(第 43 行)也使用了const auto&,因此这只是一个可选的风格统一建议。可选的修改:
-using Color = SetterProp<Token, QColor, [](auto& self, const auto& v) { self.set_color(v); }>; +using Color = SetterProp<Token, QColor, [](auto& self, auto v) { self.set_color(v); }>;creeper-qt/widget/snackbar.hh (1)
20-20: 空命名空间可能不必要。
creeper::snackbar::pro命名空间为空。如果不打算在近期使用,可以考虑移除以保持代码整洁。example/widgets.cmake (1)
5-10: 使用main分支存在构建不稳定性风险
stdexec使用GIT_TAG main会导致构建不可复现,且可能因上游 breaking changes 而意外失败。建议固定到特定的 commit hash 或 release tag,与
httplib的v0.28.0保持一致的版本管理策略。FetchContent_Declare( stdexec GIT_REPOSITORY https://github.com/NVIDIA/stdexec.git - GIT_TAG main + GIT_TAG nvhpc-25.01 # 或使用特定 commit hash )example/widgets/util/string.hh (1)
5-8: 域名不一致
kGithubBaseUrl使用www.github.com,而 HTML 中的链接使用github.com。建议统一使用https://github.com(不带 www),以保持一致性。-constexpr auto kGithubBaseUrl = "https://www.github.com"; +constexpr auto kGithubBaseUrl = "https://github.com";example/widgets/util/executor.hh (2)
14-14: 原始指针的生命周期管理
context_使用原始QObject*指针,如果QObject在操作完成前被销毁,可能导致悬空指针访问。对于示例代码可以接受,但生产环境建议使用
QPointer<QObject>来安全地检测对象是否已被销毁。Also applies to: 64-64, 80-80
1-1: AI 生成代码标注建议补充说明代码已经过人工审查和调整,或添加更详细的来源说明,以便后续维护者了解代码背景。
example/widgets/util/download.cc (1)
11-16: 可选的改进:考虑将超时时间设置为可配置的。目前连接超时和读取超时是硬编码的(5秒和2秒)。如果将来需要针对不同的网络环境调整这些值,可以考虑将它们作为 GetUrlTask 的成员变量,使其可配置。
example/widgets/util/proxy.cc (1)
12-12: 使用非标准的 GNU 扩展可能影响可移植性。
?:运算符是 GCC 的扩展,不是标准 C++。如果项目需要在其他编译器(如 MSVC、Clang 的严格模式)上编译,可能会遇到问题。应用此补丁使用标准 C++ 语法:
-auto proxy = std::getenv("http_proxy") ?: std::getenv("https_proxy"); +auto proxy = std::getenv("http_proxy"); +if (!proxy) proxy = std::getenv("https_proxy"); if (!proxy) return std::nullopt;example/widgets/components/asset-center.cc (1)
32-37: 建议验证 qApp 的有效性。
QMetaObject::invokeMethod(qApp, ...)假设qApp不为 nullptr。虽然在正常的 Qt 应用程序生命周期内这通常是安全的,但如果在QApplication创建之前或销毁之后调用此函数,会导致崩溃。考虑添加检查:
auto update_ui(double ratio, std::string _status) { + if (!qApp) return; QMetaObject::invokeMethod(qApp, [this, ratio, _status = std::move(_status)] { status = QString::fromStdString( std::format("<code>[Status] {:3.2f}% ({})</code>", ratio, _status)); }); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.clangd(1 hunks)creeper-qt/widget/snackbar.hh(1 hunks)creeper-qt/widget/text.hh(1 hunks)example/widgets.cmake(2 hunks)example/widgets/components/asset-center.cc(1 hunks)example/widgets/components/asset-center.hh(1 hunks)example/widgets/util/download.cc(1 hunks)example/widgets/util/download.hh(1 hunks)example/widgets/util/executor.hh(1 hunks)example/widgets/util/proxy.cc(1 hunks)example/widgets/util/proxy.hh(1 hunks)example/widgets/util/string.hh(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
example/widgets/util/download.cc (3)
example/widgets/util/download.hh (1)
client(26-26)example/widgets/util/proxy.cc (2)
read_proxy(24-29)read_proxy(24-24)example/widgets/util/proxy.hh (1)
read_proxy(15-15)
example/widgets/util/proxy.hh (1)
example/widgets/util/download.hh (1)
client(26-26)
example/widgets/components/asset-center.cc (2)
creeper-qt/widget/text.hh (2)
manager(17-20)manager(17-17)example/widgets/components/asset-center.hh (1)
AssetCenter(11-11)
example/widgets/util/proxy.cc (2)
example/widgets/util/download.hh (1)
client(26-26)example/widgets/util/proxy.hh (1)
parse_proxy_from_env(19-19)
🔇 Additional comments (15)
creeper-qt/widget/text.hh (4)
37-37: 参数传递方式的改进很好!将
bool类型从const auto& v改为auto v(按值传递)是合适的,因为对于基本类型,按值传递更高效且避免了不必要的间接引用。
38-39: 新增属性实现正确!
TextFormat属性的实现符合预期,枚举类型使用按值传递是合适的。
44-45: 新增属性实现正确!
OpenExternalLinks属性的实现正确,布尔类型使用按值传递与WordWrap保持一致。
47-47: 新增动作属性实现正确!
AdjustSize作为ActionProp的实现符合预期,正确地调用了底层的adjustSize()方法。creeper-qt/widget/snackbar.hh (4)
2-3: 包含文件正确!PIMPL 模式和基类继承所需的头文件包含正确。
12-14: 嵌套消息结构设计合理!将
Message结构嵌套在Snackbar类中提高了封装性,std::string text成员适合存储消息文本。
23-23: 类型别名设计良好!
SnackbarMessage别名为嵌套的Message类型提供了便捷的访问方式,改善了 API 的易用性。
16-16: 验证receive()方法的noexcept规范。
receive()方法被标记为noexcept,这表示一个强保证。由于实现代码不可见(PIMPL 模式),无法直接验证该方法是否真正遵守了noexcept契约,特别是在以下情况下:
- 对
Message中std::string text成员的操作- 任何可能失败的堆分配或回调操作
建议确认实现确实不会抛出异常,或在必要时移除
noexcept规范。.clangd (1)
5-5: AI 摘要与代码不一致摘要声称"添加了
-fconcepts-diagnostics-depth=10",但实际上该标志位于Remove部分,意味着 clangd 会从编译命令中移除此标志,而非添加。请确认这是预期行为。如果需要增强概念相关的诊断信息,应将此标志移至
Add部分。example/widgets/components/asset-center.hh (1)
7-12: PIMPL 重构结构清晰头文件简洁明了,正确使用了 PIMPL 模式封装实现细节,符合接口与实现分离的最佳实践。
example/widgets/util/proxy.hh (1)
1-21: 代码结构良好!头文件结构清晰,使用了前向声明来减少依赖,函数签名简洁明了,注释也很清楚。
example/widgets/util/proxy.cc (1)
24-29: read_proxy 实现正确。假设
parse_proxy_from_env的异常处理问题得到修复,此函数的逻辑是正确的。example/widgets/util/download.hh (1)
1-29: API 设计清晰合理!GetUrlTask 的接口设计清晰,使用 std::optional 来表示可选的 save_path,使用 std::function 来支持进度回调。注释也清楚地说明了功能和异常处理。
example/widgets/components/asset-center.cc (1)
94-147: UI 组件构建代码结构良好!
Component()方法使用声明式的方式构建了清晰的 UI 布局,代码可读性强。example/widgets/util/download.cc (1)
23-24: GetUrlTask 使用硬编码的常量 URL,当前不存在 SSRF 风险。GetUrlTask 结构体本身不进行 URL 验证,但在当前代码库中,base_url 和 path 均为硬编码常量(来自 string.hh 中的 kGithubBaseUrl 和 kFontUrlPath),仅用于访问 GitHub 的固定资源。如果后续需要支持动态 URL 输入,则应添加验证。
| if (result && result->status == 200) { | ||
| auto stream = std::ofstream { *save_path, std::ios::binary }; | ||
| stream.write(result->body.data(), result->body.size()); | ||
| stream.flush(); | ||
| std::println("Download successfully"); |
There was a problem hiding this comment.
必须检查文件写入操作是否成功。
当前代码没有检查 stream.write 和 stream.flush 是否成功。如果磁盘空间不足、权限不够或发生I/O错误,写入操作可能会静默失败,导致文件不完整但用户看到"Download successfully"的消息。
应用此补丁来添加错误检查:
if (result && result->status == 200) {
auto stream = std::ofstream { *save_path, std::ios::binary };
+ if (!stream) {
+ throw std::runtime_error { "无法打开文件进行写入" };
+ }
stream.write(result->body.data(), result->body.size());
- stream.flush();
+ if (!stream.flush()) {
+ throw std::runtime_error { "文件写入失败" };
+ }
std::println("Download successfully");📝 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.
| if (result && result->status == 200) { | |
| auto stream = std::ofstream { *save_path, std::ios::binary }; | |
| stream.write(result->body.data(), result->body.size()); | |
| stream.flush(); | |
| std::println("Download successfully"); | |
| if (result && result->status == 200) { | |
| auto stream = std::ofstream { *save_path, std::ios::binary }; | |
| if (!stream) { | |
| throw std::runtime_error { "无法打开文件进行写入" }; | |
| } | |
| stream.write(result->body.data(), result->body.size()); | |
| stream.flush(); | |
| if (stream.fail()) { | |
| throw std::runtime_error { "文件写入失败" }; | |
| } | |
| std::println("Download successfully"); |
| bool success = QMetaObject::invokeMethod( | ||
| self.context_, | ||
| [r = std::move(self.receiver_)]() mutable { | ||
| // 在目标线程执行 | ||
| if (stdexec::get_stop_token(stdexec::get_env(r)).stop_requested()) { | ||
| stdexec::set_stopped(std::move(r)); | ||
| } else { | ||
| stdexec::set_value(std::move(r)); | ||
| } | ||
| }, | ||
| Qt::QueuedConnection); | ||
|
|
||
| if (!success) { | ||
| // 如果投递失败(例如线程正在析构),抛出错误 | ||
| // 注意:此时 receiver 已经被 move 进 lambda,处理这种情况比较棘手。 | ||
| // 生产环境通常需要更复杂的生命周期管理 (如使用 shared_state)。 | ||
| qDebug() << "Failed to invoke method on Qt Object!"; | ||
| } |
There was a problem hiding this comment.
invokeMethod 失败时的错误处理存在问题
当 invokeMethod 失败时(第 45-50 行),receiver_ 已被 move 进 lambda 中,但代码仅记录日志而未发送任何完成信号。这会导致操作永远无法完成,调用方可能无限等待。
注释中已提到此问题,但仍需处理。一种方案是在 move 之前检查投递是否可行,或使用 shared_ptr 包装 receiver 以便在失败时仍可访问。
friend void tag_invoke(stdexec::start_t, QtOperationState& self) noexcept {
if (!self.context_) {
stdexec::set_error(std::move(self.receiver_),
std::make_exception_ptr(std::runtime_error("Context is null")));
return;
}
+ // 使用 shared_ptr 包装以处理投递失败的情况
+ auto shared_receiver = std::make_shared<std::optional<Receiver>>(std::move(self.receiver_));
+
bool success = QMetaObject::invokeMethod(
self.context_,
- [r = std::move(self.receiver_)]() mutable {
+ [shared_receiver]() mutable {
+ auto& r = *shared_receiver;
+ if (!r) return;
+ auto receiver = std::move(*r);
+ r.reset();
// 在目标线程执行
- if (stdexec::get_stop_token(stdexec::get_env(r)).stop_requested()) {
- stdexec::set_stopped(std::move(r));
+ if (stdexec::get_stop_token(stdexec::get_env(receiver)).stop_requested()) {
+ stdexec::set_stopped(std::move(receiver));
} else {
- stdexec::set_value(std::move(r));
+ stdexec::set_value(std::move(receiver));
}
},
Qt::QueuedConnection);
if (!success) {
- qDebug() << "Failed to invoke method on Qt Object!";
+ if (auto& r = *shared_receiver) {
+ stdexec::set_error(std::move(*r),
+ std::make_exception_ptr(std::runtime_error("Failed to invoke method")));
+ }
}
}Committable suggestion skipped: line range outside the PR's diff.
| if (auto colon = url.find(':'); colon != std::string_view::npos) { | ||
| return std::make_pair( | ||
| std::string { url.substr(0, colon) }, std::stoi(std::string { url.substr(colon + 1) })); | ||
| } |
There was a problem hiding this comment.
std::stoi 可能抛出未捕获的异常导致程序崩溃。
std::stoi 在解析失败时会抛出 std::invalid_argument,在数值超出范围时会抛出 std::out_of_range。如果环境变量中的端口号格式错误(如 "example.com:abc" 或 "example.com:999999"),程序将崩溃。
应用此补丁添加异常处理和端口范围验证:
if (auto colon = url.find(':'); colon != std::string_view::npos) {
+ try {
+ auto port = std::stoi(std::string { url.substr(colon + 1) });
+ if (port < 1 || port > 65535) {
+ return std::nullopt;
+ }
return std::make_pair(
- std::string { url.substr(0, colon) }, std::stoi(std::string { url.substr(colon + 1) }));
+ std::string { url.substr(0, colon) }, port);
+ } catch (const std::invalid_argument&) {
+ return std::nullopt;
+ } catch (const std::out_of_range&) {
+ return std::nullopt;
+ }
}🤖 Prompt for AI Agents
In example/widgets/util/proxy.cc around lines 17 to 20, the code uses std::stoi
on the substring after ':' which can throw std::invalid_argument or
std::out_of_range and crash; update the code to robustly parse and validate the
port: extract the port substring, verify it contains only digits, parse it with
a safe unsigned conversion inside a try-catch (or use std::stoul), then check
the numeric range (1..65535), and on any parse error or out-of-range value
return a sentinel (e.g., host with port -1) or handle the error path
consistently; ensure exceptions from parsing are caught and do not propagate.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
example/widgets/util/download.cc (1)
42-66: 先前审查中标记的问题仍未解决。此代码段存在先前审查中已标记的两个问题:
- 语言不一致(第 44 行和第 59 行):错误消息使用中文"操作取消",而成功消息使用英文"Download successfully"。
- 缺少文件写入错误检查(第 55-59 行):没有检查文件流是否成功打开、写入和刷新操作是否成功。如果磁盘空间不足或权限不够,写入可能会静默失败。
请参考之前的审查评论以获取详细的修复建议。
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
example/widgets.cmake(2 hunks)example/widgets/util/download.cc(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- example/widgets.cmake
🧰 Additional context used
🧬 Code graph analysis (1)
example/widgets/util/download.cc (3)
example/widgets/util/download.hh (1)
client(26-26)example/widgets/util/proxy.hh (1)
read_proxy(15-15)example/widgets/util/proxy.cc (2)
read_proxy(24-29)read_proxy(24-24)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
example/widgets/util/proxy.cc (1)
16-19: std::stoi 可能抛出未捕获的异常。此问题已在之前的审查中指出。
std::stoi在解析失败或数值超出范围时会抛出异常,导致程序崩溃。example/widgets/util/download.cc (3)
10-15: 读取超时设置过短。此问题已在之前的审查中指出。2 秒的读取超时对于下载任务来说过短,建议增加到 30 秒或更长。
17-20: 错误消息语言不一致。此问题已在之前的审查中指出。错误消息使用中文("操作取消"),成功消息使用英文("Download successfully")。
30-34: 缺少文件写入错误检查。此问题已在之前的审查中指出。应检查
std::ofstream打开是否成功以及write/flush操作是否完成。
🧹 Nitpick comments (1)
example/widgets/util/proxy.cc (1)
11-11: 使用了非标准的 GCC 扩展运算符。
?:(Elvis 运算符) 是 GCC/Clang 扩展,不是标准 C++。这会降低代码可移植性。应用此补丁使用标准写法:
- auto proxy = std::getenv("http_proxy") ?: std::getenv("https_proxy"); + auto* http_proxy = std::getenv("http_proxy"); + auto* proxy = http_proxy ? http_proxy : std::getenv("https_proxy");
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
example/widgets.cmake(2 hunks)example/widgets/util/download.cc(1 hunks)example/widgets/util/httplib.hh(1 hunks)example/widgets/util/proxy.cc(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- example/widgets.cmake
🧰 Additional context used
🧬 Code graph analysis (2)
example/widgets/util/proxy.cc (2)
example/widgets/util/download.hh (1)
client(26-26)example/widgets/util/proxy.hh (1)
parse_proxy_from_env(19-19)
example/widgets/util/download.cc (3)
example/widgets/util/download.hh (1)
client(26-26)example/widgets/util/proxy.cc (2)
read_proxy(23-28)read_proxy(23-23)example/widgets/util/proxy.hh (1)
read_proxy(15-15)
🔇 Additional comments (3)
example/widgets/util/httplib.hh (1)
24-27: LGTM!使用
#pragma GCC diagnostic抑制第三方头文件警告是合理的做法。example/widgets/util/proxy.cc (1)
23-28: LGTM!函数正确检查 optional 并配置代理,日志输出有助于调试。
example/widgets/util/download.cc (1)
35-40: LGTM!错误处理覆盖了连接错误和 HTTP 状态码错误,逻辑正确。
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
example/widgets/util/download.cc (3)
12-12: 读取超时设置过短。2秒的读取超时对于下载可能较大的资产文件过短,在网络较慢或文件较大时会导致正常下载失败。建议增加到至少30秒。
18-20: 错误消息语言不一致。此处使用中文错误消息("操作取消"),但第43行的成功消息使用英文("Download successfully")。建议统一使用一种语言以保持一致性。
39-43: 必须检查文件操作是否成功。当前代码未检查:
- 文件流是否成功打开(第40行)
write和flush操作是否成功(第41-42行)如果磁盘空间不足、权限不够或发生I/O错误,写入操作可能会静默失败,导致文件不完整但用户看到"Download successfully"的消息。
应用此差异添加错误检查:
if (result && result->status == 200) { auto stream = std::ofstream { *save_path, std::ios::binary }; + if (!stream) { + throw std::runtime_error { "无法打开文件进行写入" }; + } stream.write(result->body.data(), result->body.size()); - stream.flush(); + if (!stream.flush()) { + throw std::runtime_error { "文件写入失败" }; + } std::println("Download successfully");
🧹 Nitpick comments (1)
example/widgets/util/download.cc (1)
34-37: 考虑添加下载取消功能。进度回调 lambda 始终返回
true,无法根据progress_callback的返回值取消下载。如果需要支持用户取消下载,可以考虑让progress_callback返回布尔值并在此处转发。应用此差异以支持取消功能:
- auto result = client.Get(path, [this](std::size_t now, std::size_t total) { - if (progress_callback) progress_callback(now, total); - return true; + auto result = client.Get(path, [this](std::size_t now, std::size_t total) -> bool { + if (progress_callback) return progress_callback(now, total); + return true; });注意:这需要将
progress_callback的类型从std::function<void(std::size_t, std::size_t)>改为std::function<bool(std::size_t, std::size_t)>。
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
example/widgets.cmake(2 hunks)example/widgets/util/download.cc(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- example/widgets.cmake
🔇 Additional comments (1)
example/widgets/util/download.cc (1)
22-32: LGTM!客户端配置和日志记录器的设置逻辑正确,捕获
start_time的方式是安全的。
Summary by CodeRabbit
发布说明
新功能
改进
✏️ Tip: You can customize this high-level summary in your review settings.