Skip to content

Conversation

@mhduiy
Copy link
Contributor

@mhduiy mhduiy commented Oct 16, 2025

  1. Added foundSize function to parse icon size from directory names using multiple strategies
  2. Implemented duplicate entry detection to skip existing icons in DCI files
  3. Added QFileInfo and QLogging includes for new functionality
  4. Removed debug-specific code that was adding unnecessary prefixes
  5. Improved size detection logic to handle both numeric and "NxN" format directory names
  6. Added fallback to parent directory when current directory name doesn't contain size information

Log: Improved icon theme generation with better size detection and duplicate prevention

Influence:

  1. Test icon theme generation with various directory naming conventions
  2. Verify that duplicate icons are properly skipped during generation
  3. Test with directories containing numeric names vs "widthxheight" format
  4. Verify fallback to parent directory size detection works correctly
  5. Check that existing icons in DCI files are not overwritten
  6. Test with different directory structures and naming patterns

feat: 改进图标尺寸检测和重复项处理

  1. 添加 foundSize 函数,使用多种策略从目录名解析图标尺寸
  2. 实现重复条目检测,跳过 DCI 文件中已存在的图标
  3. 添加 QFileInfo 和 QLogging 包含以支持新功能
  4. 移除添加不必要前缀的调试专用代码
  5. 改进尺寸检测逻辑,支持数字和"NxN"格式的目录名
  6. 在当前目录名不包含尺寸信息时,添加回退到父目录的检测

Log: 改进图标主题生成,提供更好的尺寸检测和重复项预防

Influence:

  1. 测试使用不同目录命名约定的图标主题生成
  2. 验证在生成过程中重复图标是否被正确跳过
  3. 测试包含数字名称与"宽x高"格式的目录
  4. 验证回退到父目录尺寸检测是否正常工作
  5. 检查 DCI 文件中现有图标是否不会被覆盖
  6. 测试不同的目录结构和命名模式

@mhduiy mhduiy requested a review from 18202781743 October 16, 2025 10:55
deepin-ci-robot added a commit to linuxdeepin/dtk6gui that referenced this pull request Oct 16, 2025
Synchronize source files from linuxdeepin/dtkgui.

Source-pull-request: linuxdeepin/dtkgui#348
@deepin-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, mhduiy

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

1. Added foundSize function to parse icon size from directory names
using multiple strategies
2. Implemented duplicate entry detection to skip existing icons in DCI
files
3. Added QFileInfo and QLogging includes for new functionality
4. Removed debug-specific code that was adding unnecessary prefixes
5. Improved size detection logic to handle both numeric and "NxN" format
directory names
6. Added fallback to parent directory when current directory name
doesn't contain size information

Log: Improved icon theme generation with better size detection and
duplicate prevention

Influence:
1. Test icon theme generation with various directory naming conventions
2. Verify that duplicate icons are properly skipped during generation
3. Test with directories containing numeric names vs "widthxheight"
format
4. Verify fallback to parent directory size detection works correctly
5. Check that existing icons in DCI files are not overwritten
6. Test with different directory structures and naming patterns

feat: 改进图标尺寸检测和重复项处理

1. 添加 foundSize 函数,使用多种策略从目录名解析图标尺寸
2. 实现重复条目检测,跳过 DCI 文件中已存在的图标
3. 添加 QFileInfo 和 QLogging 包含以支持新功能
4. 移除添加不必要前缀的调试专用代码
5. 改进尺寸检测逻辑,支持数字和"NxN"格式的目录名
6. 在当前目录名不包含尺寸信息时,添加回退到父目录的检测

Log: 改进图标主题生成,提供更好的尺寸检测和重复项预防

Influence:
1. 测试使用不同目录命名约定的图标主题生成
2. 验证在生成过程中重复图标是否被正确跳过
3. 测试包含数字名称与"宽x高"格式的目录
4. 验证回退到父目录尺寸检测是否正常工作
5. 检查 DCI 文件中现有图标是否不会被覆盖
6. 测试不同的目录结构和命名模式
deepin-ci-robot added a commit to linuxdeepin/dtk6gui that referenced this pull request Oct 16, 2025
Synchronize source files from linuxdeepin/dtkgui.

Source-pull-request: linuxdeepin/dtkgui#348
@mhduiy
Copy link
Contributor Author

mhduiy commented Oct 16, 2025

/forcemerge

@deepin-ci-robot
Copy link
Contributor

deepin pr auto review

代码审查报告

语法逻辑

  1. foundSize 函数实现了从文件路径中解析图标尺寸的功能,逻辑基本合理,但可以进一步优化错误处理。
  2. 主函数中的文件处理流程清晰,但存在一些可以简化的条件判断。
  3. dciChecker 函数的实现简洁有效,用于检查操作结果并处理错误。

代码质量

  1. 优点

    • 使用了 lambda 表达式使代码更简洁
    • 有适当的错误处理机制
    • 变量命名清晰,易于理解
  2. 改进建议

    • foundSize 函数中的 parseSize lambda 可以提取为独立函数,提高代码复用性
    • 主函数中的 qualityList 处理逻辑可以封装为独立函数
    • 建议添加更多的注释说明关键业务逻辑

代码性能

  1. foundSize 函数在解析目录名时进行了多次字符串操作,可以考虑优化:

    • 使用 QStringRef 进行部分字符串操作,减少字符串拷贝
    • 缓存已解析的目录名,避免重复计算
  2. 主函数中的文件遍历可以:

    • 考虑使用并行处理提高大文件集合的处理速度
    • 添加进度显示,提升用户体验

代码安全

  1. foundSize 函数中的字符串转换操作:

    • toUInt() 的结果进行了 ok 检查,这是好的实践
    • 建议添加对解析结果的范围检查,确保尺寸值在合理范围内
  2. 文件操作相关:

    • 所有文件操作都有错误检查,这是好的实践
    • 建议添加对文件权限的检查,避免因权限不足导致的运行时错误
  3. 输入验证:

    • qualityList 的分割结果进行了大小检查
    • 建议对所有外部输入进行更严格的验证,防止潜在的注入攻击

具体改进建议

  1. parseSize 提取为独立函数:
static uint parseSizeFromString(const QString &dirName) {
    bool ok;
    if (int size = dirName.toUInt(&ok); ok && size > 0 && size <= 512) {  // 添加范围检查
        return size;
    }

    if (dirName.contains('x') && dirName.split('x').size() == 2) {
        if (int size = dirName.split('x').first().toUInt(&ok); ok && size > 0 && size <= 512) {
            return size;
        }
    }

    return 0;
}
  1. 添加尺寸范围常量定义:
static const uint MIN_ICON_SIZE = 16;
static const uint MAX_ICON_SIZE = 512;
  1. 主函数中的文件处理部分可以考虑使用并行处理:
// 使用 QtConcurrent 进行并行处理
QList<QFuture<void>> futures;
for (const QFileInfo &file : files) {
    futures.append(QtConcurrent::run([&, file]() {
        // 原有的文件处理逻辑
    }));
}
// 等待所有任务完成
for (auto &future : futures) {
    future.waitForFinished();
}
  1. 添加更详细的日志记录:
qDebug() << "Processing file:" << file.absoluteFilePath();
qDebug() << "Detected icon size:" << iconSize;

总体而言,这段代码结构清晰,功能实现完整,但仍有优化空间,特别是在性能、安全性和代码组织方面。建议按照上述建议进行改进,以提高代码质量和可维护性。

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Oct 16, 2025

This pr force merged! (status: blocked)

@deepin-bot deepin-bot bot merged commit 39a50cc into linuxdeepin:master Oct 16, 2025
20 of 22 checks passed
mhduiy pushed a commit to linuxdeepin/dtk6gui that referenced this pull request Oct 16, 2025
Synchronize source files from linuxdeepin/dtkgui.

Source-pull-request: linuxdeepin/dtkgui#348
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