fix: Enhance file monitoring capabilities#361
Merged
deepin-bot[bot] merged 2 commits intolinuxdeepin:masterfrom Aug 13, 2025
Merged
fix: Enhance file monitoring capabilities#361deepin-bot[bot] merged 2 commits intolinuxdeepin:masterfrom
deepin-bot[bot] merged 2 commits intolinuxdeepin:masterfrom
Conversation
Reviewer's GuideThis PR refactors FileInotify to support monitoring of not-yet-existing directories by tracking parent directories and pending targets, refines addWatcher to separate existing vs non-existing paths, enriches logging for monitoring events, and improves cleanup logic; it also simplifies FileInotifyGroup to delegate path validation. Sequence diagram for monitoring non-existing directories via parent watchersequenceDiagram
participant FileInotifyGroup
participant FileInotify
participant QFileSystemWatcher
actor User
User->>FileInotifyGroup: startWatch(paths, album, UID)
FileInotifyGroup->>FileInotify: addWather(paths, album, UID)
FileInotify->>FileInotify: Separate existing/non-existing paths
FileInotify->>QFileSystemWatcher: addPaths(existingPaths)
FileInotify->>FileInotify: For each nonExistingPath
FileInotify->>QFileSystemWatcher: addPath(parentPath)
FileInotify->>FileInotify: Track pendingDirs and parentToChildren
Class diagram for updated FileInotify monitoring logicclassDiagram
class FileInotify {
- bool m_running
- QStringList m_newFile
- QStringList m_deleteFile
- QStringList m_currentDirs
- QStringList m_pendingDirs
- QStringList m_parentDirs
- QMap<QString, QStringList> m_parentToChildren
- QString m_currentAlbum
- int m_currentUID
- QStringList m_Supported
+ void checkNewPath()
+ void checkPendingDirectories(const QString &changedPath)
+ void addParentWatcher(const QString &parentPath, const QString &targetChild)
+ void addWather(const QStringList &paths, const QString &album, int UID)
+ void clear()
}
FileInotify --> QFileSystemWatcher : uses
FileInotify --> QTimer : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @Kakueeen - I've reviewed your changes - here's some feedback:
- In clear(), besides resetting the member lists, explicitly call m_watcher.removePaths on m_currentDirs and m_parentDirs so the QFileSystemWatcher isn’t left monitoring stale directories.
- FileInotifyGroup.startWatch now always creates a FileInotify even if no paths can be monitored—consider re-adding the pre-filter step or an early return when there are no existing or parent-watchable directories to avoid idle watchers.
- The timer intervals (500ms, 1500ms) are hard-coded in several places—extract them into named constants or configuration parameters to make the timing logic clearer and easier to tweak.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In clear(), besides resetting the member lists, explicitly call m_watcher.removePaths on m_currentDirs and m_parentDirs so the QFileSystemWatcher isn’t left monitoring stale directories.
- FileInotifyGroup.startWatch now always creates a FileInotify even if no paths can be monitored—consider re-adding the pre-filter step or an early return when there are no existing or parent-watchable directories to avoid idle watchers.
- The timer intervals (500ms, 1500ms) are hard-coded in several places—extract them into named constants or configuration parameters to make the timing logic clearer and easier to tweak.
## Individual Comments
### Comment 1
<location> `src/src/fileMonitor/fileinotify.cpp:102` </location>
<code_context>
+ QStringList existingPaths;
+ QStringList nonExistingPaths;
+
+ for (const QString &path : paths) {
+ QFileInfo info(path);
+ if (info.exists() && info.isDir()) {
</code_context>
<issue_to_address>
No deduplication of paths before adding to watcher.
Deduplicate input paths to prevent redundant monitoring and improve performance.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
// 分类路径：存在的和不存在的
QStringList existingPaths;
QStringList nonExistingPaths;
for (const QString &path : paths) {
QFileInfo info(path);
if (info.exists() && info.isDir()) {
existingPaths.append(path);
} else {
nonExistingPaths.append(path);
qDebug() << "Path does not exist, will try parent monitoring:" << path;
}
}
=======
// 分类路径：存在的和不存在的
QStringList existingPaths;
QStringList nonExistingPaths;
// Deduplicate input paths
QStringList dedupedPaths = QStringList(paths.toSet().toList());
for (const QString &path : dedupedPaths) {
QFileInfo info(path);
if (info.exists() && info.isDir()) {
existingPaths.append(path);
} else {
nonExistingPaths.append(path);
qDebug() << "Path does not exist, will try parent monitoring:" << path;
}
}
>>>>>>> REPLACE
</suggested_fix>
### Comment 2
<location> `src/src/fileMonitor/fileinotify.cpp:323` </location>
<code_context>
+ for (const QString &foundDir : foundDirs) {
</code_context>
<issue_to_address>
Timer is started for each newly created directory in a loop.
Start the timer once after processing all directories to prevent redundant restarts.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
lzwind
previously approved these changes
Aug 12, 2025
Contributor
|
TAG Bot New tag: 6.0.41 |
- Added functionality to monitor newly created directories and handle pending directories. - Implemented parent directory monitoring for non-existing paths. - Improved logging for directory changes and monitoring status. Log: Enhance file monitoring capabilities Bug: https://pms.uniontech.com/bug-view-327673.html
as title Log: fix bug Bug: https://pms.uniontech.com/bug-view-329071.html
deepin pr auto review代码审查意见：
以上是针对代码审查意见的总结，希望能够对您有所帮助。 |
lzwind
approved these changes
Aug 13, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Kakueeen, lzwind The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Contributor
Author
|
/forcemerge |
Contributor
|
This pr force merged! (status: unstable) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Log: Enhance file monitoring capabilities
Bug: https://pms.uniontech.com/bug-view-327673.html
Summary by Sourcery
Enhance file monitoring by adding support for parent-directory watchers for non-existent paths, auto-detecting and watching newly created subdirectories, and improving logging and state cleanup across the monitoring lifecycle.
New Features:
Enhancements: