fix: add MIME info change detection and reload #322
Merged
Conversation
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
Reviewer's GuideAdds automatic detection and reloading of MIME information when the user’s mimeapps.list changes, exposes a new DBus signal to notify clients, and protects against reload loops while updating related DBus and service plumbing. Sequence diagram for MIME info reload on mimeapps.list changessequenceDiagram
actor ExternalTool
participant FileSystem as QFileSystemWatcher
participant MimeManager as MimeManager1Service
participant DebounceTimer as QTimer_mimeAppsDebounce
participant AppManager as ApplicationManager1Service
participant Client as DBusClient
ExternalTool->>FileSystem: modify ~/.config/mimeapps.list
FileSystem-->>MimeManager: fileChanged(path)
alt path not in watcher.files() and file exists
MimeManager->>FileSystem: addPath(path)
end
alt internalWriteInProgress is true
MimeManager->>MimeManager: set internalWriteInProgress = false
Note over MimeManager: change ignored, no reload
else external change
MimeManager->>DebounceTimer: start(50ms)
end
DebounceTimer-->>MimeManager: timeout()
MimeManager->>MimeManager: handleMimeAppsFileDebounced()
MimeManager->>AppManager: reloadMimeInfos()
AppManager->>MimeManager: m_mimeManager->reset()
AppManager->>AppManager: scanMimeInfos()
AppManager-->>MimeManager: emit MimeInfoChanged
MimeManager-->>Client: DBus signal MimeInfoChanged
Updated class diagram for MIME manager reload and DBus signalclassDiagram
class ApplicationManager1Service {
+void initService(QDBusConnection &connection) noexcept
+void reloadMimeInfos() noexcept
+void scanMimeInfos() noexcept
+QSharedPointer~ApplicationService~ addApplication(DesktopFile desktopFileSource) noexcept
+void removeOneApplication(QString appId) noexcept
+void removeAllApplication() noexcept
}
class MimeManager1Service {
+MimeManager1Service(ApplicationManager1Service *parent)
+~MimeManager1Service()
+void setDefaultApplication(QStringMap defaultApps) noexcept
+void unsetDefaultApplication(QStringList mimeTypes) noexcept
+signal void MimeInfoChanged()
-void onMimeAppsFileChanged(QString path)
-void handleMimeAppsFileDebounced()
-QMimeDatabase m_database
-vector~MimeInfo~ m_infos
-QFileSystemWatcher m_mimeAppsWatcher
-bool m_internalWriteInProgress
-QTimer m_mimeAppsDebounceTimer
}
class MimeManager1Adaptor {
+MimeManager1Adaptor(MimeManager1Service *parent)
+public slots void setDefaultApplication(QStringMap defaultApps)
+public slots void unsetDefaultApplication(QStringList mimeTypes)
+signal void MimeInfoChanged()
}
class QFileSystemWatcher {
+void addPath(QString path)
+QStringList files() const
+signal void fileChanged(QString path)
}
class QTimer {
+void setSingleShot(bool singleShot)
+void setInterval(int msec)
+void start()
+signal void timeout()
}
ApplicationManager1Service --> MimeManager1Service : owns m_mimeManager
MimeManager1Service ..> ApplicationManager1Service : parent()
MimeManager1Service --> QFileSystemWatcher : uses m_mimeAppsWatcher
MimeManager1Service --> QTimer : uses m_mimeAppsDebounceTimer
MimeManager1Adaptor ..> MimeManager1Service : adapts to DBus
MimeManager1Adaptor ..> MimeManager1Service : forwards MimeInfoChanged signal
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 - I've found 3 issues, and left some high level feedback:
- The
m_internalWriteInProgressflag is only reset on write failure or when a file change event arrives, so ifwriteToFile()succeeds but the watcher never emitsfileChanged(e.g. no actual on-disk change), the flag may stay true and cause the next external change to be skipped; consider resetting it explicitly after a successful reload or with a timeout. - The watcher currently adds
mimeapps.listonly if it exists at construction time and onfileChanged, so if the file is created after startup it will never be watched; consider also watching the config directory (or re-checking periodically) so that a newly createdmimeapps.listis picked up.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `m_internalWriteInProgress` flag is only reset on write failure or when a file change event arrives, so if `writeToFile()` succeeds but the watcher never emits `fileChanged` (e.g. no actual on-disk change), the flag may stay true and cause the next external change to be skipped; consider resetting it explicitly after a successful reload or with a timeout.
- The watcher currently adds `mimeapps.list` only if it exists at construction time and on `fileChanged`, so if the file is created after startup it will never be watched; consider also watching the config directory (or re-checking periodically) so that a newly created `mimeapps.list` is picked up.
## Individual Comments
### Comment 1
<location> `src/dbus/applicationmanager1service.cpp:282` </location>
<code_context>
+ qInfo() << "Reloading MIME info due to external configuration change.";
+ m_mimeManager->reset();
+ scanMimeInfos();
+ emit m_mimeManager->MimeInfoChanged();
+}
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Emitting another object's signal directly from here likely violates C++ access control and may not compile
Because Q_SIGNALS makes MimeInfoChanged() a protected member, it can only be called from within MimeManager1Service or its friends/derived classes. ApplicationManager1Service cannot legally call m_mimeManager->MimeInfoChanged() directly. Instead, consider adding a public method on MimeManager1Service (e.g., notifyMimeInfoChanged()) that emits the signal internally, or emit a signal on ApplicationManager1Service and connect it to MimeManager1Service::MimeInfoChanged().
</issue_to_address>
### Comment 2
<location> `src/dbus/mimemanager1service.cpp:181-182` </location>
<code_context>
+
+void MimeManager1Service::onMimeAppsFileChanged(const QString &path)
+{
+ if (!m_mimeAppsWatcher.files().contains(path) && QFileInfo::exists(path)) {
+ m_mimeAppsWatcher.addPath(path);
+ }
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** File-based watching may miss changes when mimeapps.list is deleted and later recreated
QFileSystemWatcher will stop watching the path once mimeapps.list is removed, but your handler only re-adds it if QFileInfo::exists(path) is true at notification time. If the file is deleted and later recreated, you may never reattach the watcher and will miss subsequent changes. Consider also watching the parent directory and re-adding the file when it is (re)created, or otherwise handling the case where the file doesn’t exist at the moment of the change signal.
Suggested implementation:
```cpp
void MimeManager1Service::onMimeAppsFileChanged(const QString &path)
{
const QString userMimeAppsFile = getXDGConfigHome() + "/mimeapps.list";
// Ensure we (re)attach a file watcher whenever mimeapps.list exists but is not watched.
// QFileSystemWatcher stops watching a file once it is removed, so this covers the
// "delete + recreate" case when combined with a directory watcher on the parent dir.
if (!m_mimeAppsWatcher.files().contains(userMimeAppsFile) && QFileInfo::exists(userMimeAppsFile)) {
m_mimeAppsWatcher.addPath(userMimeAppsFile);
}
// Debounce downstream handling of mimeapps.list changes
m_mimeAppsDebounceTimer.setSingleShot(true);
if (!m_mimeAppsDebounceTimer.isActive()) {
m_mimeAppsDebounceTimer.start();
}
}
```
To fully implement the suggestion and cover the delete + recreate scenario, you should also:
1. **Watch the parent directory (likely in the MimeManager1Service constructor or initialization code):**
- Determine the XDG config directory (same `getXDGConfigHome()` you use for the file).
- Add it to the watcher if not already present:
```cpp
const QString userConfigDir = getXDGConfigHome();
if (!m_mimeAppsWatcher.directories().contains(userConfigDir) && QFileInfo::exists(userConfigDir)) {
m_mimeAppsWatcher.addPath(userConfigDir);
}
```
- Connect the `directoryChanged` signal to a new slot:
```cpp
connect(&m_mimeAppsWatcher, &QFileSystemWatcher::directoryChanged,
this, &MimeManager1Service::onMimeAppsDirectoryChanged);
```
2. **Implement the `onMimeAppsDirectoryChanged` slot to re-attach the file watcher when mimeapps.list is (re)created:**
```cpp
void MimeManager1Service::onMimeAppsDirectoryChanged(const QString &dirPath)
{
const QString userConfigDir = getXDGConfigHome();
if (dirPath != userConfigDir) {
return;
}
const QString userMimeAppsFile = userConfigDir + "/mimeapps.list";
// If the file now exists but isn't being watched, re-add it.
if (QFileInfo::exists(userMimeAppsFile) &&
!m_mimeAppsWatcher.files().contains(userMimeAppsFile)) {
m_mimeAppsWatcher.addPath(userMimeAppsFile);
// Optionally treat this as a "file changed" event so existing logic runs:
onMimeAppsFileChanged(userMimeAppsFile);
}
}
```
3. **Ensure the declaration of `onMimeAppsDirectoryChanged` is added to the corresponding header (`mimemanager1service.h`), e.g.:**
```cpp
private slots:
void onMimeAppsFileChanged(const QString &path);
void onMimeAppsDirectoryChanged(const QString &dirPath);
```
With these changes, the parent directory is always watched, and the file watcher is automatically reattached when `mimeapps.list` is deleted and later recreated.
</issue_to_address>
### Comment 3
<location> `src/dbus/mimemanager1service.cpp:28` </location>
<code_context>
+ m_mimeAppsWatcher.addPath(userMimeAppsFile);
+ }
+
+ m_mimeAppsDebounceTimer.setSingleShot(true);
+ m_mimeAppsDebounceTimer.setInterval(50);
+ connect(&m_mimeAppsDebounceTimer, &QTimer::timeout, this, &MimeManager1Service::handleMimeAppsFileDebounced);
</code_context>
<issue_to_address>
**issue (complexity):** Consider replacing the shared `m_internalWriteInProgress` flag with scoped `QSignalBlocker` usage and an optional helper for debounce to localize state and simplify control flow.
You can simplify the control flow and remove the fragile `m_internalWriteInProgress` state entirely by scoping the suppression of `fileChanged` using `QSignalBlocker` on the watcher. That keeps the “ignore internal writes” behaviour but makes it local and non‑leaky.
### 1. Replace `m_internalWriteInProgress` with a scoped signal blocker
```cpp
#include <QSignalBlocker>
```
```cpp
void MimeManager1Service::setDefaultApplication(const ObjectMap &defaultApps) noexcept
{
auto &app = m_infos.front().appsList();
auto userConfig = std::find_if(app.begin(), app.end(), [](const MimeApps &config) {
return !config.isDesktopSpecific();
});
if (userConfig == app.end()) {
qWarning() << "couldn't find user mimeApps";
safe_sendErrorReply(QDBusError::InternalError);
return;
}
// Scoped suppression of QFileSystemWatcher notifications
QSignalBlocker watcherBlocker(&m_mimeAppsWatcher);
for (auto it = defaultApps.constKeyValueBegin(); it != defaultApps.constKeyValueEnd(); ++it) {
userConfig->setDefaultApplication(it->first, it->second);
}
if (!userConfig->writeToFile()) {
safe_sendErrorReply(QDBusError::Failed,
"set default app failed, these config will be reset after re-login.");
// no extra state to reset; watcherBlocker destructor re‑enables signals
return;
}
}
```
```cpp
void MimeManager1Service::unsetDefaultApplication(const QStringList &mimeTypes) noexcept
{
auto &app = m_infos.front().appsList();
auto userConfig = std::find_if(app.begin(), app.end(), [](const MimeApps &config) {
return !config.isDesktopSpecific();
});
if (userConfig == app.end()) {
qWarning() << "couldn't find user mimeApps";
safe_sendErrorReply(QDBusError::InternalError);
return;
}
QSignalBlocker watcherBlocker(&m_mimeAppsWatcher);
for (const auto &mime : mimeTypes) {
userConfig->unsetDefaultApplication(mime);
}
if (!userConfig->writeToFile()) {
safe_sendErrorReply(QDBusError::Failed,
"unset default app failed, these config will be reset after re-login.");
return;
}
}
```
Then `onMimeAppsFileChanged` no longer needs to know about internal write state:
```cpp
void MimeManager1Service::onMimeAppsFileChanged(const QString &path)
{
if (!m_mimeAppsWatcher.files().contains(path) && QFileInfo::exists(path)) {
m_mimeAppsWatcher.addPath(path);
}
m_mimeAppsDebounceTimer.start();
}
```
This:
- Keeps the feature (watching and reloading `mimeapps.list`).
- Ensures internal writes never trigger reloads.
- Removes the cross‑method lifecycle of `m_internalWriteInProgress` and its hidden coupling.
### 2. Optional: isolate debounce logic into a helper
To reduce the cognitive load further, you can hide the timer in a small helper and keep `onMimeAppsFileChanged` high‑level:
```cpp
void MimeManager1Service::scheduleMimeAppsReload()
{
m_mimeAppsDebounceTimer.start();
}
void MimeManager1Service::onMimeAppsFileChanged(const QString &path)
{
if (!m_mimeAppsWatcher.files().contains(path) && QFileInfo::exists(path)) {
m_mimeAppsWatcher.addPath(path);
}
scheduleMimeAppsReload();
}
```
This keeps all existing behaviour but makes the state management explicit, scoped, and easier to reason about.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| qInfo() << "Reloading MIME info due to external configuration change."; | ||
| m_mimeManager->reset(); | ||
| scanMimeInfos(); | ||
| emit m_mimeManager->MimeInfoChanged(); |
There was a problem hiding this comment.
issue (bug_risk): Emitting another object's signal directly from here likely violates C++ access control and may not compile
Because Q_SIGNALS makes MimeInfoChanged() a protected member, it can only be called from within MimeManager1Service or its friends/derived classes. ApplicationManager1Service cannot legally call m_mimeManager->MimeInfoChanged() directly. Instead, consider adding a public method on MimeManager1Service (e.g., notifyMimeInfoChanged()) that emits the signal internally, or emit a signal on ApplicationManager1Service and connect it to MimeManager1Service::MimeInfoChanged().
Comment on lines
+181
to
+184
| if (!m_mimeAppsWatcher.files().contains(path) && QFileInfo::exists(path)) { | ||
| m_mimeAppsWatcher.addPath(path); |
There was a problem hiding this comment.
suggestion (bug_risk): File-based watching may miss changes when mimeapps.list is deleted and later recreated
QFileSystemWatcher will stop watching the path once mimeapps.list is removed, but your handler only re-adds it if QFileInfo::exists(path) is true at notification time. If the file is deleted and later recreated, you may never reattach the watcher and will miss subsequent changes. Consider also watching the parent directory and re-adding the file when it is (re)created, or otherwise handling the case where the file doesn’t exist at the moment of the change signal.
Suggested implementation:
void MimeManager1Service::onMimeAppsFileChanged(const QString &path)
{
const QString userMimeAppsFile = getXDGConfigHome() + "/mimeapps.list";
// Ensure we (re)attach a file watcher whenever mimeapps.list exists but is not watched.
// QFileSystemWatcher stops watching a file once it is removed, so this covers the
// "delete + recreate" case when combined with a directory watcher on the parent dir.
if (!m_mimeAppsWatcher.files().contains(userMimeAppsFile) && QFileInfo::exists(userMimeAppsFile)) {
m_mimeAppsWatcher.addPath(userMimeAppsFile);
}
// Debounce downstream handling of mimeapps.list changes
m_mimeAppsDebounceTimer.setSingleShot(true);
if (!m_mimeAppsDebounceTimer.isActive()) {
m_mimeAppsDebounceTimer.start();
}
}
To fully implement the suggestion and cover the delete + recreate scenario, you should also:
-
Watch the parent directory (likely in the MimeManager1Service constructor or initialization code):
- Determine the XDG config directory (same
getXDGConfigHome()you use for the file). - Add it to the watcher if not already present:
const QString userConfigDir = getXDGConfigHome(); if (!m_mimeAppsWatcher.directories().contains(userConfigDir) && QFileInfo::exists(userConfigDir)) { m_mimeAppsWatcher.addPath(userConfigDir); }
- Connect the
directoryChangedsignal to a new slot:connect(&m_mimeAppsWatcher, &QFileSystemWatcher::directoryChanged, this, &MimeManager1Service::onMimeAppsDirectoryChanged);
- Determine the XDG config directory (same
-
Implement the
onMimeAppsDirectoryChangedslot to re-attach the file watcher when mimeapps.list is (re)created:void MimeManager1Service::onMimeAppsDirectoryChanged(const QString &dirPath) { const QString userConfigDir = getXDGConfigHome(); if (dirPath != userConfigDir) { return; } const QString userMimeAppsFile = userConfigDir + "/mimeapps.list"; // If the file now exists but isn't being watched, re-add it. if (QFileInfo::exists(userMimeAppsFile) && !m_mimeAppsWatcher.files().contains(userMimeAppsFile)) { m_mimeAppsWatcher.addPath(userMimeAppsFile); // Optionally treat this as a "file changed" event so existing logic runs: onMimeAppsFileChanged(userMimeAppsFile); } }
-
Ensure the declaration of
onMimeAppsDirectoryChangedis added to the corresponding header (mimemanager1service.h), e.g.:private slots: void onMimeAppsFileChanged(const QString &path); void onMimeAppsDirectoryChanged(const QString &dirPath);
With these changes, the parent directory is always watched, and the file watcher is automatically reattached when mimeapps.list is deleted and later recreated.
| m_mimeAppsWatcher.addPath(userMimeAppsFile); | ||
| } | ||
|
|
||
| m_mimeAppsDebounceTimer.setSingleShot(true); |
There was a problem hiding this comment.
issue (complexity): Consider replacing the shared m_internalWriteInProgress flag with scoped QSignalBlocker usage and an optional helper for debounce to localize state and simplify control flow.
You can simplify the control flow and remove the fragile m_internalWriteInProgress state entirely by scoping the suppression of fileChanged using QSignalBlocker on the watcher. That keeps the “ignore internal writes” behaviour but makes it local and non‑leaky.
1. Replace m_internalWriteInProgress with a scoped signal blocker
#include <QSignalBlocker>void MimeManager1Service::setDefaultApplication(const ObjectMap &defaultApps) noexcept
{
auto &app = m_infos.front().appsList();
auto userConfig = std::find_if(app.begin(), app.end(), [](const MimeApps &config) {
return !config.isDesktopSpecific();
});
if (userConfig == app.end()) {
qWarning() << "couldn't find user mimeApps";
safe_sendErrorReply(QDBusError::InternalError);
return;
}
// Scoped suppression of QFileSystemWatcher notifications
QSignalBlocker watcherBlocker(&m_mimeAppsWatcher);
for (auto it = defaultApps.constKeyValueBegin(); it != defaultApps.constKeyValueEnd(); ++it) {
userConfig->setDefaultApplication(it->first, it->second);
}
if (!userConfig->writeToFile()) {
safe_sendErrorReply(QDBusError::Failed,
"set default app failed, these config will be reset after re-login.");
// no extra state to reset; watcherBlocker destructor re‑enables signals
return;
}
}void MimeManager1Service::unsetDefaultApplication(const QStringList &mimeTypes) noexcept
{
auto &app = m_infos.front().appsList();
auto userConfig = std::find_if(app.begin(), app.end(), [](const MimeApps &config) {
return !config.isDesktopSpecific();
});
if (userConfig == app.end()) {
qWarning() << "couldn't find user mimeApps";
safe_sendErrorReply(QDBusError::InternalError);
return;
}
QSignalBlocker watcherBlocker(&m_mimeAppsWatcher);
for (const auto &mime : mimeTypes) {
userConfig->unsetDefaultApplication(mime);
}
if (!userConfig->writeToFile()) {
safe_sendErrorReply(QDBusError::Failed,
"unset default app failed, these config will be reset after re-login.");
return;
}
}Then onMimeAppsFileChanged no longer needs to know about internal write state:
void MimeManager1Service::onMimeAppsFileChanged(const QString &path)
{
if (!m_mimeAppsWatcher.files().contains(path) && QFileInfo::exists(path)) {
m_mimeAppsWatcher.addPath(path);
}
m_mimeAppsDebounceTimer.start();
}This:
- Keeps the feature (watching and reloading
mimeapps.list). - Ensures internal writes never trigger reloads.
- Removes the cross‑method lifecycle of
m_internalWriteInProgressand its hidden coupling.
2. Optional: isolate debounce logic into a helper
To reduce the cognitive load further, you can hide the timer in a small helper and keep onMimeAppsFileChanged high‑level:
void MimeManager1Service::scheduleMimeAppsReload()
{
m_mimeAppsDebounceTimer.start();
}
void MimeManager1Service::onMimeAppsFileChanged(const QString &path)
{
if (!m_mimeAppsWatcher.files().contains(path) && QFileInfo::exists(path)) {
m_mimeAppsWatcher.addPath(path);
}
scheduleMimeAppsReload();
}This keeps all existing behaviour but makes the state management explicit, scoped, and easier to reason about.
BLumia
reviewed
Jan 22, 2026
| " <arg direction=\"out\" type=\"a{oa{sa{sv}}}\" name=\"applications_and_properties\"/>\n" | ||
| " <annotation value=\"ObjectMap\" name=\"org.qtproject.QtDBus.QtTypeName.Out0\"/>\n" | ||
| " </method>\n" | ||
| " <signal name=\"MimeInfoChanged\">\n" |
Member
There was a problem hiding this comment.
这个信号名字是不是有可能有误导，有可能文件内容实际并未实际有变化，只是因为filewatcher监视到了文件变化导致单纯的重载入了一次mime信息？
Contributor
There was a problem hiding this comment.
这个信号名字是不是有可能有误导，有可能文件内容实际并未实际有变化，只是因为filewatcher监视到了文件变化导致单纯的重载入了一次mime信息？
我们需要暴露出去的应该是mimeInfo变化的信号吧，监视文件变化只是我们实现的方式，如果真的出现内容未变化，而触发了信号，那也是我们后面需要修复的吧，
| qInfo() << "Reloading MIME info due to external configuration change."; | ||
| m_mimeManager->reset(); | ||
| scanMimeInfos(); | ||
| emit m_mimeManager->MimeInfoChanged(); |
Member
There was a problem hiding this comment.
同其他备注，重载就一定有变化吗？以及是否把这个信号移动到 handleMimeAppsFileDebounced() 调用 reloadMimeInfos() 之后更合适？
|
|
||
| void ApplicationManager1Service::reloadMimeInfos() noexcept | ||
| { | ||
| qInfo() << "Reloading MIME info due to external configuration change."; |
Member
There was a problem hiding this comment.
这个注释是不是应该写到 handleMimeAppsFileDebounced() 调用 reloadMimeInfos() 的位置？
18202781743
reviewed
Jan 22, 2026
| // 添加用户配置目录下的 mimeapps.list 文件到监控 | ||
| QString userMimeAppsFile = getXDGConfigHome() + "/mimeapps.list"; | ||
| if (QFileInfo::exists(userMimeAppsFile)) { | ||
| m_mimeAppsWatcher.addPath(userMimeAppsFile); |
Contributor
There was a problem hiding this comment.
必须要存在么？这个文件会不会第一次在am起来后才创建的？
另外，如果文件不存在，要不要提示个信息，那个定时器也不用刷了吧，
|
|
||
| if (!userConfig->writeToFile()) { | ||
| safe_sendErrorReply(QDBusError::Failed, "set default app failed, these config will be reset after re-login."); | ||
| m_internalWriteInProgress = false; |
Contributor
There was a problem hiding this comment.
有定时器，是不是不需要这个m_internalWriteInProgress这个flag了，
There was a problem hiding this comment.
Pull request overview
This pull request adds automatic MIME information reload capability when the user's mimeapps.list configuration file is modified externally. The implementation uses QFileSystemWatcher to monitor the file, implements a debounce mechanism to handle rapid changes, and includes an internal write flag to prevent reload loops from the application's own modifications.
Changes:
- Added MimeInfoChanged DBus signal to notify clients when MIME configuration is updated
- Implemented file system monitoring with debouncing (50ms) for the user's mimeapps.list file
- Added internal write protection mechanism to distinguish between internal and external file modifications
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| api/dbus/org.desktopspec.MimeManager1.xml | Added MimeInfoChanged signal definition to the DBus interface specification |
| toolGenerate/qdbusxml2cpp/org.desktopspec.MimeManager1Adaptor.h | Generated DBus adaptor header with MimeInfoChanged signal |
| src/dbus/mimemanager1service.h | Added file watcher, debounce timer, internal write flag, and signal/slot declarations |
| src/dbus/mimemanager1service.cpp | Implemented file monitoring logic, debouncing, and internal write protection |
| src/dbus/applicationmanager1service.h | Added reloadMimeInfos public method |
| src/dbus/applicationmanager1service.cpp | Implemented reloadMimeInfos to reset and rescan MIME info, emitting change signal |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+122
to
133
| m_internalWriteInProgress = true; | ||
| for (auto it = defaultApps.constKeyValueBegin(); it != defaultApps.constKeyValueEnd(); ++it) { | ||
| userConfig->setDefaultApplication(it->first, it->second); | ||
| } | ||
|
|
||
| if (!userConfig->writeToFile()) { | ||
| safe_sendErrorReply(QDBusError::Failed, "set default app failed, these config will be reset after re-login."); | ||
| m_internalWriteInProgress = false; | ||
| return; | ||
| } |
There was a problem hiding this comment.
The internal write flag is not reset after writeToFile succeeds. While the design expects the file system watcher to consume this flag in onMimeAppsFileChanged, this creates a reliability issue. If the file change event doesn't fire for any reason (e.g., file system watcher malfunction, write doesn't trigger change event, or timing issues), the flag will remain true indefinitely, blocking all future external change detection. Consider resetting the flag using a timer-based fallback or RAII pattern to ensure it's always cleared, even if the file change event doesn't arrive.
Comment on lines
+145
to
156
| m_internalWriteInProgress = true; | ||
| for (const auto &mime : mimeTypes) { | ||
| userConfig->unsetDefaultApplication(mime); | ||
| } | ||
|
|
||
| if (!userConfig->writeToFile()) { | ||
| safe_sendErrorReply(QDBusError::Failed, "unset default app failed, these config will be reset after re-login."); | ||
| m_internalWriteInProgress = false; | ||
| return; | ||
| } |
There was a problem hiding this comment.
The internal write flag is not reset after writeToFile succeeds. While the design expects the file system watcher to consume this flag in onMimeAppsFileChanged, this creates a reliability issue. If the file change event doesn't fire for any reason (e.g., file system watcher malfunction, write doesn't trigger change event, or timing issues), the flag will remain true indefinitely, blocking all future external change detection. Consider resetting the flag using a timer-based fallback or RAII pattern to ensure it's always cleared, even if the file change event doesn't arrive.
Comment on lines
22
to
29
| // 添加用户配置目录下的 mimeapps.list 文件到监控 | ||
| QString userMimeAppsFile = getXDGConfigHome() + "/mimeapps.list"; | ||
| if (QFileInfo::exists(userMimeAppsFile)) { | ||
| m_mimeAppsWatcher.addPath(userMimeAppsFile); | ||
| } | ||
|
|
There was a problem hiding this comment.
The file watcher is set up in the constructor before scanMimeInfos creates the user mimeapps.list file if it doesn't exist. If the file doesn't exist at construction time, it won't be added to the watcher. Even though there's re-add logic in onMimeAppsFileChanged at lines 181-183, it won't help because that handler is never called for unwatched files. Consider moving the file watcher setup to a separate initialization method that's called after scanMimeInfos, or add a mechanism to check and add the file to the watcher after scanMimeInfos completes.
Suggested change
| // 添加用户配置目录下的 mimeapps.list 文件到监控 | |
| QString userMimeAppsFile = getXDGConfigHome() + "/mimeapps.list"; | |
| if (QFileInfo::exists(userMimeAppsFile)) { | |
| m_mimeAppsWatcher.addPath(userMimeAppsFile); | |
| } | |
| // 添加用户配置目录下的 mimeapps.list 文件到监控（如果已存在） | |
| const QString userMimeAppsFile = getXDGConfigHome() + "/mimeapps.list"; | |
| if (QFileInfo::exists(userMimeAppsFile)) { | |
| m_mimeAppsWatcher.addPath(userMimeAppsFile); | |
| } | |
| // 同时监控用户配置目录本身，以便在 mimeapps.list 创建后将其加入监控 | |
| const QString userConfigDir = getXDGConfigHome(); | |
| if (!m_mimeAppsWatcher.directories().contains(userConfigDir)) { | |
| m_mimeAppsWatcher.addPath(userConfigDir); | |
| } | |
| connect(&m_mimeAppsWatcher, &QFileSystemWatcher::directoryChanged, this, | |
| [this, userConfigDir](const QString &path) { | |
| if (path != userConfigDir) { | |
| return; | |
| } | |
| const QString userMimeAppsFile = userConfigDir + "/mimeapps.list"; | |
| if (QFileInfo::exists(userMimeAppsFile) | |
| && !m_mimeAppsWatcher.files().contains(userMimeAppsFile)) { | |
| m_mimeAppsWatcher.addPath(userMimeAppsFile); | |
| } | |
| }); |
18202781743
reviewed
Jan 22, 2026
| qInfo() << "Reloading MIME info due to external configuration change."; | ||
| m_mimeManager->reset(); | ||
| scanMimeInfos(); | ||
| emit m_mimeManager->MimeInfoChanged(); |
Contributor
There was a problem hiding this comment.
信号一般是类自己发的，给别人监听用的，
ApplicationManager1Service::reloadMimeInfos这个信号应该不需要吧，直接在handleMimeAppsFileDebounced里做了，
18202781743
reviewed
Jan 22, 2026
xionglinlin
force-pushed
the
master
branch
2 times, most recently
from
3d3beb0 to
5bf1ed7
Compare
18202781743
reviewed
Jan 22, 2026
|
|
||
| if (!userConfig->writeToFile()) { | ||
| safe_sendErrorReply(QDBusError::Failed, "set default app failed, these config will be reset after re-login."); | ||
| m_internalWriteInProgress = false; |
Contributor
There was a problem hiding this comment.
m_internalWriteInProgress这个是不是在写完就应该重置为false？而不是非要失败才重置为false，
18202781743
reviewed
Jan 22, 2026
|
|
||
| // 如果是内部写入导致的文件变化，忽略 | ||
| if (m_internalWriteInProgress) { | ||
| m_internalWriteInProgress = false; |
18202781743
approved these changes
Jan 22, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, xionglinlin 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 |
Added file system monitoring for user mimeapps.list configuration file to detect external changes and reload MIME information automatically. The fix includes a new DBus signal to notify clients when MIME info changes, debounced file change handling, and protection against reload loops from internal writes. 1. Added MimeInfoChanged signal to DBus interface to notify clients when MIME information is updated 2. Implemented QFileSystemWatcher to monitor user's mimeapps.list file for external modifications 3. Added debounce timer (50ms) to handle rapid file change events and avoid multiple reloads 4. Added internal write flag to prevent reload loops when changes originate from the application itself 5. Created reloadMimeInfos() method to reset MIME manager and rescan all MIME information 6. Updated copyright years from 2023 to 2026 across modified files Log: Added automatic reload of MIME information when user configuration files are externally modified Influence: 1. Test that external modifications to ~/.config/mimeapps.list trigger MIME info reload 2. Verify that internal writes (setDefaultApplication/ unsetDefaultApplication) don't cause reload loops 3. Test that MimeInfoChanged signal is emitted when configuration changes 4. Verify debounce mechanism works correctly with rapid file changes 5. Test that MIME cache is properly updated after reload 6. Verify default application settings persist correctly after reload fix: 添加MIME信息变更检测和重新加载功能 添加了对用户mimeapps.list配置文件的文件系统监控，以检测外部更改并自动重 新加载MIME信息。该修复包括一个新的DBus信号用于在MIME信息变更时通知客户 端、防抖的文件变更处理以及防止内部写入导致的重载循环。 1. 在DBus接口中添加MimeInfoChanged信号，用于在MIME信息更新时通知客户端 2. 实现QFileSystemWatcher来监控用户的mimeapps.list文件的外部修改 3. 添加防抖定时器（50毫秒）以处理快速的文件变更事件，避免多次重新加载 4. 添加内部写入标志，防止应用程序自身发起更改时产生重载循环 5. 创建reloadMimeInfos()方法用于重置MIME管理器并重新扫描所有MIME信息 6. 将修改文件的版权年份从2023更新到2026 Log: 新增用户配置文件被外部修改时自动重新加载MIME信息的功能 Influence: 1. 测试外部修改~/.config/mimeapps.list文件是否会触发MIME信息重新加载 2. 验证内部写入（setDefaultApplication/unsetDefaultApplication）不会导致 重载循环 3. 测试配置变更时MimeInfoChanged信号是否正确发射 4. 验证防抖机制在快速文件变更时工作正常 5. 测试重新加载后MIME缓存是否正确更新 6. 验证默认应用程序设置在重新加载后是否正确保持 PMS: BUG-288389
deepin pr auto review这份代码实现了对 MIME 类型配置文件的监控和自动重载功能，整体设计合理，使用了防抖（debounce）机制和内部写入标志来避免重复触发和循环更新。以下是对代码的详细审查和改进建议： 1. 语法逻辑问题1：文件监控的健壮性在 改进建议： void MimeManager1Service::onMimeAppsFileChanged(const QString &path)
{
// 检查文件是否存在，如果不存在则等待文件创建
if (!QFileInfo::exists(path)) {
qWarning() << "MimeApps file removed, waiting for recreation:" << path;
return;
}
// 确保文件仍在监控列表中
if (!m_mimeAppsWatcher.files().contains(path)) {
if (!m_mimeAppsWatcher.addPath(path)) {
qWarning() << "Failed to re-add path to watcher:" << path;
}
}
// 如果是内部写入导致的文件变化，忽略
if (m_internalWriteInProgress) {
m_internalWriteInProgress = false;
return;
}
m_mimeAppsDebounceTimer.start();
}问题2：
|
Contributor
Author
|
/forcemerge |
|
This pr force merged! (status: blocked) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
Added file system monitoring for user mimeapps.list configuration file to detect external changes and reload MIME information automatically. The fix includes a new DBus signal to notify clients when MIME info changes, debounced file change handling, and protection against reload loops from internal writes.
Log: Added automatic reload of MIME information when user configuration files are externally modified
Influence:
fix: 添加MIME信息变更检测和重新加载功能
添加了对用户mimeapps.list配置文件的文件系统监控，以检测外部更改并自动重
新加载MIME信息。该修复包括一个新的DBus信号用于在MIME信息变更时通知客户
端、防抖的文件变更处理以及防止内部写入导致的重载循环。
Log: 新增用户配置文件被外部修改时自动重新加载MIME信息的功能
Influence:
PMS: BUG-288389
Summary by Sourcery
Add automatic reload of MIME information when the user MIME configuration changes and notify clients via a new DBus signal.
New Features:
Enhancements: