fix: Fixed custom mode exceptions #589
Conversation
Fixed custom mode exceptions Log: Fixed custom mode exceptions
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @pengfeixx - I've reviewed your changes - here's some feedback:
Overall Comments:
- The repeated QEventLoop and QFileSystemWatcher setup in
handleSettingsandcloseEventshould be extracted into a separate function. - Consider using an enum instead of integers for
set.start.crashto improve readability and maintainability.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| @@ -2877,6 +2877,12 @@ void MainWindow::handleSettings(DSettingsDialog *dsd) | |||
| dsd ->show(); | |||
| #endif | |||
|
|
|||
| static QEventLoop loop; | |||
There was a problem hiding this comment.
issue (complexity): Consider refactoring the blocking "wait-for file change" pattern into a single helper that uses asynchronous signals instead of nested QEventLoop calls to reduce complexity.
Consider refactoring the blocking “wait‐for file change” pattern into a single helper that uses asynchronous signals instead of nested QEventLoop calls. For example, define a helper that wraps a QFileSystemWatcher and emits a signal when a file changes:
// FileWatcherHelper.h
class FileWatcherHelper : public QObject {
Q_OBJECT
public:
explicit FileWatcherHelper(const QString &path, QObject *parent = nullptr)
: QObject(parent), m_fileWatcher(new QFileSystemWatcher(this)) {
m_fileWatcher->addPath(path);
connect(m_fileWatcher, &QFileSystemWatcher::fileChanged, this, &FileWatcherHelper::fileUpdated);
}
signals:
void fileUpdated();
private:
QFileSystemWatcher *m_fileWatcher;
};Then replace the repeated static QEventLoop and QFileSystemWatcher blocks with an asynchronous slot:
// Instead of blocking using loop.exec(), use a slot or lambda
auto watcher = new FileWatcherHelper(Settings::get().configPath(), this);
connect(watcher, &FileWatcherHelper::fileUpdated, this, [this, watcher]() {
// Execute the logic that should run after file update
Settings::get().setInternalOption("global_volume", m_nDisplayVolume > 100 ? 100 : m_nDisplayVolume);
Settings::get().onSetCrash();
m_pEngine->savePlaybackPosition();
// Clean up watcher if no longer needed:
watcher->deleteLater();
});This approach removes nested loops while preserving the original functionality. Adjust other identical patterns similarly to reduce complexity and improve maintainability.
deepin pr auto review代码审查意见：
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: pengfeixx, rb-union 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 |
Fixed custom mode exceptions
Log: Fixed custom mode exceptions
Summary by Sourcery
Fixes custom mode exceptions by adjusting the logic for handling crash recovery and decode mode initialization. It ensures that settings are properly applied and persisted, and prevents potential issues during application startup and settings changes.