Skip to content

fix: improve breadcrumbs for mounted devices#3724

Open
Johnson-zs wants to merge 1 commit intolinuxdeepin:masterfrom
Johnson-zs:master
Open

fix: improve breadcrumbs for mounted devices#3724
Johnson-zs wants to merge 1 commit intolinuxdeepin:masterfrom
Johnson-zs:master

Conversation

@Johnson-zs
Copy link
Copy Markdown
Contributor

  1. Refactor device icon selection logic into helper function
    devIconName()
  2. Implement dedicated helper appendCrumbs() for building breadcrumb
    trails
  3. Fix SMB/gvfs mounted devices display in title bar crumbs
  4. Ensure consistent icon representation for different device types
  5. Improve path handling for mounted device breadcrumbs
  6. Optimize synchronous file info creation for better performance

Log: Improved display of mounted devices in title bar breadcrumbs

Influence:

  1. Test title bar breadcrumbs for different mounted devices (SMB, USB,
    etc.)
  2. Verify correct icons appear for various device types
  3. Check breadcrumb behavior with nested directories on mounted devices
  4. Validate path handling for both block devices and protocol mounts

fix: 优化挂载设备在标题栏面包屑中的显示

  1. 将设备图标选择逻辑重构为 helper 函数 devIconName()
  2. 实现专用的 appendCrumbs() helper 用于构建面包屑路径
  3. 修复 SMB/gvfs 挂载设备在标题栏面包屑中的显示问题
  4. 确保不同类型设备图标显示的一致性
  5. 改进挂载设备路径处理方式
  6. 优化同步文件信息创建以提高性能

Log: 优化标题栏中挂载设备的显示效果

Influence:

  1. 测试不同类型挂载设备(SMB, USB等)的标题栏面包屑显示
  2. 验证各类设备图标是否正确显示
  3. 检查挂载设备上嵌套目录的面包屑行为
  4. 验证块设备和协议挂载的路径处理是否正确

Bug: https://pms.uniontech.com/bug-view-357197.html

1. Refactor device icon selection logic into helper function
devIconName()
2. Implement dedicated helper appendCrumbs() for building breadcrumb
trails
3. Fix SMB/gvfs mounted devices display in title bar crumbs
4. Ensure consistent icon representation for different device types
5. Improve path handling for mounted device breadcrumbs
6. Optimize synchronous file info creation for better performance

Log: Improved display of mounted devices in title bar breadcrumbs

Influence:
1. Test title bar breadcrumbs for different mounted devices (SMB, USB,
etc.)
2. Verify correct icons appear for various device types
3. Check breadcrumb behavior with nested directories on mounted devices
4. Validate path handling for both block devices and protocol mounts

fix: 优化挂载设备在标题栏面包屑中的显示

1. 将设备图标选择逻辑重构为 helper 函数 devIconName()
2. 实现专用的 appendCrumbs() helper 用于构建面包屑路径
3. 修复 SMB/gvfs 挂载设备在标题栏面包屑中的显示问题
4. 确保不同类型设备图标显示的一致性
5. 改进挂载设备路径处理方式
6. 优化同步文件信息创建以提高性能

Log: 优化标题栏中挂载设备的显示效果

Influence:
1. 测试不同类型挂载设备(SMB, USB等)的标题栏面包屑显示
2. 验证各类设备图标是否正确显示
3. 检查挂载设备上嵌套目录的面包屑行为
4. 验证块设备和协议挂载的路径处理是否正确

Bug: https://pms.uniontech.com/bug-view-357197.html
Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @Johnson-zs, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@deepin-ci-robot
Copy link
Copy Markdown
Contributor

[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:

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
Copy Markdown
Contributor

deepin pr auto review

这段代码主要包含两个文件的修改,涉及到了设备挂载路径面包屑解析和标题栏辅助工具的优化。我将从语法逻辑、代码质量、代码性能和代码安全四个方面进行详细审查。

1. 语法逻辑审查

computereventreceiver.cpp 修改部分:

  • 改进点: 引入了 devIconNameappendCrumbs 两个匿名命名空间内的静态辅助函数,将复杂的逻辑从主函数 parseDevMountCrumb 中剥离出来。这极大地提高了代码的可读性和模块化程度。
  • 逻辑问题(严重):devIconName 函数中,对于非媒体、非移动设备、非Android/iOS设备的情况,函数返回空字符串 {}
    • parseDevMountCrumb 中,紧接着调用了 if (iconName.isEmpty()) return false;
    • 后果: 这意味着如果设备图标不符合上述特定条件(例如普通的本地硬盘、网络硬盘等),面包屑将完全无法生成。原代码中有一个 else 分支执行 iconName += "-symbolic";,这保证了即使没有特殊匹配,也会有一个默认的符号图标。
    • 建议: 修改 devIconName 函数,为默认情况提供一个兜底逻辑,而不是返回空字符串。例如,返回原始图标名加上 -symbolic 后缀,或者返回一个通用的 drive-harddisk-symbolic

titlebarhelper.cpp 修改部分:

  • 改进点:InfoFactory::create 调用中增加了 Global::CreateFileInfoType::kCreateFileInfoSync 参数。
  • 逻辑: 这表明开发者希望同步获取文件信息,这通常是合理的,因为后续代码立即使用了 infoPointer。如果异步创建,后续代码可能在没有准备好的对象上操作,导致空指针或无效数据。

2. 代码质量审查

  • 优点:

    • 代码复用与解耦: computereventreceiver.cpp 将图标选择和面包屑构建逻辑提取为独立函数,符合单一职责原则,使得主函数 parseDevMountCrumb 的流程非常清晰(验证 -> 获取信息 -> 构建面包屑)。
    • Const 正确性: 新代码大量使用了 const 修饰符(如 const QString filePath, const auto iconName),这有助于防止意外修改数据,提高代码健壮性。
    • 命名规范: 辅助函数命名清晰,如 devIconNameappendCrumbs,直观表达了其功能。
  • 缺点:

    • 魔法字符串: 代码中依然存在硬编码的字符串,如 "media-optical-symbolic""CrumbData_Key_Url" 等。虽然这可能是项目现有的规范,但集中定义这些常量会更好,便于维护和避免拼写错误。

3. 代码性能审查

  • 优点:

    • 减少重复计算:appendCrumbs 中,currPrefix 的构建方式(currPrefix += '/' + dirName)比原代码中可能存在的多次字符串拼接更高效。
    • 避免不必要的拷贝: 使用 const QString & 传递字符串,以及 QList<QVariantMap> * 指针传递容器,避免了大数据量的拷贝。
  • 潜在问题:

    • 文件信息查询: InfoFactory::create<...>(..., kCreateFileInfoSync) 是一个同步IO操作。在UI线程(根据代码上下文推测这可能是UI相关逻辑)中执行同步IO可能会导致界面卡顿,特别是当文件系统响应慢或网络延迟高时。如果这是在主线程调用,需要权衡UI响应速度和数据准确性。

4. 代码安全审查

  • 路径处理安全:

    • appendCrumbs 中,使用 fullPath.mid(devPath.size()) 来获取子路径。这里假设 devPath 一定是 fullPath 的前缀。虽然前面的逻辑 if (!DevProxyMng->isFileOfExternalMounts(filePath)) 做了一定的过滤,但在处理字符串时,最好确保 devPath 确实以 / 结尾(如果不是根目录),或者确保 fullPath 确实以 devPath 开头,否则 mid 可能截取到错误的路径段。不过原代码也是类似逻辑,这里主要依赖 DevProxyMng 的正确性。
    • 使用 QUrl::fromLocalFile 构建URL是安全的做法,避免了手动拼接URL字符串可能带来的注入或格式错误风险。
  • 空指针检查:

    • const DFMEntryFileInfoPointer info(new EntryFileInfo(devUrl)); 这里使用了裸指针初始化智能指针。虽然 std::shared_ptr(假设 DFMEntryFileInfoPointer 是其别名)能管理内存,但如果 new EntryFileInfo 抛出异常,程序可能会中断。不过考虑到 Qt 的异常处理机制通常不鼓励使用异常,这点问题不大。
    • 后续检查了 if (!info || !info->targetUrl().isLocalFile()),这是很好的防御性编程实践。

综合改进建议

  1. 修复逻辑漏洞(关键): 修改 computereventreceiver.cpp 中的 devIconName 函数,确保它总是返回一个有效的图标名称,而不是空字符串。

    // 修改后的 devIconName 示例
    static QString devIconName(const DFMEntryFileInfoPointer &info)
    {
        const QString iconName = info->fileIcon().name();
        if (iconName.startsWith("media"))
            return "media-optical-symbolic";
        if (info->order() == AbstractEntryFileEntity::kOrderRemovableDisks)
            return "drive-removable-media-symbolic";
        if (iconName == "android-device")
            return "phone-symbolic";
        if (iconName == "ios-device")
            return "phone-apple-iphone-symbolic";
        
        // 默认处理:添加 -symbolic 后缀,或者返回通用图标
        if (!iconName.isEmpty()) {
            return iconName + "-symbolic";
        }
        return "drive-harddisk-symbolic"; // 最终兜底
    }
  2. 性能优化建议: 对于 titlebarhelper.cpp 中的同步文件信息获取,建议确认这是否在主线程执行。如果是,且性能测试发现卡顿,考虑:

    • 使用缓存机制。
    • 或者改用异步方式,先显示默认名称,获取到真实名称后再更新UI。
  3. 代码健壮性:appendCrumbs 中,虽然逻辑上 filePath 应该包含 devPath,但为了绝对安全,可以加一个断言或检查:

    Q_ASSERT(fullPath.startsWith(devPath));
    if (!fullPath.startsWith(devPath)) {
        // 记录错误日志或做异常处理
        return;
    }
  4. 常量管理: 建议将 "CrumbData_Key_Url", "CrumbData_Key_IconName" 等字符串键值定义为 static const QString 或者在类的头文件中定义为静态常量,以减少拼写错误并提高可维护性。

总结来说,这次代码重构在结构和清晰度上有很大提升,但在 devIconName 的默认处理逻辑上存在一个可能导致功能退化的严重缺陷,必须修复。同时,同步IO操作值得在性能测试中关注。

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