Skip to content

sync: from linuxdeepin/qt5integration#77

Merged
18202781743 merged 1 commit intomasterfrom
sync-pr-268-nosync
May 26, 2025
Merged

sync: from linuxdeepin/qt5integration#77
18202781743 merged 1 commit intomasterfrom
sync-pr-268-nosync

Conversation

@deepin-ci-robot
Copy link
Contributor

Synchronize source files from linuxdeepin/qt5integration.

Source-pull-request: linuxdeepin/qt5integration#268

Synchronize source files from linuxdeepin/qt5integration.

Source-pull-request: linuxdeepin/qt5integration#268
@deepin-ci-robot
Copy link
Contributor Author

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: deepin-ci-robot

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

@deepin-ci-robot
Copy link
Contributor Author

deepin pr auto review

代码审查意见:

  1. 代码可读性

    • if语句中,条件判断的顺序和注释应该保持一致,以提高代码的可读性。例如,先检查文件是否存在,再检查文件是否有效。
  2. 性能优化

    • QFileInfo对象svgFileInfo在两个地方被创建,可以将其移动到if语句外部,以避免重复创建对象。这样可以减少不必要的系统调用,提高性能。
  3. 代码重复

    • QFileInfo cacheFileInfo(cacheFile);这行代码在if语句内部被重复创建,应该将其移动到if语句外部,以避免重复代码。
  4. 逻辑错误

    • 注释中提到ostree管理的源文件 lastModified 被重置为1970-01-01,qt认为是无效时间,无法设置到缓存文件中,但是代码中并没有处理这种情况。如果svgFileInfo.lastModified()返回的是无效时间,那么svgFileInfo.lastModified() != cacheFileInfo.lastModified()的比较可能会出错。应该添加对无效时间的处理逻辑。
  5. 代码风格

    • 注释应该使用中文,但是代码中的英文注释应该保持一致,以避免混淆。

综合以上意见,修改后的代码可能如下:

QPixmap QSvgIconEngine::pixmap(const QSize &size, QIcon::Mode mode,
                               const QTransform &transform) {
    // ... 其他代码 ...

    QString cacheFile;

    const QFileInfo svgFileInfo(svgFile);
    // ostree管理的源文件 lastModified 被重置为1970-01-01,qt认为是无效时间,无法设置到缓存文件中
    if (Q_LIKELY(!svgFile.startsWith(":/") && svgFileInfo.exists() && svgFileInfo.lastModified().isValid())) {
        static const QString &cachePath = getIconCachePath();

        if (Q_LIKELY(!cachePath.isEmpty())) {
            cacheFile = cachePath + "/" + svgFile;
        }
    }

    const QFileInfo cacheFileInfo(cacheFile);
    if (Q_LIKELY(cacheFileInfo.exists())) {
        if (Q_UNLIKELY(svgFileInfo.lastModified() != cacheFileInfo.lastModified())) {
            // clear invalid cache file
            QFile::remove(cacheFile);
        }
    }

    // ... 其他代码 ...
}

以上修改提高了代码的可读性、性能和一致性,并处理了潜在的逻辑错误。

@18202781743 18202781743 merged commit c4feda7 into master May 26, 2025
29 of 31 checks passed
@18202781743 18202781743 deleted the sync-pr-268-nosync branch May 26, 2025 02:09
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.

2 participants