From ca61073bf026561c68aa5c18db0e8290bf660f82 Mon Sep 17 00:00:00 2001
From: Igor Kushnir <igorkuo@gmail.com>
Date: Sun, 11 May 2025 17:17:42 +0300
Subject: [PATCH] Implement LSPClientConfigPage::defaults() correctly

Functions that override KTextEditor::ConfigPage::defaults() are empty or
simply call reset() in many KTextEditor and Kate classes. This is not a
problem for Kate, KWrite, RKWard, Kile or KTechLab, because these
applications never invoke defaults(). KDevelop, however, invokes
defaults() when the user clicks the Restore Defaults button in the
Configure KDevelop dialog.

Instead of calling reset() in LSPClientConfigPage::defaults(), set the
checked state of each checkbox on the Client Settings tab of the LSP
Client configuration page to the default value of the corresponding
option as defined in LSPClientPlugin::readConfig().

Do not clear the empty-by-default UI elements on the second and third
tabs of the LSP Client configuration page to avoid accidental user data
loss. KDevelop's Clazy and Clang-Tidy configuration pages similarly do
not remove check sets configured by the user when the Restore Defaults
button is clicked.

Extract struct LSPClientPluginOptions and initialize its data members to
the actual default values defined in LSPClientPlugin::readConfig() so as
to reuse the default values of the LSP Client options and thus prevent
them from getting out of sync. The extracted struct also allows to share
between LSPClientConfigPage::reset() and LSPClientConfigPage::defaults()
the code that populates the checkboxes.

Reorder the declarations of the data members moved into
LSPClientPluginOptions and the assignments to them in
LSPClientPlugin::readConfig() to match the order in
LSPClientConfigPage::apply() and LSPClientConfigPage::reset().

Make LSPClientPlugin::readConfig() private and document the assumption
relied upon by the updated definition of this function.
---
 addons/lspclient/lspclientconfigpage.cpp | 54 ++++++++++++---------
 addons/lspclient/lspclientconfigpage.h   |  3 ++
 addons/lspclient/lspclientplugin.cpp     | 47 ++++++++++--------
 addons/lspclient/lspclientplugin.h       | 62 +++++++++++++++---------
 4 files changed, 102 insertions(+), 64 deletions(-)

diff --git a/addons/lspclient/lspclientconfigpage.cpp b/addons/lspclient/lspclientconfigpage.cpp
index f55079e3f4..83fa669dd3 100644
--- a/addons/lspclient/lspclientconfigpage.cpp
+++ b/addons/lspclient/lspclientconfigpage.cpp
@@ -157,30 +157,35 @@ void LSPClientConfigPage::apply()
     m_plugin->writeConfig();
 }
 
+void LSPClientConfigPage::resetUiTo(const LSPClientPluginOptions &options)
+{
+    ui->chkSymbolDetails->setChecked(options.m_symbolDetails);
+    ui->chkSymbolTree->setChecked(options.m_symbolTree);
+    ui->chkSymbolExpand->setChecked(options.m_symbolExpand);
+    ui->chkSymbolSort->setChecked(options.m_symbolSort);
+
+    ui->chkShowCompl->setChecked(options.m_showCompl);
+    ui->chkComplDoc->setChecked(options.m_complDoc);
+    ui->chkRefDeclaration->setChecked(options.m_refDeclaration);
+    ui->chkComplParens->setChecked(options.m_complParens);
+
+    ui->chkAutoHover->setChecked(options.m_autoHover);
+    ui->chkOnTypeFormatting->setChecked(options.m_onTypeFormatting);
+    ui->chkIncrementalSync->setChecked(options.m_incrementalSync);
+    ui->chkHighlightGoto->setChecked(options.m_highlightGoto);
+    ui->chkSemanticHighlighting->setChecked(options.m_semanticHighlighting);
+    ui->chkSignatureHelp->setChecked(options.m_signatureHelp);
+    ui->chkAutoImport->setChecked(options.m_autoImport);
+    ui->chkFmtOnSave->setChecked(options.m_fmtOnSave);
+    ui->chkInlayHint->setChecked(options.m_inlayHints);
+
+    ui->chkDiagnostics->setChecked(options.m_diagnostics);
+    ui->chkMessages->setChecked(options.m_messages);
+}
+
 void LSPClientConfigPage::reset()
 {
-    ui->chkSymbolDetails->setChecked(m_plugin->m_symbolDetails);
-    ui->chkSymbolTree->setChecked(m_plugin->m_symbolTree);
-    ui->chkSymbolExpand->setChecked(m_plugin->m_symbolExpand);
-    ui->chkSymbolSort->setChecked(m_plugin->m_symbolSort);
-
-    ui->chkShowCompl->setChecked(m_plugin->m_showCompl);
-    ui->chkComplDoc->setChecked(m_plugin->m_complDoc);
-    ui->chkRefDeclaration->setChecked(m_plugin->m_refDeclaration);
-    ui->chkComplParens->setChecked(m_plugin->m_complParens);
-
-    ui->chkAutoHover->setChecked(m_plugin->m_autoHover);
-    ui->chkOnTypeFormatting->setChecked(m_plugin->m_onTypeFormatting);
-    ui->chkIncrementalSync->setChecked(m_plugin->m_incrementalSync);
-    ui->chkHighlightGoto->setChecked(m_plugin->m_highlightGoto);
-    ui->chkSemanticHighlighting->setChecked(m_plugin->m_semanticHighlighting);
-    ui->chkSignatureHelp->setChecked(m_plugin->m_signatureHelp);
-    ui->chkAutoImport->setChecked(m_plugin->m_autoImport);
-    ui->chkFmtOnSave->setChecked(m_plugin->m_fmtOnSave);
-    ui->chkInlayHint->setChecked(m_plugin->m_inlayHints);
-
-    ui->chkDiagnostics->setChecked(m_plugin->m_diagnostics);
-    ui->chkMessages->setChecked(m_plugin->m_messages);
+    resetUiTo(*m_plugin);
 
     ui->edtConfigPath->setUrl(m_plugin->m_configPath);
 
@@ -196,7 +201,10 @@ void LSPClientConfigPage::reset()
 
 void LSPClientConfigPage::defaults()
 {
-    reset();
+    resetUiTo(LSPClientPluginOptions{});
+
+    // The user can easily manually revert the list of Allowed & Blocked Servers and the User Server Settings
+    // to their empty defaults. So do not automatically clear the possibly valuable user data on the other tabs.
 }
 
 void LSPClientConfigPage::readUserConfig(const QString &fileName)
diff --git a/addons/lspclient/lspclientconfigpage.h b/addons/lspclient/lspclientconfigpage.h
index 9c05f6a40f..dd920feb21 100644
--- a/addons/lspclient/lspclientconfigpage.h
+++ b/addons/lspclient/lspclientconfigpage.h
@@ -9,6 +9,7 @@
 #include <KTextEditor/ConfigPage>
 
 class LSPClientPlugin;
+struct LSPClientPluginOptions;
 
 namespace Ui
 {
@@ -35,6 +36,8 @@ public:
     void showContextMenuAllowedBlocked(const QPoint &pos);
 
 private:
+    void resetUiTo(const LSPClientPluginOptions &options);
+
     void readUserConfig(const QString &fileName);
     void updateConfigTextErrorState();
 
diff --git a/addons/lspclient/lspclientplugin.cpp b/addons/lspclient/lspclientplugin.cpp
index 6367a3a6b9..d1becb8dc1 100644
--- a/addons/lspclient/lspclientplugin.cpp
+++ b/addons/lspclient/lspclientplugin.cpp
@@ -110,27 +110,36 @@ KTextEditor::ConfigPage *LSPClientPlugin::configPage(int number, QWidget *parent
 
 void LSPClientPlugin::readConfig()
 {
+    // the indirection allows to make this function reusable by simply replacing
+    // the reference `defaults` with `const LSPClientPluginOptions defaults`.
+    const auto &defaults = *this;
+
     KConfigGroup config(KSharedConfig::openConfig(), CONFIG_LSPCLIENT);
-    m_symbolDetails = config.readEntry(CONFIG_SYMBOL_DETAILS, false);
-    m_symbolTree = config.readEntry(CONFIG_SYMBOL_TREE, true);
-    m_symbolExpand = config.readEntry(CONFIG_SYMBOL_EXPAND, true);
-    m_symbolSort = config.readEntry(CONFIG_SYMBOL_SORT, false);
-    m_complDoc = config.readEntry(CONFIG_COMPLETION_DOC, true);
-    m_refDeclaration = config.readEntry(CONFIG_REFERENCES_DECLARATION, true);
-    m_complParens = config.readEntry(CONFIG_COMPLETION_PARENS, true);
-    m_autoHover = config.readEntry(CONFIG_AUTO_HOVER, true);
-    m_onTypeFormatting = config.readEntry(CONFIG_TYPE_FORMATTING, false);
-    m_incrementalSync = config.readEntry(CONFIG_INCREMENTAL_SYNC, false);
-    m_highlightGoto = config.readEntry(CONFIG_HIGHLIGHT_GOTO, true);
-    m_diagnostics = config.readEntry(CONFIG_DIAGNOSTICS, true);
-    m_messages = config.readEntry(CONFIG_MESSAGES, true);
+
+    m_symbolDetails = config.readEntry(CONFIG_SYMBOL_DETAILS, defaults.m_symbolDetails);
+    m_symbolTree = config.readEntry(CONFIG_SYMBOL_TREE, defaults.m_symbolTree);
+    m_symbolExpand = config.readEntry(CONFIG_SYMBOL_EXPAND, defaults.m_symbolExpand);
+    m_symbolSort = config.readEntry(CONFIG_SYMBOL_SORT, defaults.m_symbolSort);
+
+    m_showCompl = config.readEntry(CONFIG_SHOW_COMPL, defaults.m_showCompl);
+    m_complDoc = config.readEntry(CONFIG_COMPLETION_DOC, defaults.m_complDoc);
+    m_refDeclaration = config.readEntry(CONFIG_REFERENCES_DECLARATION, defaults.m_refDeclaration);
+    m_complParens = config.readEntry(CONFIG_COMPLETION_PARENS, defaults.m_complParens);
+
+    m_autoHover = config.readEntry(CONFIG_AUTO_HOVER, defaults.m_autoHover);
+    m_onTypeFormatting = config.readEntry(CONFIG_TYPE_FORMATTING, defaults.m_onTypeFormatting);
+    m_incrementalSync = config.readEntry(CONFIG_INCREMENTAL_SYNC, defaults.m_incrementalSync);
+    m_highlightGoto = config.readEntry(CONFIG_HIGHLIGHT_GOTO, defaults.m_highlightGoto);
+    m_semanticHighlighting = config.readEntry(CONFIG_SEMANTIC_HIGHLIGHTING, defaults.m_semanticHighlighting);
+    m_signatureHelp = config.readEntry(CONFIG_SIGNATURE_HELP, defaults.m_signatureHelp);
+    m_autoImport = config.readEntry(CONFIG_AUTO_IMPORT, defaults.m_autoImport);
+    m_fmtOnSave = config.readEntry(CONFIG_FORMAT_ON_SAVE, defaults.m_fmtOnSave);
+    m_inlayHints = config.readEntry(CONFIG_INLAY_HINT, defaults.m_inlayHints);
+
+    m_diagnostics = config.readEntry(CONFIG_DIAGNOSTICS, defaults.m_diagnostics);
+    m_messages = config.readEntry(CONFIG_MESSAGES, defaults.m_messages);
+
     m_configPath = config.readEntry(CONFIG_SERVER_CONFIG, QUrl());
-    m_semanticHighlighting = config.readEntry(CONFIG_SEMANTIC_HIGHLIGHTING, true);
-    m_signatureHelp = config.readEntry(CONFIG_SIGNATURE_HELP, true);
-    m_autoImport = config.readEntry(CONFIG_AUTO_IMPORT, true);
-    m_fmtOnSave = config.readEntry(CONFIG_FORMAT_ON_SAVE, false);
-    m_inlayHints = config.readEntry(CONFIG_INLAY_HINT, false);
-    m_showCompl = config.readEntry(CONFIG_SHOW_COMPL, true);
 
     // read allow + block lists as two separate keys, let block always win
     const auto allowed = config.readEntry(CONFIG_ALLOWED_COMMANDS, QStringList());
diff --git a/addons/lspclient/lspclientplugin.h b/addons/lspclient/lspclientplugin.h
index f87b752082..8ede057f1f 100644
--- a/addons/lspclient/lspclientplugin.h
+++ b/addons/lspclient/lspclientplugin.h
@@ -17,7 +17,38 @@
 
 class LSPClientServerManager;
 
-class LSPClientPlugin : public KTextEditor::Plugin
+/**
+ * This struct aggregates the options that are reset to their
+ * default values when the user clicks the Restore Defaults button.
+ *
+ * @see LSPClientConfigPage::defaults()
+ */
+struct LSPClientPluginOptions {
+    bool m_symbolDetails = false;
+    bool m_symbolTree = true;
+    bool m_symbolExpand = true;
+    bool m_symbolSort = false;
+
+    bool m_showCompl = true;
+    bool m_complDoc = true;
+    bool m_refDeclaration = true;
+    bool m_complParens = true;
+
+    bool m_autoHover = true;
+    bool m_onTypeFormatting = false;
+    bool m_incrementalSync = false;
+    bool m_highlightGoto = true;
+    bool m_semanticHighlighting = true;
+    bool m_signatureHelp = true;
+    bool m_autoImport = true;
+    bool m_fmtOnSave = false;
+    bool m_inlayHints = false;
+
+    bool m_diagnostics = true;
+    bool m_messages = true;
+};
+
+class LSPClientPlugin : public KTextEditor::Plugin, public LSPClientPluginOptions
 {
     Q_OBJECT
 
@@ -38,7 +69,6 @@ public:
     int configPages() const override;
     KTextEditor::ConfigPage *configPage(int number = 0, QWidget *parent = nullptr) override;
 
-    void readConfig();
     void writeConfig() const;
 
     // path for local setting files, auto-created on load
@@ -49,27 +79,7 @@ public:
 
     QStringList m_alwaysDisabledLanguages;
 
-    // settings
-    bool m_symbolDetails = false;
-    bool m_symbolExpand = false;
-    bool m_symbolTree = false;
-    bool m_symbolSort = false;
-    bool m_complDoc = false;
-    bool m_refDeclaration = false;
-    bool m_complParens = false;
-    bool m_diagnostics = false;
-    bool m_messages = false;
-    bool m_autoHover = false;
-    bool m_onTypeFormatting = false;
-    bool m_incrementalSync = false;
-    bool m_highlightGoto = true;
     QUrl m_configPath;
-    bool m_semanticHighlighting = false;
-    bool m_signatureHelp = true;
-    bool m_autoImport = true;
-    bool m_fmtOnSave = false;
-    bool m_inlayHints = false;
-    bool m_showCompl = true;
 
     // debug mode?
     const bool m_debugMode;
@@ -110,6 +120,14 @@ private Q_SLOTS:
     void askForCommandLinePermission(const QString &fullCommandLineString);
 
 private:
+    /**
+     * Read from config and assign to data members.
+     *
+     * This function may be called only from the constructor because it assumes
+     * that the LSPClientPluginOptions base of this object is default-initialized.
+     */
+    void readConfig();
+
     // server manager to pass along
     std::shared_ptr<LSPClientServerManager> m_serverManager;
 };
-- 
GitLab