Skip to content

Conversation

@LiHua000
Copy link
Contributor

add XPS_SUPPORT_ENABLED to all Dr::Xps

Log: as title

add XPS_SUPPORT_ENABLED to all Dr::Xps

Log: as title
@deepin-ci-robot
Copy link

deepin pr auto review

我来对这个diff进行代码审查:

  1. 语法逻辑:
  • 代码语法正确,使用了条件编译宏 #ifdef XPS_SUPPORT_ENABLED 来控制XPS相关功能的编译
  • 逻辑上,当未启用XPS支持时,将 isXpsDocument 设为 false 是合理的做法
  • calculatePrintTargetSize 函数中,当不是XPS文档时直接返回空尺寸的做法是正确的
  1. 代码质量:
  • 优点:使用了条件编译来处理可选功能,避免了运行时的条件判断
  • 建议:可以考虑将 isXpsDocument 的判断逻辑封装成一个内联函数,避免重复代码,例如:
inline bool isXpsDocument() const {
#ifdef XPS_SUPPORT_ENABLED
    return m_fileType == Dr::XPS;
#else
    return false;
#endif
}
  1. 代码性能:
  • 使用条件编译可以减少运行时判断,提高性能
  • 建议在 calculatePrintTargetSize 函数中也使用类似的 isXpsDocument() 函数来保持一致性
  1. 代码安全:
  • 使用条件编译确保了在未启用XPS支持时不会执行相关代码,提高了代码安全性
  • 建议在 calculatePrintTargetSize 中添加对 pageRect 的有效性检查,类似于 targetRectForSize 中的检查
  1. 其他建议:
  • 可以考虑将 targetRectForSize 这个lambda表达式提取为一个私有成员函数,因为它在多个地方被使用
  • 建议添加更多的注释来说明条件编译的目的和使用场景

总的来说,这个改动是合理的,主要改进了代码的条件编译处理,但还可以通过一些小的重构来提高代码的可维护性和一致性。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: LiHua000, 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 a1c4d9e into linuxdeepin:master Nov 13, 2025
6 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