Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup code naming conventions and CMake files #8480

Draft
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

louib
Copy link
Member

@louib louib commented Sep 19, 2022

This PR further cleans up the KeePassXC code naming conventions, file locations, and CMake configuration files. The long term goal is to create an architectural boundary around the core module, in preparation of libkdbx.

TODO:

  • Rename all instances of keepassx to keepassxc
  • Move all files destined for core under the core subdirectory (TOTP, format, keys, crypto, etc)
  • Move all files related to fundamental GUI operations under the gui subdirectory
  • Consolidate all thirdparty files under the thirdparty subdirectory
  • Move all files related to specific toggleable features to their respective subdirectories

Testing strategy

Standard test cases apply

Type of change

  • ✅ Refactor (significant modification to existing code)

@louib

This comment was marked as outdated.

@louib louib force-pushed the fork_keepassx_core branch from 250f18a to b6b4778 Compare September 24, 2022 01:20
@droidmonkey

This comment was marked as outdated.

@louib

This comment was marked as outdated.

@louib louib force-pushed the fork_keepassx_core branch from b6b4778 to fa333cf Compare September 24, 2022 02:20
@droidmonkey

This comment was marked as outdated.

@louib

This comment was marked as outdated.

@droidmonkey

This comment was marked as outdated.

@droidmonkey droidmonkey added this to the v2.8.0 milestone Oct 2, 2022
@droidmonkey droidmonkey added high priority 🚨 pr: refactoring Pull request that refactors code labels Oct 2, 2022
@louib louib force-pushed the fork_keepassx_core branch 4 times, most recently from 7318512 to 1ec8b1a Compare October 8, 2022 14:47
@droidmonkey

This comment was marked as outdated.

@louib

This comment was marked as outdated.

@yan12125

This comment was marked as outdated.

@louib louib force-pushed the fork_keepassx_core branch 2 times, most recently from 9786f04 to c97db51 Compare November 20, 2022 14:24
@yan12125

This comment was marked as outdated.

@louib

This comment was marked as outdated.

@droidmonkey

This comment was marked as outdated.

@louib

This comment was marked as outdated.

@louib louib force-pushed the fork_keepassx_core branch from 7f6557a to dcfea23 Compare November 22, 2022 02:03
@codecov-commenter
Copy link

codecov-commenter commented Nov 22, 2022

Codecov Report

Base: 64.41% // Head: 64.41% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (be94f97) compared to base (3e3e87d).
Patch has no changes to coverable lines.

❗ Current head be94f97 differs from pull request most recent head bcb547a. Consider uploading reports for the commit bcb547a to get more accurate results

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #8480   +/-   ##
========================================
  Coverage    64.41%   64.41%           
========================================
  Files          341      341           
  Lines        44257    44257           
========================================
+ Hits         28505    28507    +2     
+ Misses       15752    15750    -2     
Impacted Files Coverage Δ
src/core/Global.h 100.00% <ø> (ø)
src/gui/Clipboard.h 100.00% <ø> (ø)
src/gui/IconDownloader.cpp 45.83% <ø> (ø)
src/gui/MainWindow.cpp 71.49% <ø> (ø)
src/gui/UpdateCheckDialog.cpp 0.00% <ø> (ø)
src/networking/HibpDownloader.cpp 6.67% <ø> (ø)
src/networking/NetworkManager.cpp 100.00% <ø> (ø)
src/networking/UpdateChecker.cpp 93.40% <ø> (ø)
src/networking/UpdateChecker.h 100.00% <ø> (ø)
src/gui/osutils/nixutils/NixUtils.cpp 34.95% <0.00%> (+1.08%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@droidmonkey

This comment was marked as outdated.

@louib

This comment was marked as outdated.

@droidmonkey
Copy link
Member

droidmonkey commented Nov 25, 2022

I made a bunch of edits to standardize on "keepassxc" over "keepassx", moved some statements around in various CMakeLists.txt, and also cleaned up some test code as well.

louib and others added 6 commits December 21, 2022 22:16
This PR splits the GUI source files from the core
source files. The immediate goal is to allow the
CLI to require only a minimum number of dynamic
libraries. The long term goal is to create an
architectural boundary around the core module,
in preparation of libkdbx.
* Rename all variables to use keepassxc instead of keepassx
* Move some definitions to their proper locations in CMakeLists.txt
* Disable quick unlock when running TestGuiBrowser
* Use QString and auto pointers in TestMerge. This test was failing under Visual Studio.
@louib louib force-pushed the fork_keepassx_core branch from 38d06a2 to f23b07f Compare December 22, 2022 03:16
@yan12125
Copy link
Contributor

undefined reference to xxx linker failures on Ubuntu Linux CI can be fixed with:

diff --git a/src/browser/CMakeLists.txt b/src/browser/CMakeLists.txt
index 98715cb1..d1dbf50f 100755
--- a/src/browser/CMakeLists.txt
+++ b/src/browser/CMakeLists.txt
@@ -33,5 +33,5 @@ if(WITH_XC_BROWSER)
             )
 
     add_library(keepassxcbrowser STATIC ${keepassxcbrowser_SOURCES})
-    target_link_libraries(keepassxcbrowser Qt5::Core Qt5::Concurrent Qt5::Widgets Qt5::Network ${BOTAN2_LIBRARIES})
+    target_link_libraries(keepassxcbrowser keepassxc_gui Qt5::Core Qt5::Concurrent Qt5::Widgets Qt5::Network ${BOTAN2_LIBRARIES})
 endif()
diff --git a/src/fdosecrets/CMakeLists.txt b/src/fdosecrets/CMakeLists.txt
index fec3f99d..860bf8a8 100644
--- a/src/fdosecrets/CMakeLists.txt
+++ b/src/fdosecrets/CMakeLists.txt
@@ -31,5 +31,5 @@ if(WITH_XC_FDOSECRETS)
         objects/Prompt.cpp
         dbus/DBusTypes.cpp
     )
-    target_link_libraries(fdosecrets Qt5::Core Qt5::Widgets Qt5::DBus ${BOTAN2_LIBRARIES})
+    target_link_libraries(fdosecrets keepassxc_gui Qt5::Core Qt5::Widgets Qt5::DBus ${BOTAN2_LIBRARIES})
 endif()
diff --git a/src/sshagent/CMakeLists.txt b/src/sshagent/CMakeLists.txt
index 96946741..29c109cf 100644
--- a/src/sshagent/CMakeLists.txt
+++ b/src/sshagent/CMakeLists.txt
@@ -12,5 +12,5 @@ if(WITH_XC_SSHAGENT)
     )
 
     add_library(sshagent STATIC ${sshagent_SOURCES})
-    target_link_libraries(sshagent Qt5::Core Qt5::Widgets Qt5::Network)
+    target_link_libraries(sshagent keepassxc_gui Qt5::Core Qt5::Widgets Qt5::Network)
 endif()

Apparently those three plugins still depend on GUI codes.

@louib louib force-pushed the fork_keepassx_core branch from 6eebc8f to bcb547a Compare December 27, 2022 18:21
@louib
Copy link
Member Author

louib commented Dec 27, 2022

@droidmonkey failures on Ubuntu are looking legit:

undefined reference to `AutoType::instance()'

The autotype feature is not decoupled from the GUI, there's a lot of direct calls to autotype modules that are not fenced behind a WITH_XC_AUTOTYPE. I think part of 48715c6 will have to be reverted for this PR to be mergeable again.

@droidmonkey
Copy link
Member

Auto type only makes sense in a gui context. Shouldn't be hard to move everything over to the gui side of the build.

@yan12125
Copy link
Contributor

yan12125 commented Dec 28, 2022

Auto type only makes sense in a gui context. Shouldn't be hard to move everything over to the gui side of the build.

Auto type is already in the GUI side. The issue is that GUI codes interleaved with auto-type codes, so disabling auto-type may cause issues.

there's a lot of direct calls to autotype modules that are not fenced behind a WITH_XC_AUTOTYPE

On the other hand, I did some quick and dirty wrapping:

An ongoing patch on top of this branch
diff --git a/src/gui/ApplicationSettingsWidget.cpp b/src/gui/ApplicationSettingsWidget.cpp
index 7972e8e0c7..b7e09f6aa0 100644
--- a/src/gui/ApplicationSettingsWidget.cpp
+++ b/src/gui/ApplicationSettingsWidget.cpp
@@ -24,7 +24,9 @@
 
 #include "config-keepassx.h"
 
+#ifdef WITH_XC_AUTOTYPE
 #include "autotype/AutoType.h"
+#endif
 #include "core/Translator.h"
 #include "gui/Icons.h"
 #include "gui/MainWindow.h"
@@ -102,9 +104,13 @@ ApplicationSettingsWidget::ApplicationSettingsWidget(QWidget* parent)
     addPage(tr("General"), icons()->icon("preferences-other"), m_generalWidget);
     addPage(tr("Security"), icons()->icon("security-high"), m_secWidget);
 
+#ifdef WITH_XC_AUTOTYPE
     if (!autoType()->isAvailable()) {
         m_generalUi->generalSettingsTabWidget->removeTab(1);
     }
+#else
+    m_generalUi->generalSettingsTabWidget->removeTab(1);
+#endif
 
     connect(this, SIGNAL(accepted()), SLOT(saveSettings()));
     connect(this, SIGNAL(rejected()), SLOT(reject()));
@@ -272,6 +278,7 @@ void ApplicationSettingsWidget::loadSettings()
     m_generalUi->autoTypeAskCheckBox->setChecked(config()->get(Config::Security_AutoTypeAsk).toBool());
     m_generalUi->autoTypeRelockDatabaseCheckBox->setChecked(config()->get(Config::Security_RelockAutoType).toBool());
 
+#ifdef WITH_XC_AUTOTYPE
     if (autoType()->isAvailable()) {
         m_globalAutoTypeKey = static_cast<Qt::Key>(config()->get(Config::GlobalAutoTypeKey).toInt());
         m_globalAutoTypeModifiers =
@@ -284,6 +291,7 @@ void ApplicationSettingsWidget::loadSettings()
         m_generalUi->autoTypeDelaySpinBox->setValue(config()->get(Config::AutoTypeDelay).toInt());
         m_generalUi->autoTypeStartDelaySpinBox->setValue(config()->get(Config::AutoTypeStartDelay).toInt());
     }
+#endif
 
     m_generalUi->trayIconAppearance->clear();
 #if defined(Q_OS_MACOS) || defined(Q_OS_WIN)
@@ -405,6 +413,7 @@ void ApplicationSettingsWidget::saveSettings()
     config()->set(Config::Security_AutoTypeAsk, m_generalUi->autoTypeAskCheckBox->isChecked());
     config()->set(Config::Security_RelockAutoType, m_generalUi->autoTypeRelockDatabaseCheckBox->isChecked());
 
+#ifdef WITH_XC_AUTOTYPE
     if (autoType()->isAvailable()) {
         config()->set(Config::GlobalAutoTypeKey, m_generalUi->autoTypeShortcutWidget->key());
         config()->set(Config::GlobalAutoTypeModifiers,
@@ -413,6 +422,7 @@ void ApplicationSettingsWidget::saveSettings()
         config()->set(Config::AutoTypeDelay, m_generalUi->autoTypeDelaySpinBox->value());
         config()->set(Config::AutoTypeStartDelay, m_generalUi->autoTypeStartDelaySpinBox->value());
     }
+#endif
     config()->set(Config::Security_ClearClipboard, m_secUi->clearClipboardCheckBox->isChecked());
     config()->set(Config::Security_ClearClipboardTimeout, m_secUi->clearClipboardSpinBox->value());
 
@@ -497,10 +507,12 @@ void ApplicationSettingsWidget::resetSettings()
 
 void ApplicationSettingsWidget::reject()
 {
+#ifdef WITH_XC_AUTOTYPE
     // register the old key again as it might have changed
     if (m_globalAutoTypeKey > 0 && m_globalAutoTypeModifiers > 0) {
         autoType()->registerGlobalShortcut(m_globalAutoTypeKey, m_globalAutoTypeModifiers);
     }
+#endif
 }
 
 void ApplicationSettingsWidget::autoSaveToggled(bool checked)
@@ -557,4 +569,4 @@ void ApplicationSettingsWidget::selectBackupDirectory()
         m_generalUi->backupFilePath->setText(
             QDir(backupDirectory).filePath(config()->getDefault(Config::BackupFilePathPattern).toString()));
     }
-}
\ No newline at end of file
+}
diff --git a/src/gui/DatabaseTabWidget.cpp b/src/gui/DatabaseTabWidget.cpp
index 8d95aaef14..686e18cd1a 100644
--- a/src/gui/DatabaseTabWidget.cpp
+++ b/src/gui/DatabaseTabWidget.cpp
@@ -20,7 +20,9 @@
 #include <QFileInfo>
 #include <QTabBar>
 
+#ifdef WITH_XC_AUTOTYPE
 #include "autotype/AutoType.h"
+#endif
 #include "core/Tools.h"
 #include "format/CsvExporter.h"
 #include "gui/Clipboard.h"
@@ -53,9 +55,11 @@ DatabaseTabWidget::DatabaseTabWidget(QWidget* parent)
     connect(this, SIGNAL(currentChanged(int)), SLOT(emitActiveDatabaseChanged()));
     connect(this, SIGNAL(activeDatabaseChanged(DatabaseWidget*)),
             m_dbWidgetStateSync, SLOT(setActive(DatabaseWidget*)));
+#ifdef WITH_XC_AUTOTYPE
     connect(autoType(), SIGNAL(globalAutoTypeTriggered(const QString&)), SLOT(performGlobalAutoType(const QString&)));
     connect(autoType(), SIGNAL(autotypeRetypeTimeout()), SLOT(relockPendingDatabase()));
     connect(autoType(), SIGNAL(autotypeRejected()), SLOT(relockPendingDatabase()));
+#endif
     connect(m_databaseOpenDialog.data(), &DatabaseOpenDialog::dialogFinished,
             this, &DatabaseTabWidget::handleDatabaseUnlockDialogFinished);
     // clang-format on
@@ -849,6 +853,7 @@ void DatabaseTabWidget::emitDatabaseLockChanged()
     }
 }
 
+#ifdef WITH_XC_AUTOTYPE
 void DatabaseTabWidget::performGlobalAutoType(const QString& search)
 {
     auto currentDbWidget = currentDatabaseWidget();
@@ -875,6 +880,7 @@ void DatabaseTabWidget::performGlobalAutoType(const QString& search)
         autoType()->performGlobalAutoType(unlockedDatabases, search);
     }
 }
+#endif
 
 void DatabaseTabWidget::performBrowserUnlock()
 {
diff --git a/src/gui/DatabaseTabWidget.h b/src/gui/DatabaseTabWidget.h
index 38a1822bfe..6ac9b04f7f 100644
--- a/src/gui/DatabaseTabWidget.h
+++ b/src/gui/DatabaseTabWidget.h
@@ -84,7 +84,9 @@ public slots:
     void showDatabaseSecurity();
     void showDatabaseReports();
     void showDatabaseSettings();
+#ifdef WITH_XC_AUTOTYPE
     void performGlobalAutoType(const QString& search);
+#endif
     void performBrowserUnlock();
 
 signals:
diff --git a/src/gui/DatabaseWidget.cpp b/src/gui/DatabaseWidget.cpp
index 76191fd2bc..cabb5e40f1 100644
--- a/src/gui/DatabaseWidget.cpp
+++ b/src/gui/DatabaseWidget.cpp
@@ -32,7 +32,9 @@
 #include <QTextEdit>
 #include <core/Tools.h>
 
+#ifdef WITH_XC_AUTOTYPE
 #include "autotype/AutoType.h"
+#endif
 #include "core/EntrySearcher.h"
 #include "core/Merger.h"
 #include "gui/Clipboard.h"
@@ -769,6 +771,7 @@ void DatabaseWidget::removeFromAgent()
 }
 #endif
 
+#ifdef WITH_XC_AUTOTYPE
 void DatabaseWidget::performAutoType(const QString& sequence)
 {
     auto currentEntry = currentSelectedEntry();
@@ -813,6 +816,7 @@ void DatabaseWidget::performAutoTypeTOTP()
 {
     performAutoType(QStringLiteral("{TOTP}"));
 }
+#endif
 
 void DatabaseWidget::openUrl()
 {
diff --git a/src/gui/DatabaseWidget.h b/src/gui/DatabaseWidget.h
index daca949e1b..bb9d4215bb 100644
--- a/src/gui/DatabaseWidget.h
+++ b/src/gui/DatabaseWidget.h
@@ -187,12 +187,14 @@ public slots:
     void addToAgent();
     void removeFromAgent();
 #endif
+#ifdef WITH_XC_AUTOTYPE
     void performAutoType(const QString& sequence = {});
     void performAutoTypeUsername();
     void performAutoTypeUsernameEnter();
     void performAutoTypePassword();
     void performAutoTypePasswordEnter();
     void performAutoTypeTOTP();
+#endif
     void openUrl();
     void downloadSelectedFavicons();
     void downloadAllFavicons();
diff --git a/src/gui/MainWindow.cpp b/src/gui/MainWindow.cpp
index 437653a3a9..eb8af0b7b0 100644
--- a/src/gui/MainWindow.cpp
+++ b/src/gui/MainWindow.cpp
@@ -35,7 +35,9 @@
 
 #include "Application.h"
 #include "Clipboard.h"
+#ifdef WITH_XC_AUTOTYPE
 #include "autotype/AutoType.h"
+#endif
 #include "core/InactivityTimer.h"
 #include "core/Resources.h"
 #include "core/Tools.h"
@@ -150,6 +152,7 @@ MainWindow::MainWindow()
     m_entryNewContextMenu = new QMenu(this);
     m_entryNewContextMenu->addAction(m_ui->actionEntryNew);
 
+#ifdef WITH_XC_AUTOTYPE
     // Build Entry Level Auto-Type menu
     auto autotypeMenu = new QMenu({}, this);
     autotypeMenu->addAction(m_ui->actionEntryAutoTypeSequence);
@@ -164,6 +167,7 @@ MainWindow::MainWindow()
     if (autoTypeButton) {
         autoTypeButton->setPopupMode(QToolButton::MenuButtonPopup);
     }
+#endif
 
     auto databaseLockMenu = new QMenu({}, this);
     databaseLockMenu->addAction(m_ui->actionLockAllDatabases);
@@ -247,17 +251,21 @@ MainWindow::MainWindow()
     m_actionMultiplexer.connect(m_setTagsMenuActions, SIGNAL(triggered(QAction*)), SLOT(setTag(QAction*)));
     connect(m_ui->menuTags, &QMenu::aboutToShow, this, &MainWindow::updateSetTagsMenu);
 
+#ifdef WITH_XC_AUTOTYPE
     Qt::Key globalAutoTypeKey = static_cast<Qt::Key>(config()->get(Config::GlobalAutoTypeKey).toInt());
     Qt::KeyboardModifiers globalAutoTypeModifiers =
         static_cast<Qt::KeyboardModifiers>(config()->get(Config::GlobalAutoTypeModifiers).toInt());
     if (globalAutoTypeKey > 0 && globalAutoTypeModifiers > 0) {
         autoType()->registerGlobalShortcut(globalAutoTypeKey, globalAutoTypeModifiers);
     }
+#endif
 
     m_ui->toolbarSeparator->setVisible(false);
     m_showToolbarSeparator = config()->get(Config::GUI_ApplicationTheme).toString() != "classic";
 
+#ifdef WITH_XC_AUTOTYPE
     m_ui->actionEntryAutoType->setVisible(autoType()->isAvailable());
+#endif
 
     m_inactivityTimer = new InactivityTimer(this);
     connect(m_inactivityTimer, SIGNAL(inactivityDetected()), this, SLOT(lockDatabasesAfterInactivity()));
@@ -284,7 +292,9 @@ MainWindow::MainWindow()
     m_ui->actionEntryMoveDown->setShortcut(Qt::CTRL + Qt::ALT + Qt::Key_Down);
     m_ui->actionEntryCopyUsername->setShortcut(Qt::CTRL + Qt::Key_B);
     m_ui->actionEntryCopyPassword->setShortcut(Qt::CTRL + Qt::Key_C);
+#ifdef WITH_XC_AUTOTYPE
     m_ui->actionEntryAutoTypeSequence->setShortcut(Qt::CTRL + Qt::SHIFT + Qt::Key_V);
+#endif
     m_ui->actionEntryOpenUrl->setShortcut(Qt::CTRL + Qt::SHIFT + Qt::Key_U);
     m_ui->actionEntryCopyURL->setShortcut(Qt::CTRL + Qt::Key_U);
     m_ui->actionEntryRestore->setShortcut(Qt::CTRL + Qt::Key_R);
@@ -314,7 +324,9 @@ MainWindow::MainWindow()
     m_ui->actionEntryMoveDown->setShortcutVisibleInContextMenu(true);
     m_ui->actionEntryCopyUsername->setShortcutVisibleInContextMenu(true);
     m_ui->actionEntryCopyPassword->setShortcutVisibleInContextMenu(true);
+#ifdef WITH_XC_AUTOTYPE
     m_ui->actionEntryAutoTypeSequence->setShortcutVisibleInContextMenu(true);
+#endif
     m_ui->actionEntryOpenUrl->setShortcutVisibleInContextMenu(true);
     m_ui->actionEntryCopyURL->setShortcutVisibleInContextMenu(true);
     m_ui->actionEntryAddToAgent->setShortcutVisibleInContextMenu(true);
@@ -392,6 +404,7 @@ MainWindow::MainWindow()
     m_ui->actionEntryEdit->setIcon(icons()->icon("entry-edit"));
     m_ui->actionEntryDelete->setIcon(icons()->icon("entry-delete"));
     m_ui->actionEntryRestore->setIcon(icons()->icon("entry-restore"));
+#ifdef WITH_XC_AUTOTYPE
     m_ui->actionEntryAutoType->setIcon(icons()->icon("auto-type"));
     m_ui->actionEntryAutoTypeSequence->setIcon(icons()->icon("auto-type"));
     m_ui->actionEntryAutoTypeUsername->setIcon(icons()->icon("auto-type"));
@@ -399,6 +412,7 @@ MainWindow::MainWindow()
     m_ui->actionEntryAutoTypePassword->setIcon(icons()->icon("auto-type"));
     m_ui->actionEntryAutoTypePasswordEnter->setIcon(icons()->icon("auto-type"));
     m_ui->actionEntryAutoTypeTOTP->setIcon(icons()->icon("auto-type"));
+#endif
     m_ui->actionEntryMoveUp->setIcon(icons()->icon("move-up"));
     m_ui->actionEntryMoveDown->setIcon(icons()->icon("move-down"));
     m_ui->actionEntryCopyUsername->setIcon(icons()->icon("username-copy"));
@@ -511,6 +525,7 @@ MainWindow::MainWindow()
     m_actionMultiplexer.connect(m_ui->actionEntryCopyPassword, SIGNAL(triggered()), SLOT(copyPassword()));
     m_actionMultiplexer.connect(m_ui->actionEntryCopyURL, SIGNAL(triggered()), SLOT(copyURL()));
     m_actionMultiplexer.connect(m_ui->actionEntryCopyNotes, SIGNAL(triggered()), SLOT(copyNotes()));
+#ifdef WITH_XC_AUTOTYPE
     m_actionMultiplexer.connect(m_ui->actionEntryAutoType, SIGNAL(triggered()), SLOT(performAutoType()));
     m_actionMultiplexer.connect(m_ui->actionEntryAutoTypeSequence, SIGNAL(triggered()), SLOT(performAutoType()));
     m_actionMultiplexer.connect(
@@ -522,6 +537,7 @@ MainWindow::MainWindow()
     m_actionMultiplexer.connect(
         m_ui->actionEntryAutoTypePasswordEnter, SIGNAL(triggered()), SLOT(performAutoTypePasswordEnter()));
     m_actionMultiplexer.connect(m_ui->actionEntryAutoTypeTOTP, SIGNAL(triggered()), SLOT(performAutoTypeTOTP()));
+#endif
     m_actionMultiplexer.connect(m_ui->actionEntryOpenUrl, SIGNAL(triggered()), SLOT(openUrl()));
     m_actionMultiplexer.connect(m_ui->actionEntryDownloadIcon, SIGNAL(triggered()), SLOT(downloadSelectedFavicons()));
 #ifdef WITH_XC_SSHAGENT
@@ -930,6 +946,7 @@ void MainWindow::setMenuActionState(DatabaseWidget::Mode mode)
             m_ui->menuEntryCopyAttribute->setEnabled(singleEntrySelected);
             m_ui->menuEntryTotp->setEnabled(singleEntrySelected);
             m_ui->menuTags->setEnabled(entriesSelected);
+#ifdef WITH_XC_AUTOTYPE
             m_ui->actionEntryAutoType->setEnabled(singleEntrySelected);
             m_ui->actionEntryAutoType->menu()->setEnabled(singleEntrySelected);
             m_ui->actionEntryAutoTypeSequence->setText(
@@ -944,6 +961,7 @@ void MainWindow::setMenuActionState(DatabaseWidget::Mode mode)
                                                                && dbWidget->currentEntryHasPassword());
             m_ui->actionEntryAutoTypeTOTP->setEnabled(singleEntrySelected && dbWidget->currentEntryHasTotp());
             m_ui->actionEntryAutoTypeTOTP->setVisible(singleEntrySelected && dbWidget->currentEntryHasTotp());
+#endif
             m_ui->actionEntryOpenUrl->setEnabled(singleEntrySelected && dbWidget->currentEntryHasUrl());
             m_ui->actionEntryTotp->setEnabled(singleEntrySelected && dbWidget->currentEntryHasTotp());
             m_ui->actionEntryCopyTotp->setEnabled(singleEntrySelected && dbWidget->currentEntryHasTotp());
@@ -998,7 +1016,9 @@ void MainWindow::setMenuActionState(DatabaseWidget::Mode mode)
                                                                m_ui->actionEntryCopyPassword,
                                                                m_ui->actionEntryCopyURL,
                                                                m_ui->actionEntryOpenUrl,
+#ifdef WITH_XC_AUTOTYPE
                                                                m_ui->actionEntryAutoType,
+#endif
                                                                m_ui->actionEntryDownloadIcon,
                                                                m_ui->actionEntryCopyNotes,
                                                                m_ui->actionEntryCopyTitle,
diff --git a/src/gui/entry/EditEntryWidget.cpp b/src/gui/entry/EditEntryWidget.cpp
index b03d469e55..1b752307ab 100644
--- a/src/gui/entry/EditEntryWidget.cpp
+++ b/src/gui/entry/EditEntryWidget.cpp
@@ -18,7 +18,9 @@
 
 #include "EditEntryWidget.h"
 #include "ui_EditEntryWidgetAdvanced.h"
+#ifdef WITH_XC_AUTOTYPE
 #include "ui_EditEntryWidgetAutoType.h"
+#endif
 #include "ui_EditEntryWidgetBrowser.h"
 #include "ui_EditEntryWidgetHistory.h"
 #include "ui_EditEntryWidgetMain.h"
@@ -29,7 +31,9 @@
 #include <QSortFilterProxyModel>
 #include <QStringListModel>
 
+#ifdef WITH_XC_AUTOTYPE
 #include "autotype/AutoType.h"
+#endif
 #include "core/AutoTypeAssociations.h"
 #include "core/Clock.h"
 #include "core/Config.h"
@@ -63,7 +67,9 @@ EditEntryWidget::EditEntryWidget(QWidget* parent)
     , m_entry(nullptr)
     , m_mainUi(new Ui::EditEntryWidgetMain())
     , m_advancedUi(new Ui::EditEntryWidgetAdvanced())
+#ifdef WITH_XC_AUTOTYPE
     , m_autoTypeUi(new Ui::EditEntryWidgetAutoType())
+#endif
     , m_sshAgentUi(new Ui::EditEntryWidgetSSHAgent())
     , m_historyUi(new Ui::EditEntryWidgetHistory())
     , m_browserUi(new Ui::EditEntryWidgetBrowser())
@@ -97,7 +103,9 @@ EditEntryWidget::EditEntryWidget(QWidget* parent)
     setupMain();
     setupAdvanced();
     setupIcon();
+#ifdef WITH_XC_AUTOTYPE
     setupAutoType();
+#endif
 
 #ifdef WITH_XC_SSHAGENT
     setupSSHAgent();
@@ -213,6 +221,7 @@ void EditEntryWidget::setupIcon()
     connect(this, SIGNAL(rejected()), m_iconsWidget, SLOT(abortRequests()));
 }
 
+#ifdef WITH_XC_AUTOTYPE
 void EditEntryWidget::openAutotypeHelp()
 {
     QDesktopServices::openUrl(
@@ -254,6 +263,7 @@ void EditEntryWidget::setupAutoType()
     connect(m_autoTypeUi->windowSequenceEdit, SIGNAL(textChanged(QString)), SLOT(applyCurrentAssoc()));
     // clang-format on
 }
+#endif
 
 #ifdef WITH_XC_BROWSER
 void EditEntryWidget::setupBrowser()
@@ -459,6 +469,7 @@ void EditEntryWidget::setupEntryUpdate()
     // Icon tab
     connect(m_iconsWidget, SIGNAL(widgetUpdated()), this, SLOT(setModified()));
 
+#ifdef WITH_XC_AUTOTYPE
     // Auto-Type tab
     connect(m_autoTypeUi->enableButton, SIGNAL(stateChanged(int)), this, SLOT(setModified()));
     connect(m_autoTypeUi->customWindowSequenceButton, SIGNAL(stateChanged(int)), this, SLOT(setModified()));
@@ -468,6 +479,7 @@ void EditEntryWidget::setupEntryUpdate()
     connect(m_autoTypeUi->sequenceEdit, SIGNAL(textChanged(QString)), this, SLOT(setModified()));
     connect(m_autoTypeUi->windowTitleCombo, SIGNAL(currentIndexChanged(int)), this, SLOT(setModified()));
     connect(m_autoTypeUi->windowTitleCombo, SIGNAL(editTextChanged(QString)), this, SLOT(setModified()));
+#endif
 
     // Properties and History tabs don't need extra connections
 
@@ -886,9 +898,11 @@ void EditEntryWidget::setForms(Entry* entry, bool restore)
     setupColorButton(true, entry->foregroundColor());
     setupColorButton(false, entry->backgroundColor());
     m_iconsWidget->setEnabled(!m_history);
+#ifdef WITH_XC_AUTOTYPE
     m_autoTypeUi->sequenceEdit->setReadOnly(m_history);
     m_autoTypeUi->windowTitleCombo->lineEdit()->setReadOnly(m_history);
     m_autoTypeUi->windowSequenceEdit->setReadOnly(m_history);
+#endif
     m_historyWidget->setEnabled(!m_history);
 
     m_mainUi->titleEdit->setText(entry->title());
@@ -932,6 +946,7 @@ void EditEntryWidget::setForms(Entry* entry, bool restore)
     iconStruct.number = entry->iconNumber();
     m_iconsWidget->load(entry->uuid(), m_db, iconStruct, entry->webUrl());
 
+#ifdef WITH_XC_AUTOTYPE
     m_autoTypeUi->enableButton->setChecked(entry->autoTypeEnabled());
     if (entry->defaultAutoTypeSequence().isEmpty()) {
         m_autoTypeUi->inheritSequenceButton->setChecked(true);
@@ -951,6 +966,7 @@ void EditEntryWidget::setForms(Entry* entry, bool restore)
         m_autoTypeUi->windowTitleCombo->refreshWindowList();
     }
     updateAutoTypeEnabled();
+#endif
 
 #ifdef WITH_XC_SSHAGENT
     if (sshAgent()->isEnabled()) {
@@ -1067,6 +1083,7 @@ bool EditEntryWidget::commitEntry()
         return true;
     }
 
+#ifdef WITH_XC_AUTOTYPE
     // Check Auto-Type validity early
     QString error;
     if (m_autoTypeUi->customSequenceButton->isChecked()
@@ -1099,6 +1116,7 @@ bool EditEntryWidget::commitEntry()
             }
         }
     }
+#endif
 
     if (m_advancedUi->attributesView->currentIndex().isValid() && m_advancedUi->attributesEdit->isEnabled()) {
         QString key = m_attributesModel->keyByIndex(m_advancedUi->attributesView->currentIndex());
@@ -1193,6 +1211,7 @@ void EditEntryWidget::updateEntryData(Entry* entry) const
         entry->setIcon(iconStruct.uuid);
     }
 
+#ifdef WITH_XC_AUTOTYPE
     entry->setAutoTypeEnabled(m_autoTypeUi->enableButton->isChecked());
     if (m_autoTypeUi->inheritSequenceButton->isChecked()) {
         entry->setDefaultAutoTypeSequence(QString());
@@ -1201,6 +1220,7 @@ void EditEntryWidget::updateEntryData(Entry* entry) const
     }
 
     entry->autoTypeAssociations()->copyDataFrom(m_autoTypeAssoc);
+#endif
 
 #ifdef WITH_XC_SSHAGENT
     if (sshAgent()->isEnabled()) {
@@ -1443,6 +1463,7 @@ void EditEntryWidget::toggleCurrentAttributeVisibility()
     }
 }
 
+#ifdef WITH_XC_AUTOTYPE
 void EditEntryWidget::updateAutoTypeEnabled()
 {
     bool autoTypeEnabled = m_autoTypeUi->enableButton->isChecked();
@@ -1533,6 +1554,7 @@ void EditEntryWidget::applyCurrentAssoc()
 
     m_autoTypeAssoc->update(index.row(), assoc);
 }
+#endif
 
 void EditEntryWidget::showHistoryEntry()
 {
diff --git a/src/gui/entry/EditEntryWidget.h b/src/gui/entry/EditEntryWidget.h
index 89422c0d4d..046f9a0ea7 100644
--- a/src/gui/entry/EditEntryWidget.h
+++ b/src/gui/entry/EditEntryWidget.h
@@ -94,6 +94,7 @@ private slots:
     void updateCurrentAttribute();
     void protectCurrentAttribute(bool state);
     void toggleCurrentAttributeVisibility();
+#ifdef WITH_XC_AUTOTYPE
     void updateAutoTypeEnabled();
     void openAutotypeHelp();
     void insertAutoTypeAssoc();
@@ -101,6 +102,7 @@ private slots:
     void loadCurrentAssoc(const QModelIndex& current);
     void clearCurrentAssoc();
     void applyCurrentAssoc();
+#endif
     void showHistoryEntry();
     void restoreHistoryEntry();
     void deleteHistoryEntry();
@@ -137,7 +139,9 @@ private slots:
     void setupMain();
     void setupAdvanced();
     void setupIcon();
+#ifdef WITH_XC_AUTOTYPE
     void setupAutoType();
+#endif
 #ifdef WITH_XC_BROWSER
     void setupBrowser();
 #endif
@@ -170,7 +174,9 @@ private slots:
 #endif
     const QScopedPointer<Ui::EditEntryWidgetMain> m_mainUi;
     const QScopedPointer<Ui::EditEntryWidgetAdvanced> m_advancedUi;
+#ifdef WITH_XC_AUTOTYPE
     const QScopedPointer<Ui::EditEntryWidgetAutoType> m_autoTypeUi;
+#endif
     const QScopedPointer<Ui::EditEntryWidgetSSHAgent> m_sshAgentUi;
     const QScopedPointer<Ui::EditEntryWidgetHistory> m_historyUi;
     const QScopedPointer<Ui::EditEntryWidgetBrowser> m_browserUi;

There is still one issue:

/usr/bin/ld: libkeepassxc_gui.a(ApplicationSettingsWidget.cpp.o): in function `Ui_ApplicationSettingsWidgetGeneral::setupUi(QWidget*)':
/home/yen/tmp/keepassxc/build/src/keepassxc_gui_autogen/include/ui_ApplicationSettingsWidgetGeneral.h:741: undefined reference to `ShortcutWidget::ShortcutWidget(QWidget*)'

ApplicationSettingsWidgetGeneral.ui uses a widget class in autotype/ [1], and it cannot be simply excluded from the build as that UI file includes both Basic Settings and Auto-Type tabs. A fix may be moving Auto-Type settings to a separate page like Browser Integration or SSH Agent.

[1]

<header>autotype/ShortcutWidget.h</header>

@droidmonkey
Copy link
Member

I have wanted to merge the general and security settings pages for a while and push autotype to a tab like you said, that would be a good change.

@louib
Copy link
Member Author

louib commented Aug 6, 2023

I opened a new PR with only the initial refactoring here. I think this PR is getting a bit too large to review in a single pass, and there's no need to since the first commits are stable and self-contained. I'm going to keep this PR around and rebase once #9701 is merged.

@droidmonkey droidmonkey changed the title Separate GUI sources from core sources. Cleanup code naming conventions and CMake files Aug 6, 2023
@droidmonkey droidmonkey marked this pull request as draft August 6, 2023 19:32
@droidmonkey
Copy link
Member

This PR should be rebased once #11003 is merged. Some of the changes overlap and the other PR is more developed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup high priority 🚨 pr: refactoring Pull request that refactors code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants