master

[Johnson-zs]

• Revert "test: enhance file sorting tests with directories and symlinks"
• Revert "refactor: optimize file sorting implementation"

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Johnson-zs

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

Details

Needs approval from an approver in each of these files:

• debian/deepin/OWNERS

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]

deepin pr auto review

Git Diff 代码审查报告

总体评估

这段代码主要进行了以下修改：

1. 重构了测试框架，从自定义测试入口切换到Qt标准的QTEST_MAIN宏
2. 删除了DFileSorter及其相关类（DSortKeyCache）
3. 简化了DEnumerator中的排序逻辑
4. 调整了CMakeLists.txt构建配置

以下是对各个方面的详细审查和改进建议：

1. 语法逻辑分析

CMakeLists.txt 中的问题

if(DFM_BUILD_WITH_QT6)
    target_include_directories(dfm-io-test
        PRIVATE
        ${CMAKE_SOURCE_DIR}/src/dfm-io/dfm-io
    )
else()
    target_include_directories(dfm-io-test
        PRIVATE
        ${CMAKE_SOURCE_DIR}/src/dfm-io/dfm-io
    )
endif()

问题：if和else分支完全相同，这是冗余代码。此外，删除了之前包含的${CMAKE_SOURCE_DIR}/src/dfm-io/dfm-io/sort路径，但不确定这是否是故意的。

改进建议：

# 如果DFM_BUILD_WITH_QT6需要不同的处理，应该明确区分
target_include_directories(dfm-io-test
    PRIVATE
    ${CMAKE_SOURCE_DIR}/src/dfm-io/dfm-io
    ${CMAKE_SOURCE_DIR}/src/dfm-io/dfm-io/sort  # 如果需要sort目录
)

denumerator.cpp 中的问题

while (1) {
    FTSENT *ent = fts_read(d->fts);
    // ...
}

问题：使用while(1)而不是while(true)不符合现代C++风格，可读性较差。

改进建议：

while (true) {
    FTSENT *ent = fts_read(d->fts);
    // ...
}

2. 代码质量分析

删除DFileSorter的影响

代码中删除了DFileSorter及其相关类，包括：

• 按名称、大小、修改时间、访问时间的排序功能
• 混排/分离目录和文件的选项
• 符号链接的特殊处理

问题：删除这些功能后，DEnumerator::sortFileInfoList()的实现变得非常简单，只保留了基本的目录/文件分离功能，没有实现真正的排序逻辑。这可能导致功能缺失。

改进建议：

1. 确认删除排序功能是否是预期行为
2. 如果需要保留排序功能，应该在DEnumerator中实现新的排序逻辑
3. 如果不需要排序功能，应该更新相关文档和接口说明

测试代码简化

// 之前
int run_tst_DfmIO(int argc, char *argv[])
{
    tst_DfmIO tc;
    return QTest::qExec(&tc, argc, argv);
}

// 之后
QTEST_MAIN(tst_DfmIO)

评价：这是一个好的改进，使用Qt标准的QTEST_MAIN宏简化了测试代码，减少了样板代码。

3. 代码性能分析

文件遍历性能

while (1) {
    FTSENT *ent = fts_read(d->fts);
    // ...
    d->insertSortFileInfoList(listFile, listDir, ent, d->fts, hideList);
}

问题：代码中没有显示insertSortFileInfoList的实现，但如果它只是简单地将文件信息添加到列表中，而没有进行排序，那么性能可能会受到影响，特别是对于大型目录。

改进建议：

1. 如果需要排序，考虑在遍历过程中进行部分排序，而不是在遍历完成后统一排序
2. 如果不需要排序，应该明确说明这一点，并考虑是否需要保留sortFileInfoList方法

4. 代码安全分析

内存管理

fts_close(d->fts);
d->fts = nullptr;

// Clean up allocated memory
free(dirPath);

评价：正确关闭了fts句柄并释放了dirPath，这是好的做法。

潜在的空指针问题

if (!d->fts)
    d->openDirByfts();

if (!d->fts)
    return {};

评价：正确检查了fts指针是否为空，并在失败时返回空列表，这是安全的做法。

5. 其他建议

测试覆盖率

删除了DFileSorter相关的测试，但没有添加新的测试来验证简化后的排序逻辑。建议：

1. 添加新的测试用例来验证简化后的sortFileInfoList方法
2. 如果删除排序功能是预期行为，应该更新相关测试用例

文档更新

删除DFileSorter后，应该更新以下内容：

1. API文档
2. 用户手册
3. 开发者指南
4. 更新日志

总结

这段代码主要进行了测试框架的重构和排序功能的简化。主要问题包括：

1. CMakeLists.txt中的冗余代码
2. 删除排序功能可能导致功能缺失
3. 测试覆盖率可能不足
4. 文档可能需要更新

建议：

1. 清理CMakeLists.txt中的冗余代码
2. 确认删除排序功能是否是预期行为
3. 添加新的测试用例
4. 更新相关文档
5. 考虑使用更现代的C++风格（如while(true)代替while(1)）

[Johnson-zs]

/forcemerge

[deepin-bot]

This pr force merged! (status: blocked)