Skip to content

Import stdexec and httplib and refactor example widgets#18

Open
creeper5820 wants to merge 12 commits intomainfrom
refactor/example
Open

Import stdexec and httplib and refactor example widgets#18
creeper5820 wants to merge 12 commits intomainfrom
refactor/example

Conversation

@creeper5820
Copy link
Owner

@creeper5820 creeper5820 commented Dec 16, 2025

Summary by CodeRabbit

发布说明

  • 新功能

    • 资源中心:新增完整字体下载 UI、保存位置选择与打开文件夹操作
    • 异步调度:集成 Qt 线程调度器以便将任务调度回主线程
    • 下载任务:新增可配置的 HTTP 下载任务,支持进度回调与代理读取
  • 改进

    • 通知栏:重构为新 Snackbar 类型并提供消息接收接口
    • 文本组件:新增文本格式、外部链接与自动调整尺寸选项
    • 构建/兼容性:增强构建配置与平台兼容性(包括代理与 MinGW 支持)

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 16, 2025

Walkthrough

新增 AssetCenter 小部件(PIMPL)并实现基于 stdexec 的异步下载流水线;新增下载任务、代理解析、Qt 调度器、httplib 兼容层、工具与字符串常量;调整 snackbar/text API、示例构建依赖、CI 安装项及 .clangd 编译标志。

Changes

Cohort / File(s) 变更摘要
编译配置
\.clangd
移除 -mno-direct-extern-access,添加 -fconcepts-diagnostics-depth=10
Snackbar API
creeper-qt/widget/snackbar.hh
引入 snackbar::details::Snackbar(PIMPL 风格),将 Message 变为嵌套类型并更新 SnackbarMessage 别名;移除旧独立 Message
Text Widget API
creeper-qt/widget/text.hh
新增公开别名 TextFormatOpenExternalLinksAdjustSize;将 WordWrap 回调参数由 const auto& 改为 auto
示例构建调整
example/widgets.cmake
使用 FetchContent 引入 stdexeccpp-httplib,添加 zstd 兼容处理,并将新库作为可执行目标的 PRIVATE 依赖(stdexechttplib::httplibstdc++exp 等)。
资源中心组件(头/实现)
example/widgets/components/asset-center.hh, example/widgets/components/asset-center.cc
将 AssetCenter 改为 PIMPL 实现;在实现文件中构建 UI 布局并实现位置选择、异步下载流程、进度与错误处理,公开构造函数与默认析构。
下载任务抽象(头/实现)
example/widgets/util/download.hh, example/widgets/util/download.cc
新增 util::GetUrlTask:包含 base_url、path、可选 save_path 与进度回调;实现 configure_client()(超时、重定向、代理)与 execute()(发起 GET、处理状态并写入文件,失败时抛异常)。
代理支持(头/实现)
example/widgets/util/proxy.hh, example/widgets/util/proxy.cc
新增 parse_proxy_from_env()read_proxy(),从 http_proxy/https_proxy 环境变量解析 host:port 并为 httplib::Client 设置代理(使用 std::println 做诊断输出)。
Qt/stdexec 调度器
example/widgets/util/executor.hh
新增 QtSchedulerQtScheduleSender 与模板 QtOperationState,通过 QMetaObject::invokeMethod 将 stdexec 任务调度到 Qt 对象线程上下文,包含 stop token 处理与基本错误路径。
字符串常量
example/widgets/util/string.hh
添加字体下载相关常量:GitHub 基址、字体路径、文件名与含说明的 HTML 描述文本。
httplib 兼容层
example/widgets/util/httplib.hh
为 Windows/MinGW 提供对 GetAddrInfoExCancel 的惰性解析包装以兼容 httplib;其它平台仅包含 httplib.h
CI / 依赖安装
.github/workflows/example-build.yml
在 Linux/Windows 依赖安装步骤中添加 git,并在 Linux 包列表中加入 libzstd-devzstdlibzstd1

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: 打开保存目录
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 分钟

需要额外关注的点:

  • example/widgets/components/asset-center.cc:stdexec 异步编排、异常传播、线程间通信与 UI 状态更新。
  • example/widgets/util/executor.hh:QMetaObject::invokeMethod 使用、stop token 处理与线程上下文正确性。
  • example/widgets/util/download.cc:HTTP 超时/重定向/代理设置、进度回调与文件写入错误处理(异常语义)。
  • example/widgets/util/proxy.{cc,hh}:环境变量解析对多种 URL/端口格式的鲁棒性。
  • example/widgets/util/httplib.hh:Windows/MinGW 动态解析实现与线程安全性。
  • PIMPL 生命周期管理与头/实现一致性(AssetCenter、Snackbar)。

Poem

🐰 新枝跳跃我欢腾,
网线轻唤字体行踪,
Qt 与 stdexec 手牵手,
代理悄语穿云风,
小部件亮起,兔儿歌颂。

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 标题准确地概括了PR的主要变更:导入stdexec和httplib库,以及重构示例组件。这反映了CMake配置、下载工具实现、执行器集成和资源中心重构等核心改动。
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor/example

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 737b931 and 3a67d55.

📒 Files selected for processing (2)
  • example/widgets.cmake (2 hunks)
  • example/widgets/components/asset-center.cc (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • example/widgets.cmake
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-17T14:36:12.565Z
Learnt from: creeper5820
Repo: creeper5820/creeper-qt PR: 18
File: example/widgets/util/download.cc:19-19
Timestamp: 2025-12-17T14:36:12.565Z
Learning: In the creeper-qt project, ensure all UI-facing messages in the directory example/widgets (and subdirectories) are localized to Chinese (zh-CN) to align with user expectations. Update string literals, resource bundles, and translations accordingly; verify through UI text checks and translation files to confirm consistency.

Applied to files:

  • example/widgets/components/asset-center.cc
🧬 Code graph analysis (1)
example/widgets/components/asset-center.cc (1)
creeper-qt/widget/text.hh (2)
  • manager (17-20)
  • manager (17-17)
🔇 Additional comments (1)
example/widgets/components/asset-center.cc (1)

144-144: str::kFontDescription 已正确本地化为中文。

验证完成。在 example/widgets/util/string.hh 中定义的 kFontDescription 已使用中文文本("示例中的所有 Icon 均为 Google Material Icons 字体提供"),符合项目要求。代码无需修改。


Comment @coderabbitai help to get the list of available commands and usage tips.

@creeper5820 creeper5820 self-assigned this Dec 16, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

🧹 Nitpick comments (9)
creeper-qt/widget/text.hh (1)

36-36: 考虑统一参数传递方式以提高一致性。

Color 属性使用 const auto& v 传递 QColor,而新增的属性(如 TextFormatOpenExternalLinks)使用 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,与 httplibv0.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

📥 Commits

Reviewing files that changed from the base of the PR and between 3731216 and 08e07a2.

📒 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 契约,特别是在以下情况下:

  • Messagestd::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 输入,则应添加验证。

Comment on lines +31 to +35
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");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

必须检查文件写入操作是否成功。

当前代码没有检查 stream.writestream.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.

Suggested change
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");

Comment on lines +33 to +50
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!";
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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.

Comment on lines +17 to +20
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) }));
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
example/widgets/util/download.cc (1)

42-66: 先前审查中标记的问题仍未解决。

此代码段存在先前审查中已标记的两个问题:

  1. 语言不一致(第 44 行和第 59 行):错误消息使用中文"操作取消",而成功消息使用英文"Download successfully"。
  2. 缺少文件写入错误检查(第 55-59 行):没有检查文件流是否成功打开、写入和刷新操作是否成功。如果磁盘空间不足或权限不够,写入可能会静默失败。

请参考之前的审查评论以获取详细的修复建议。

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7d8815b and a4925e4.

📒 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)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between a4925e4 and cc97840.

📒 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 状态码错误,逻辑正确。

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (3)
example/widgets/util/download.cc (3)

12-12: 读取超时设置过短。

2秒的读取超时对于下载可能较大的资产文件过短,在网络较慢或文件较大时会导致正常下载失败。建议增加到至少30秒。


18-20: 错误消息语言不一致。

此处使用中文错误消息("操作取消"),但第43行的成功消息使用英文("Download successfully")。建议统一使用一种语言以保持一致性。


39-43: 必须检查文件操作是否成功。

当前代码未检查:

  1. 文件流是否成功打开(第40行)
  2. writeflush 操作是否成功(第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

📥 Commits

Reviewing files that changed from the base of the PR and between cc97840 and 737b931.

📒 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 的方式是安全的。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant