Skip to content

Conversation

@Kakueeen
Copy link
Contributor

@Kakueeen Kakueeen commented Nov 20, 2025

Refactored the filename display logic to simplify the code and improve
maintainability. The main changes include:

  1. Removed complex text layout calculations for base name and suffix
  2. Centralized ellipsis handling to ElideTextLayout::layout for
    consistent behavior
  3. Simplified the logic for handling file suffixes based on application
    settings
  4. Improved code readability by removing redundant text list operations

The previous implementation had separate ellipsis handling for base
names and suffixes, which could cause inconsistencies in text display.
Now all ellipsis processing is handled by ElideTextLayout::layout,
ensuring uniform behavior and better support for highlighting features.

Influence:

  1. Verify filename display in list view with various name lengths
  2. Test filename display with and without suffix visibility setting
  3. Check ellipsis behavior for long filenames
  4. Verify highlighting functionality works correctly
  5. Test desktop file display remains unaffected

重构:优化文件名显示逻辑

重构了文件名显示逻辑以简化代码并提高可维护性。主要变更包括:

  1. 移除了对基础名称和后缀的复杂文本布局计算
  2. 将省略处理集中到 ElideTextLayout::layout 以实现一致的行为
  3. 简化了基于应用设置处理文件后缀的逻辑
  4. 通过移除冗余的文本列表操作提高了代码可读性

之前的实现分别处理基础名称和后缀的省略,这可能导致文本显示不一致。现在所
有省略处理都由 ElideTextLayout::layout 处理,确保统一的行为并更好地支持
高亮功能。

影响:

  1. 验证列表视图中不同长度文件名的显示
  2. 测试开启和关闭后缀显示设置时的文件名显示
  3. 检查长文件名的省略行为
  4. 验证高亮功能正常工作
  5. 测试桌面文件显示不受影响

BUG: https://pms.uniontech.com/bug-view-337877.html

Summary by Sourcery

Refactor filename rendering by centralizing ellipsis processing in ElideTextLayout and streamlining suffix visibility logic to simplify code and ensure consistent behavior

Bug Fixes:

  • Fix inconsistent ellipsis behavior between base filename and suffix display

Enhancements:

  • Refactor filename display logic to remove manual text layout calls and rely on ElideTextLayout::layout for ellipsis handling
  • Simplify file suffix appending based on application settings and eliminate redundant code paths

Refactored the filename display logic to simplify the code and improve
maintainability. The main changes include:
1. Removed complex text layout calculations for base name and suffix
2. Centralized ellipsis handling to ElideTextLayout::layout for
consistent behavior
3. Simplified the logic for handling file suffixes based on application
settings
4. Improved code readability by removing redundant text list operations

The previous implementation had separate ellipsis handling for base
names and suffixes, which could cause inconsistencies in text display.
Now all ellipsis processing is handled by ElideTextLayout::layout,
ensuring uniform behavior and better support for highlighting features.

Influence:
1. Verify filename display in list view with various name lengths
2. Test filename display with and without suffix visibility setting
3. Check ellipsis behavior for long filenames
4. Verify highlighting functionality works correctly
5. Test desktop file display remains unaffected

重构:优化文件名显示逻辑

重构了文件名显示逻辑以简化代码并提高可维护性。主要变更包括:
1. 移除了对基础名称和后缀的复杂文本布局计算
2. 将省略处理集中到 ElideTextLayout::layout 以实现一致的行为
3. 简化了基于应用设置处理文件后缀的逻辑
4. 通过移除冗余的文本列表操作提高了代码可读性

之前的实现分别处理基础名称和后缀的省略,这可能导致文本显示不一致。现在所
有省略处理都由 ElideTextLayout::layout 处理,确保统一的行为并更好地支持
高亮功能。

影响:
1. 验证列表视图中不同长度文件名的显示
2. 测试开启和关闭后缀显示设置时的文件名显示
3. 检查长文件名的省略行为
4. 验证高亮功能正常工作
5. 测试桌面文件显示不受影响

BUG: https://pms.uniontech.com/bug-view-337877.html
@sourcery-ai
Copy link

sourcery-ai bot commented Nov 20, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

This PR refactors the filename display logic in ListItemDelegate to remove manual layout computations, centralize ellipsis handling within ElideTextLayout::layout, and streamline suffix visibility based on application settings, improving readability and maintainability.

Sequence diagram for centralized ellipsis handling in filename display

sequenceDiagram
    participant ListItemDelegate
    participant ElideTextLayout
    participant Application
    ListItemDelegate->>Application: Query kShowedFileSuffix setting
    ListItemDelegate->>ListItemDelegate: Build displayName (baseName [+ suffix])
    ListItemDelegate->>ElideTextLayout: layout(rect, Qt::ElideRight, ...)
    ElideTextLayout-->>ListItemDelegate: Return elided text for display
Loading

Class diagram for refactored filename display logic in ListItemDelegate

classDiagram
    class ListItemDelegate {
        +paintFileName(painter, option, index, url, role, textLineHeight, rect)
        +getCorrectDisplayName(painter, index, option, url, role, textLineHeight, rect) : QString
    }
    class ElideTextLayout {
        +layout(rect, elideMode, highlight, brush, textList)
    }
    class Application {
        +instance()
        +genericAttribute(key)
        kShowedFileSuffix
    }
    ListItemDelegate --> ElideTextLayout : uses
    ListItemDelegate --> Application : queries settings
Loading

Flow diagram for simplified filename display logic

flowchart TD
    A["Start: paintFileName()"] --> B["Get displayName (baseName [+ suffix])"]
    B --> C["Check Application::kShowedFileSuffix"]
    C --> D["Append suffix if setting is true"]
    D --> E["Pass displayName to ElideTextLayout::layout"]
    E --> F["Display filename with ellipsis and highlighting"]
Loading

File-Level Changes

Change Details Files
Removed manual text layout and redundant operations
  • Deleted QScopedPointer ElideTextLayout creation
  • Removed manual layout calls with QRectF and textList
  • Eliminated redundant string join operations
src/plugins/filemanager/dfmplugin-workspace/views/listitemdelegate.cpp
Centralized ellipsis handling to ElideTextLayout::layout
  • Deferred all elide processing to ElideTextLayout::layout via comments
  • Removed separate elide invocation in fallback displayName path
src/plugins/filemanager/dfmplugin-workspace/views/listitemdelegate.cpp
Simplified suffix visibility logic
  • Introduced showSuffix flag from application settings
  • Appended suffix only when showSuffix is true
src/plugins/filemanager/dfmplugin-workspace/views/listitemdelegate.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@deepin-ci-robot
Copy link
Contributor

deepin pr auto review

我来分析一下这段代码的修改:

  1. 代码逻辑改进:
  • 原代码在处理文件名显示时,使用了ElideTextLayout来处理文本省略,但这种方式会导致高亮功能出现问题。
  • 新代码将文本省略处理统一交给了ElideTextLayout::layout来完成,这样可以确保高亮功能正常工作。
  • 简化了文件名处理逻辑,直接获取基础名称,然后根据设置决定是否添加后缀。
  1. 代码质量改进:
  • 移除了不必要的QScopedPointer声明,因为它在函数开始时就被创建但并未真正使用。
  • 减少了临时变量的使用(如textList、tmpFileName等),使代码更加简洁。
  • 添加了清晰的注释,说明了代码的意图。
  1. 代码性能改进:
  • 减少了字符串操作和布局计算,提高了性能。
  • 避免了不必要的文本布局计算,只在实际需要时才进行。
  1. 代码安全改进:
  • 移除了可能存在风险的字符串比较逻辑(tmpFileName.append(suffix) != itemFileDisplayName.toString())。
  • 使用更安全的字符串处理方式,直接使用Qt提供的数据访问方法。

建议:

  1. 可以考虑将displayName的初始化直接使用index.data(role).toString(),避免后续的空值判断。
  2. 对于FileUtils::isDesktopFileSuffix(url)的判断,可以考虑将其提取到函数开始处,减少嵌套层级。
  3. 可以考虑将showSuffix的获取移到函数开始处,避免重复获取。

总体来说,这次修改提高了代码的可维护性和性能,同时修复了高亮功能的问题,是一个良好的改进。

Copy link

@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.

Hey there - I've reviewed your changes and they look great!

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `src/plugins/filemanager/dfmplugin-workspace/views/listitemdelegate.cpp:686` </location>
<code_context>
+            displayName = index.data(kItemFileBaseNameRole).toString().remove('\n');

+            // 根据设置决定是否显示后缀
             bool showSuffix { Application::instance()->genericAttribute(Application::kShowedFileSuffix).toBool() };
             if (showSuffix)
                 displayName.append(suffix);
</code_context>

<issue_to_address>
**question:** Suffix display logic is now more direct, but may miss error handling for mismatched display names.

Consider whether removing the display name check could cause issues for files with unusual names or encodings, as the new logic may not always produce the intended display name.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

displayName = index.data(kItemFileBaseNameRole).toString().remove('\n');

// 根据设置决定是否显示后缀
bool showSuffix { Application::instance()->genericAttribute(Application::kShowedFileSuffix).toBool() };
Copy link

Choose a reason for hiding this comment

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

question: Suffix display logic is now more direct, but may miss error handling for mismatched display names.

Consider whether removing the display name check could cause issues for files with unusual names or encodings, as the new logic may not always produce the intended display name.

@deepin-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

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

@Johnson-zs
Copy link
Contributor

/forcemerge

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Nov 20, 2025

This pr force merged! (status: behind)

@deepin-bot deepin-bot bot merged commit 455a8d9 into linuxdeepin:master Nov 20, 2025
21 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