Skip to content

fix: Update getHtmlToPdfPath for Qt version compatibility and include cmath in XpsDocument#189

Merged
lzwind merged 2 commits intolinuxdeepin:masterfrom
dengzhongyuan365-dev:master
Sep 4, 2025
Merged

fix: Update getHtmlToPdfPath for Qt version compatibility and include cmath in XpsDocument#189
lzwind merged 2 commits intolinuxdeepin:masterfrom
dengzhongyuan365-dev:master

Conversation

@dengzhongyuan365-dev
Copy link
Contributor

fix: Update getHtmlToPdfPath for Qt version compatibility and include cmath in XpsDocument

- Updated getHtmlToPdfPath() to use conditional compilation for Qt version compatibility.
- Included <cmath> in XpsDocument.cpp for mathematical operations.

… cmath in XpsDocument

- Updated getHtmlToPdfPath() to use conditional compilation for Qt version compatibility.
- Included <cmath> in XpsDocument.cpp for mathematical operations.
@github-actions
Copy link

github-actions bot commented Sep 4, 2025

TAG Bot

TAG: 6.5.37
EXISTED: no
DISTRIBUTION: unstable

 - update version to 6.5.37

 log: update version to 6.5.37
@deepin-ci-robot
Copy link

deepin pr auto review

根据提供的git diff,我将对代码进行审查,并提出以下改进建议:

1. 版本更新

  • 版本号从6.5.36.1更新到6.5.37.1,这是一个常规的版本升级。
  • 建议确保所有平台(arm64、loong64等)的版本号同步更新,当前diff显示已正确同步。

2. getHtmlToPdfPath函数改进 (reader/document/Model.cpp)

#if (QT_VERSION > QT_VERSION_CHECK(6, 0, 0))
    path = QLibraryInfo::path(QLibraryInfo::LibrariesPath) + "/deepin-reader/htmltopdf";
#else
    path = QLibraryInfo::location(QLibraryInfo::LibrariesPath) + "/deepin-reader/htmltopdf";
#endif

优点:

  • 添加了Qt版本兼容性处理,考虑了Qt6和Qt5的差异。

改进建议:

  1. 路径拼接建议使用QDir::separator()来确保跨平台兼容性:
path = QLibraryInfo::path(QLibraryInfo::LibrariesPath) 
       + QDir::separator() 
       + "deepin-reader" 
       + QDir::separator() 
       + "htmltopdf";
  1. 建议添加错误处理和日志记录:
if (path.isEmpty()) {
    qWarning() << "Failed to get htmltopdf path";
    return QString();
}

3. XpsDocument.cpp改进

#include <cmath>

改进建议:

  1. 确保cmath头文件的使用是必要的,检查是否有实际的数学运算使用。
  2. 如果cmath仅用于少量数学运算,考虑是否可以使用更具体的数学头文件,如。

4. changelog记录

* fix: Update getHtmlToPdfPath for Qt version compatibility and include cmath in XpsDocument

改进建议:

  1. changelog记录可以更详细,例如:
* fix: Update getHtmlToPdfPath to support different Qt versions (6.x and 5.x)
* fix: Add cmath header for mathematical operations in XpsDocument

5. 整体建议

  1. 建议添加单元测试来验证getHtmlToPdfPath函数在不同Qt版本下的行为。
  2. 考虑将路径相关的配置提取到配置文件中,便于维护和修改。
  3. 建议在代码中添加更多注释,解释Qt版本兼容性处理的原因和背景。

6. 安全性考虑

  1. 在路径拼接时,建议验证路径的安全性,防止路径遍历攻击。
  2. 建议对返回的路径进行有效性检查,确保程序不会尝试访问不存在的路径。

总的来说,这次更新主要关注了Qt版本兼容性和依赖头文件的添加,这些都是良好的实践。建议在后续开发中继续保持对代码质量和安全性的关注。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dengzhongyuan365-dev, lzwind

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@lzwind lzwind merged commit 312bca3 into linuxdeepin:master Sep 4, 2025
7 checks passed
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.

3 participants