add configuration edit, export, and import #444
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
Co-authored-by: KasparMetsa <kasparme@gmail.com>
Reviewer's GuideImplements configuration edit/export/import workflows using QSettings and temporary files, refactors Configuration read/write to support external QSettings, wires UI signals to reload config after changes, slightly adjusts help/commands, and improves SSL error handling. Sequence diagram for config edit console commandsequenceDiagram
actor User
participant AbstractParser
participant QTemporaryFile
participant QSettings
participant QFile
participant RemoteEditWidget
participant Configuration
User->>AbstractParser: config edit
AbstractParser->>AbstractParser: isConnected check
alt user is connected
AbstractParser-->>User: error You must disconnect
else user is disconnected
AbstractParser-->>User: Opening configuration editor
AbstractParser->>QTemporaryFile: create temp ini file
QTemporaryFile-->>AbstractParser: fileName
AbstractParser->>QSettings: writeTo using getConfig
AbstractParser->>QFile: open temp ini for read
QFile-->>AbstractParser: content
AbstractParser->>QFile: remove temp file
alt content empty
AbstractParser-->>User: Configuration is empty or failed to export
else content ok
AbstractParser->>RemoteEditWidget: new RemoteEditWidget(true, title, content)
AbstractParser->>RemoteEditWidget: connect sig_save to lambda
RemoteEditWidget->>User: show editor window
User->>RemoteEditWidget: edits and saves
RemoteEditWidget-->>AbstractParser: sig_save(edited)
AbstractParser->>QTemporaryFile: create tempRead ini file
AbstractParser->>QTemporaryFile: write edited content
AbstractParser->>QSettings: cfg.readFrom(tempRead)
AbstractParser->>Configuration: write
AbstractParser-->>User: Configuration imported and persisted
AbstractParser-->>User: OK response
end
end
Sequence diagram for preferences configuration export and importsequenceDiagram
actor User
participant GeneralPage
participant QTemporaryFile
participant QSettings
participant QFile
participant QFileDialog
participant Configuration
participant ConfigDialog
participant OtherPages
rect rgb(230,230,250)
note over User,GeneralPage: Export workflow
User->>GeneralPage: clicks configurationExportButton
GeneralPage->>QTemporaryFile: create temp ini
QTemporaryFile-->>GeneralPage: fileName
GeneralPage->>QSettings: getConfig.writeTo
GeneralPage->>QFile: open temp for read
QFile-->>GeneralPage: content
GeneralPage->>QFileDialog: saveFileContent(content, mmapper.ini)
GeneralPage->>QFile: remove temp file
end
rect rgb(230,250,230)
note over User,GeneralPage: Import workflow
User->>GeneralPage: clicks configurationImportButton
GeneralPage->>QFileDialog: getOpenFileContent(callback importFile)
QFileDialog-->>GeneralPage: fileName, fileContent
GeneralPage->>QTemporaryFile: create temp import ini
GeneralPage->>QTemporaryFile: write content
GeneralPage->>QSettings: cfg.readFrom(tempImport)
GeneralPage->>Configuration: write
GeneralPage-->>ConfigDialog: sig_reloadConfig
ConfigDialog-->>GeneralPage: sig_loadConfig
ConfigDialog-->>OtherPages: sig_loadConfig
end
Class diagram for updated configuration and preferences classesclassDiagram
class Configuration {
+GeneralSettings general
+ColorSettings colorSettings
+NetworkSettings network
+void read()
+void write() const
+void reset()
+void readFrom(QSettings &conf)
+void writeTo(QSettings &conf) const
}
class AbstractParser {
+void doConfig(StringView cmd)
+bool isConnected() const
+Configuration & setConfig()
+const Configuration & getConfig() const
+void sendToUser(SendToUserSourceEnum source, const char *text)
+void sendOkToUser()
}
class RemoteEditWidget {
+RemoteEditWidget(bool modal, const QString &title, const QString &content, QWidget *parent)
+void show()
+void activateWindow()
+void setAttribute(Qt::WidgetAttribute attribute)
<<signal>> void sig_save(const QString &edited)
}
class GeneralPage {
+GeneralPage(QWidget *parent)
+void slot_loadConfig()
<<signal>> void sig_reloadConfig()
}
class ConfigDialog {
+ConfigDialog(QWidget *parent)
<<signal>> void sig_loadConfig()
}
class MumeFallbackSocket {
+void onSocketError(const QString &errorString)
}
Configuration <.. AbstractParser : uses
Configuration <.. GeneralPage : uses
Configuration <.. ConfigDialog : uses
AbstractParser --> RemoteEditWidget : creates and connects
GeneralPage ..> ConfigDialog : emits sig_reloadConfig
ConfigDialog ..> GeneralPage : emits sig_loadConfig
MumeFallbackSocket ..> SocketTypeEnum : 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 - I've found 1 issue, and left some high level feedback:
- The export/import logic for configuration appears duplicated across the new
config edithandler and the GeneralPage buttons (temp-file creation, QSettings write/read, QFile removal); consider extracting small helpers (e.g.,exportConfigToByteArray/importConfigFromByteArray) to centralize this behavior and reduce the chance of subtle inconsistencies. - The new public
Configuration::readFrom(QSettings &conf)andwriteTo(QSettings &conf) constmethods add a QSettings dependency to the header; ensureconfiguration.heither includes<QSettings>or forward-declaresclass QSettings;so that all translation units including this header compile cleanly.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The export/import logic for configuration appears duplicated across the new `config edit` handler and the GeneralPage buttons (temp-file creation, QSettings write/read, QFile removal); consider extracting small helpers (e.g., `exportConfigToByteArray` / `importConfigFromByteArray`) to centralize this behavior and reduce the chance of subtle inconsistencies.
- The new public `Configuration::readFrom(QSettings &conf)` and `writeTo(QSettings &conf) const` methods add a QSettings dependency to the header; ensure `configuration.h` either includes `<QSettings>` or forward-declares `class QSettings;` so that all translation units including this header compile cleanly.
## Individual Comments
### Comment 1
<location> `src/preferences/generalpage.cpp:142-143` </location>
<code_context>
+ });
+
+ connect(ui->configurationImportButton, &QAbstractButton::clicked, this, [this]() {
+ auto importFile = [this](const QString &fileName,
+ const std::optional<QByteArray> &fileContent) {
+ if (fileName.isEmpty() || !fileContent.has_value()) {
+ return;
</code_context>
<issue_to_address>
**issue (bug_risk):** Import callback signature using std::optional<QByteArray> may not match QFileDialog::getOpenFileContent expectations.
`QFileDialog::getOpenFileContent` expects a callback `void(const QString &, const QByteArray &)`. A lambda taking `const std::optional<QByteArray> &` will not match this signature and will fail to compile (no implicit conversion from `QByteArray` to `std::optional<QByteArray>`). If you need to represent “no content”, keep the Qt signature and use `content.isEmpty()` instead of `std::optional`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+142
to
+143
| auto importFile = [this](const QString &fileName, | ||
| const std::optional<QByteArray> &fileContent) { |
There was a problem hiding this comment.
issue (bug_risk): Import callback signature using std::optional may not match QFileDialog::getOpenFileContent expectations.
QFileDialog::getOpenFileContent expects a callback void(const QString &, const QByteArray &). A lambda taking const std::optional<QByteArray> & will not match this signature and will fail to compile (no implicit conversion from QByteArray to std::optional<QByteArray>). If you need to represent “no content”, keep the Qt signature and use content.isEmpty() instead of std::optional.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #444 +/- ##
==========================================
+ Coverage 66.48% 74.06% +7.58%
==========================================
Files 85 87 +2
Lines 4186 4033 -153
Branches 255 274 +19
==========================================
+ Hits 2783 2987 +204
+ Misses 1403 1046 -357 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary by Sourcery
Add configuration import/export and in-app editing capabilities, and adjust related configuration and connection handling.
New Features:
Bug Fixes:
Enhancements: