Skip to content

Conversation

@wjyrich
Copy link
Contributor

@wjyrich wjyrich commented Oct 30, 2025

  1. Add isTrashEmpty() method to detect if trash is empty using gio command
  2. Modify context menu to disable "Clean Trash" option when trash is empty
  3. Update trash tip text generation to use new queryTrashCountAndEmpty method
  4. Add property binding to dynamically enable/disable menu items based on trash state
  5. Use gio trash --list command for more reliable trash content detection

feat: 实现回收站空状态检测

  1. 添加 isTrashEmpty() 方法使用 gio 命令检测回收站是否为空
  2. 修改上下文菜单,在回收站为空时禁用"清空回收站"选项
  3. 更新回收站提示文本生成以使用新的 queryTrashCountAndEmpty 方法
  4. 添加属性绑定以根据回收站状态动态启用/禁用菜单项
  5. 使用 gio trash --list 命令进行更可靠的回收站内容检测

PMS: BUG-335901

Summary by Sourcery

Implement detection of an empty trash state using the gio command and update the UI to reflect and disable the “Clean Trash” action when no items are present

New Features:

  • Add isTrashEmpty() and queryTrashCountAndEmpty() methods to detect trash contents using the "gio trash --list" command

Enhancements:

  • Refactor getTrashTipText() to use the new query method for file count
  • Bind a trashEmpty property in QML to dynamically disable the "Clean Trash" context menu item when trash is empty

@wjyrich wjyrich requested review from 18202781743 and BLumia October 30, 2025 05:46
@sourcery-ai
Copy link

sourcery-ai bot commented Oct 30, 2025

Reviewer's Guide

This PR implements a reliable trash-empty detection feature by introducing a queryTrashCountAndEmpty method (using gio trash --list), refactoring tip text generation, exposing an isTrashEmpty invokable, and dynamically enabling/disabling the “Clean Trash” menu item based on the detected state.

Sequence diagram for context menu trash state detection and menu item enabling

sequenceDiagram
participant User as actor User
participant AppItem.qml
participant TaskManager
User->>AppItem.qml: Right-clicks on trash icon
AppItem.qml->>TaskManager: isTrashEmpty()
TaskManager->>TaskManager: queryTrashCountAndEmpty() (runs gio trash --list)
TaskManager-->>AppItem.qml: returns trashEmpty state
AppItem.qml->>AppItem.qml: Set contextMenuLoader.trashEmpty
AppItem.qml->>AppItem.qml: Enable/disable "Clean Trash" menu item
Loading

Class diagram for updated TaskManager trash detection methods

classDiagram
class TaskManager {
    +QString getTrashTipText()
    +bool isTrashEmpty() const
    +std::pair<int, bool> queryTrashCountAndEmpty() const
}
TaskManager : getTrashTipText() uses queryTrashCountAndEmpty()
TaskManager : isTrashEmpty() uses queryTrashCountAndEmpty()
Loading

File-Level Changes

Change Details Files
Implement centralized trash state query and wrappers
  • Introduce queryTrashCountAndEmpty returning file count and empty flag
  • Replace manual QDir-based count in getTrashTipText with the new query method
  • Add isTrashEmpty invokable proxy to expose empty state
  • Use QProcess to run gio trash --list for accurate detection
taskmanager.cpp
taskmanager.h
Dynamically enable/disable “Clean Trash” menu item
  • Add trashEmpty property to contextMenuLoader
  • Bind enabled of the clean-trash item to !trashEmpty
  • Refresh trashEmpty on right-click via TaskManager.isTrashEmpty()
AppItem.qml

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

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> `panels/dock/taskmanager/taskmanager.cpp:457-468` </location>
<code_context>
+    int count = 0;
+    bool empty = true;
+
+    QProcess gio;
+    gio.start("gio", QStringList() << "trash" << "--list");
+    if (gio.waitForFinished(1000) && gio.exitStatus() == QProcess::NormalExit && gio.exitCode() == 0) {
</code_context>

<issue_to_address>
**suggestion (bug_risk):** Consider error handling for QProcess failures.

Returning zero and 'empty' on process failure may hide real errors. Logging failures or adding a fallback would make the function more robust.

```suggestion
    QProcess gio;
    gio.start("gio", QStringList() << "trash" << "--list");
    if (gio.waitForFinished(1000) && gio.exitStatus() == QProcess::NormalExit && gio.exitCode() == 0) {
        const QByteArray out = gio.readAllStandardOutput();
        const QList<QByteArray> lines = out.split('\n');
        for (const QByteArray &l : lines) {
            if (!l.trimmed().isEmpty()) count++;
        }
        empty = (count == 0);
        return { count, empty };
    } else {
        qWarning("Failed to query trash with gio: exitStatus=%d, exitCode=%d, error=%d",
                 gio.exitStatus(), gio.exitCode(), gio.error());
        // Optionally, you could set count = -1 to indicate error, or keep as is.
        // count = -1;
        // empty = false;
    }
    return { count, empty };
```
</issue_to_address>

### Comment 2
<location> `panels/dock/taskmanager/taskmanager.cpp:452` </location>
<code_context>
+}

-    return tr("%1 files").arg(fileCount);
+std::pair<int, bool> TaskManager::queryTrashCountAndEmpty() const
+{
+    int count = 0;
</code_context>

<issue_to_address>
**issue (complexity):** Consider using Qt's split with SkipEmptyParts and std::ignore in structured bindings to simplify trash counting and avoid unused variables.

Here are two small tweaks that keep the current `gio`‐based approach but shave off the extra loop and the unused-variable noise:

1. In `queryTrashCountAndEmpty()`, split the output once with `Qt::SkipEmptyParts` instead of manually iterating every line.  
2. Replace `Q_UNUSED(empty)` by using `std::ignore` in your structured bindings.

```cpp
std::pair<int,bool> TaskManager::queryTrashCountAndEmpty() const
{
    QProcess gio;
    gio.start("gio", {"trash", "--list"});
    if (gio.waitForFinished(1000) && gio.exitCode() == 0) {
        // one‐liner to split & drop empty lines
        const auto out   = gio.readAllStandardOutput();
        const auto list  = QString::fromUtf8(out)
                               .split('\n', Qt::SkipEmptyParts);
        const int  count = list.size();
        return { count, count == 0 };
    }
    // on error assume empty
    return { 0, true };
}
```

```cpp
QString TaskManager::getTrashTipText()
{
    int count;
    std::tie(count, std::ignore) = queryTrashCountAndEmpty();
    return tr("%1 files").arg(count);
}

bool TaskManager::isTrashEmpty() const
{
    bool empty;
    std::tie(std::ignore, empty) = queryTrashCountAndEmpty();
    return empty;
}
```
</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.

}

return tr("%1 files").arg(fileCount);
std::pair<int, bool> TaskManager::queryTrashCountAndEmpty() const
Copy link

Choose a reason for hiding this comment

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

issue (complexity): Consider using Qt's split with SkipEmptyParts and std::ignore in structured bindings to simplify trash counting and avoid unused variables.

Here are two small tweaks that keep the current gio‐based approach but shave off the extra loop and the unused-variable noise:

  1. In queryTrashCountAndEmpty(), split the output once with Qt::SkipEmptyParts instead of manually iterating every line.
  2. Replace Q_UNUSED(empty) by using std::ignore in your structured bindings.
std::pair<int,bool> TaskManager::queryTrashCountAndEmpty() const
{
    QProcess gio;
    gio.start("gio", {"trash", "--list"});
    if (gio.waitForFinished(1000) && gio.exitCode() == 0) {
        // one‐liner to split & drop empty lines
        const auto out   = gio.readAllStandardOutput();
        const auto list  = QString::fromUtf8(out)
                               .split('\n', Qt::SkipEmptyParts);
        const int  count = list.size();
        return { count, count == 0 };
    }
    // on error assume empty
    return { 0, true };
}
QString TaskManager::getTrashTipText()
{
    int count;
    std::tie(count, std::ignore) = queryTrashCountAndEmpty();
    return tr("%1 files").arg(count);
}

bool TaskManager::isTrashEmpty() const
{
    bool empty;
    std::tie(std::ignore, empty) = queryTrashCountAndEmpty();
    return empty;
}

model: JSON.parse(menus)
delegate: LP.MenuItem {
text: modelData.name
enabled: (root.itemId === "dde-trash" && modelData.id === "clean-trash")
Copy link
Contributor

Choose a reason for hiding this comment

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

当数据为空的时候,能不能不要“清空”这一菜单项?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

单子上面说是要设置为灰色,不能点击,

QProcess gio;
gio.start("gio", QStringList() << "trash" << "--list");
if (gio.waitForFinished(1000) && gio.exitStatus() == QProcess::NormalExit && gio.exitCode() == 0) {
const QByteArray out = gio.readAllStandardOutput();
Copy link
Contributor

Choose a reason for hiding this comment

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

加上const &

1. Add isTrashEmpty() method to detect if trash is empty using gio
command
2. Modify context menu to disable "Clean Trash" option when trash is
empty
3. Update trash tip text generation to use new queryTrashCountAndEmpty
method
4. Add property binding to dynamically enable/disable menu items based
on trash state
5. Use gio trash --list command for more reliable trash content
detection

feat: 实现回收站空状态检测

1. 添加 isTrashEmpty() 方法使用 gio 命令检测回收站是否为空
2. 修改上下文菜单,在回收站为空时禁用"清空回收站"选项
3. 更新回收站提示文本生成以使用新的 queryTrashCountAndEmpty 方法
4. 添加属性绑定以根据回收站状态动态启用/禁用菜单项
5. 使用 gio trash --list 命令进行更可靠的回收站内容检测

PMS: BUG-335901
@deepin-ci-robot
Copy link

deepin pr auto review

我来对这个diff进行审查,主要从语法逻辑、代码质量、性能和安全几个方面来分析:

  1. 语法逻辑:
  • 代码语法正确,符合QML和C++的规范
  • 新增的isTrashEmpty()queryTrashCount()函数逻辑清晰
  • QML中的条件判断表达式语法正确
  1. 代码质量:
  • 优点:
    • 将回收站状态查询逻辑封装成独立函数,提高了代码复用性
    • 使用gio trash --list命令替代了直接文件系统访问,更符合现代Linux桌面环境
  • 改进建议:
    • queryTrashCount()函数中可以考虑添加错误日志,当gio命令执行失败时便于调试
    • queryTrashCount()函数返回值类型建议使用size_t而不是int,因为文件数量不会是负数
  1. 性能考虑:
  • 使用QProcess执行外部命令会有一定开销,但考虑到回收站状态查询不是高频操作,可以接受
  • waitForFinished(1000)设置了1秒超时是合理的,可以避免界面卡顿
  • 建议在queryTrashCount()中添加缓存机制,避免短时间内重复查询
  1. 安全性:
  • 使用gio命令比直接访问文件系统更安全,因为它遵循系统的权限管理
  • 建议在执行外部命令前进行参数检查,虽然当前实现中参数是固定的

具体改进建议:

// 在taskmanager.h中修改返回类型
size_t queryTrashCount() const;

// 在taskmanager.cpp中添加缓存和错误处理
private:
    mutable size_t m_lastTrashCount = 0;
    mutable QDateTime m_lastTrashCheck;

size_t TaskManager::queryTrashCount() const
{
    // 缓存检查,避免1秒内重复查询
    const auto now = QDateTime::currentDateTime();
    if (m_lastTrashCheck.isValid() && m_lastTrashCheck.secsTo(now) < 1) {
        return m_lastTrashCount;
    }

    size_t count = 0;
    QProcess gio;
    gio.start("gio", QStringList() << "trash" << "--list");
    
    if (!gio.waitForFinished(1000)) {
        qWarning() << "Failed to query trash count: timeout";
        return m_lastTrashCount;
    }
    
    if (gio.exitStatus() != QProcess::NormalExit || gio.exitCode() != 0) {
        qWarning() << "Failed to query trash count:" << gio.errorString();
        return m_lastTrashCount;
    }

    const QByteArray &out = gio.readAllStandardOutput();
    const QList<QByteArray> lines = out.split('\n');
    for (const QByteArray &l : lines) {
        if (!l.trimmed().isEmpty()) count++;
    }

    m_lastTrashCount = count;
    m_lastTrashCheck = now;
    return count;
}

这些改进可以:

  1. 提高性能(通过缓存)
  2. 增强错误处理
  3. 使用更合适的返回类型
  4. 提供更好的调试信息

总体来说,这个改动是合理的,主要改进了回收站状态检查的实现方式,使其更可靠和符合现代Linux桌面环境的最佳实践。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

@wjyrich
Copy link
Contributor Author

wjyrich commented Oct 30, 2025

/forcemerge

@deepin-bot
Copy link

deepin-bot bot commented Oct 30, 2025

This pr force merged! (status: blocked)

@deepin-bot deepin-bot bot merged commit c2cf6a1 into linuxdeepin:master Oct 30, 2025
8 of 11 checks passed
@wjyrich wjyrich deleted the fix-bug-335901 branch October 31, 2025 01:35
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