Refactor various aspects of the code

* Prefer using auto
* Reduce nesting in certain places
* Simplify code in main function
This commit is contained in:
Martchus 2024-04-01 13:59:22 +02:00
parent 0f4c30c14f
commit b2d85c7d53
8 changed files with 136 additions and 134 deletions

View File

@ -5,6 +5,7 @@
#include <passwordfile/io/field.h>
#include <passwordfile/io/parsingexception.h>
#include <passwordfile/io/passwordfile.h>
#include <passwordfile/util/openssl.h>
#include <c++utilities/application/commandlineutils.h>
#include <c++utilities/conversion/stringconversion.h>
@ -15,7 +16,9 @@
#endif
#include <algorithm>
#include <cstdlib>
#include <functional>
#include <stdexcept>
using namespace std;
using namespace std::placeholders;
@ -89,32 +92,39 @@ InteractiveCli::InteractiveCli()
, m_modified(false)
, m_quit(false)
{
Util::OpenSsl::init();
CMD_UTILS_START_CONSOLE;
}
void InteractiveCli::run(const string &file)
InteractiveCli::~InteractiveCli()
{
Util::OpenSsl::clean();
}
int InteractiveCli::run(string_view file)
{
if (!file.empty()) {
openFile(file, PasswordFileOpenFlags::Default);
}
string input;
auto input = std::string();
while (!m_quit) {
getline(m_i, input);
std::getline(m_i, input);
if (!input.empty()) {
processCommand(input);
}
}
return EXIT_SUCCESS;
}
void InteractiveCli::processCommand(const string &cmd)
void InteractiveCli::processCommand(const std::string &cmd)
{
#define CMD(value) !paramMissing &&cmd == value
#define CMD2(value1, value2) !paramMissing && (cmd == value1 || cmd == value2)
#define CMD_P(value) !paramMissing &&checkCommand(cmd, value, param, paramMissing)
#define CMD2_P(value1, value2) !paramMissing && (checkCommand(cmd, value1, param, paramMissing) || checkCommand(cmd, value2, param, paramMissing))
string param;
bool paramMissing = false;
auto param = std::string();
auto paramMissing = false;
if (CMD2("quit", "q")) {
quit();
} else if (CMD("q!")) {
@ -175,9 +185,9 @@ void InteractiveCli::processCommand(const string &cmd)
}
}
Entry *InteractiveCli::resolvePath(const string &path)
Entry *InteractiveCli::resolvePath(const std::string &path)
{
auto parts = splitString<vector<string>>(path, "/", EmptyPartsTreat::Merge);
auto parts = splitString<std::vector<std::string>>(path, "/", EmptyPartsTreat::Merge);
bool fromRoot = path.at(0) == '/';
if (fromRoot && parts.empty()) {
return m_file.rootEntry();
@ -213,7 +223,7 @@ Entry *InteractiveCli::resolvePath(const string &path)
}
}
bool InteractiveCli::checkCommand(const string &str, const char *phrase, std::string &param, bool &paramMissing)
bool InteractiveCli::checkCommand(const std::string &str, const char *phrase, std::string &param, bool &paramMissing)
{
for (auto i = str.cbegin(), end = str.cend(); i != end; ++i, ++phrase) {
if (*phrase == 0) {
@ -234,13 +244,13 @@ bool InteractiveCli::checkCommand(const string &str, const char *phrase, std::st
return false;
}
void InteractiveCli::openFile(const string &file, PasswordFileOpenFlags openFlags)
void InteractiveCli::openFile(std::string_view file, PasswordFileOpenFlags openFlags)
{
if (m_file.isOpen()) {
m_o << "file \"" << m_file.path() << "\" currently open; close first" << endl;
return;
}
m_file.setPath(file);
m_file.setPath(std::string(file));
for (;;) {
try {
try {
@ -322,7 +332,7 @@ void InteractiveCli::saveFile()
m_modified = false;
}
void InteractiveCli::createFile(const string &file)
void InteractiveCli::createFile(const std::string &file)
{
if (m_file.isOpen()) {
m_o << "file \"" << m_file.path() << "\" currently open; close first" << endl;
@ -392,7 +402,7 @@ void InteractiveCli::pwd()
m_o << joinStrings(path, "/") << endl;
}
void InteractiveCli::cd(const string &path)
void InteractiveCli::cd(const std::string &path)
{
if (!m_file.isOpen()) {
m_o << "can not change directory; no file open" << endl;
@ -450,7 +460,7 @@ void InteractiveCli::tree()
printEntries(m_currentEntry, 0);
}
void InteractiveCli::makeEntry(EntryType entryType, const string &label)
void InteractiveCli::makeEntry(EntryType entryType, const std::string &label)
{
if (!m_file.isOpen()) {
m_o << "can not make entry; no file open" << endl;
@ -473,7 +483,7 @@ void InteractiveCli::makeEntry(EntryType entryType, const string &label)
}
}
void InteractiveCli::removeEntry(const string &path)
void InteractiveCli::removeEntry(const std::string &path)
{
if (!m_file.isOpen()) {
m_o << "can not remove entry; no file open" << endl;
@ -493,16 +503,16 @@ void InteractiveCli::removeEntry(const string &path)
}
}
void InteractiveCli::renameEntry(const string &path)
void InteractiveCli::renameEntry(const std::string &path)
{
if (!m_file.isOpen()) {
m_o << "can not rename entry; no file open" << endl;
return;
}
if (Entry *entry = resolvePath(path)) {
string label;
auto label = std::string();
m_o << "enter new name: " << endl;
getline(m_i, label);
std::getline(m_i, label);
if (label.empty()) {
m_o << "can not rename; new name is empty" << endl;
} else {
@ -513,16 +523,16 @@ void InteractiveCli::renameEntry(const string &path)
}
}
void InteractiveCli::moveEntry(const string &path)
void InteractiveCli::moveEntry(const std::string &path)
{
if (!m_file.isOpen()) {
m_o << "can not rename entry; no file open" << endl;
return;
}
if (Entry *entry = resolvePath(path)) {
string newParentPath;
auto newParentPath = std::string();
m_o << "enter path of new parent: " << endl;
getline(m_i, newParentPath);
std::getline(m_i, newParentPath);
if (newParentPath.empty()) {
m_o << "can not move; path of new parent is empty" << endl;
} else {
@ -547,7 +557,7 @@ void InteractiveCli::moveEntry(const string &path)
}
}
void InteractiveCli::readField(const string &fieldName)
void InteractiveCli::readField(const std::string &fieldName)
{
if (!m_file.isOpen()) {
m_o << "can not read field; no file open" << endl;
@ -571,7 +581,7 @@ void InteractiveCli::readField(const string &fieldName)
}
}
void InteractiveCli::setField(bool useMuter, const string &fieldName)
void InteractiveCli::setField(bool useMuter, const std::string &fieldName)
{
if (!m_file.isOpen()) {
m_o << "can not set field; no file open" << endl;
@ -581,22 +591,22 @@ void InteractiveCli::setField(bool useMuter, const string &fieldName)
m_o << "can not set field; current entry is no account entry" << endl;
return;
}
vector<Field> &fields = static_cast<AccountEntry *>(m_currentEntry)->fields();
unsigned int valuesFound = 0;
string value;
auto &fields = static_cast<AccountEntry *>(m_currentEntry)->fields();
auto valuesFound = unsigned();
auto value = std::string();
m_o << "enter new value: ";
if (useMuter) {
InputMuter m;
getline(m_i, value);
std::getline(m_i, value);
m_o << endl << "repeat: ";
string repeat;
getline(m_i, repeat);
auto repeat = std::string();
std::getline(m_i, repeat);
if (value != repeat) {
m_o << "values do not match; field has not been altered" << endl;
return;
}
} else {
getline(m_i, value);
std::getline(m_i, value);
}
for (Field &field : fields) {
if (field.name() == fieldName) {
@ -643,7 +653,7 @@ void InteractiveCli::setField(bool useMuter, const string &fieldName)
}
}
void InteractiveCli::removeField(const string &fieldName)
void InteractiveCli::removeField(const std::string &fieldName)
{
if (!m_file.isOpen()) {
m_o << "can not remove field; no file open" << endl;
@ -653,8 +663,8 @@ void InteractiveCli::removeField(const string &fieldName)
m_o << "can not remove field; current entry is no account entry" << endl;
return;
}
vector<Field> &fields = static_cast<AccountEntry *>(m_currentEntry)->fields();
unsigned int valuesFound = 0;
auto &fields = static_cast<AccountEntry *>(m_currentEntry)->fields();
auto valuesFound = unsigned();
for (const Field &field : fields) {
if (field.name() == fieldName) {
++valuesFound;
@ -730,7 +740,7 @@ void InteractiveCli::quit()
}
}
string InteractiveCli::askForPassphrase(bool confirm)
std::string InteractiveCli::askForPassphrase(bool confirm)
{
if (confirm) {
m_o << "enter new passphrase: ";
@ -738,10 +748,10 @@ string InteractiveCli::askForPassphrase(bool confirm)
m_o << "enter passphrase: ";
}
m_o.flush();
string input1;
auto input1 = std::string();
{
InputMuter m;
getline(m_i, input1);
auto m = InputMuter();
std::getline(m_i, input1);
}
m_o << endl;
if (input1.empty()) {
@ -751,15 +761,15 @@ string InteractiveCli::askForPassphrase(bool confirm)
if (confirm) {
m_o << "confirm new passphrase: ";
m_o.flush();
string input2;
auto input2 = std::string();
{
InputMuter m;
getline(m_i, input2);
auto m = InputMuter();
std::getline(m_i, input2);
}
m_o << endl;
if (input1 != input2) {
m_o << "phrases do not match" << endl;
throw runtime_error("confirmation failed");
throw std::runtime_error("confirmation failed");
}
}
return input1;

View File

@ -12,7 +12,7 @@
#include <istream>
#include <ostream>
#include <string>
#include <vector>
#include <string_view>
namespace Io {
class Entry;
@ -23,7 +23,7 @@ namespace Cli {
class InputMuter {
public:
InputMuter();
explicit InputMuter();
~InputMuter();
private:
@ -39,9 +39,10 @@ void clearConsole();
class InteractiveCli {
public:
InteractiveCli();
void run(const std::string &file = std::string());
void openFile(const std::string &file, Io::PasswordFileOpenFlags openFlags);
explicit InteractiveCli();
~InteractiveCli();
int run(std::string_view file);
void openFile(std::string_view file, Io::PasswordFileOpenFlags openFlags);
void closeFile();
void saveFile();
void createFile(const std::string &file);

View File

@ -7,7 +7,7 @@ namespace QtGui {
class FieldDelegate : public QStyledItemDelegate {
public:
FieldDelegate(QObject *parent = nullptr);
explicit FieldDelegate(QObject *parent = nullptr);
void setEditorData(QWidget *editor, const QModelIndex &index) const override;
};

View File

@ -31,7 +31,8 @@ int runWidgetsGui(int argc, char *argv[], const QtConfigArguments &qtConfigArgs,
OpenSsl::init();
// init application
QApplication application(argc, argv);
auto application = QApplication(argc, argv);
QObject::connect(&application, &QCoreApplication::aboutToQuit, &OpenSsl::clean);
// restore Qt settings
auto qtSettings = QtSettings();
@ -56,7 +57,6 @@ int runWidgetsGui(int argc, char *argv[], const QtConfigArguments &qtConfigArgs,
}
// start event loop
QObject::connect(&application, &QCoreApplication::aboutToQuit, &OpenSsl::clean);
auto res = application.exec();
// save settings to disk

View File

@ -17,7 +17,7 @@ class StackSupport {
friend class StackAbsorper;
public:
StackSupport(QUndoStack *undoStack = nullptr);
explicit StackSupport(QUndoStack *undoStack = nullptr);
protected:
QUndoStack *undoStack();
@ -68,7 +68,7 @@ inline void StackSupport::clearUndoStack()
*/
class StackAbsorper {
public:
StackAbsorper(StackSupport *supported);
explicit StackAbsorper(StackSupport *supported);
~StackAbsorper();
QUndoStack *stack();

View File

@ -62,6 +62,15 @@ void CustomUndoCommand::undo()
* \brief Sets the value for the specified index and role in the specified field model.
*/
/// \cond
static QString getFieldName(FieldModel *model, int row, const QModelIndex &index)
{
return model->index(row, 0, index.parent()).data().toString();
}
/// \endcond
/*!
* \brief Constructs a new command.
*/
@ -75,7 +84,6 @@ FieldModelSetValueCommand::FieldModelSetValueCommand(FieldModel *model, const QM
, m_oldValue(model->data(index, role))
, m_role(role)
{
QString fieldName = model->index(m_row, 0, index.parent()).data().toString();
switch (role) {
case Qt::DisplayRole:
case Qt::EditRole:
@ -88,16 +96,16 @@ FieldModelSetValueCommand::FieldModelSetValueCommand(FieldModel *model, const QM
}
break;
case 1:
if (fieldName.isEmpty()) {
setText(QApplication::translate("undocommands", "setting value of empty field"));
} else {
if (const auto fieldName = getFieldName(model, m_row, index); !fieldName.isEmpty()) {
setText(QApplication::translate("undocommands", "setting value of »%1« field").arg(fieldName));
} else {
setText(QApplication::translate("undocommands", "setting value of empty field"));
}
break;
}
break;
case FieldTypeRole:
setText(QApplication::translate("undocommands", "setting type of »%1« field").arg(fieldName));
setText(QApplication::translate("undocommands", "setting type of »%1« field").arg(getFieldName(model, m_row, index)));
break;
default:
setText(QApplication::translate("undocommands", "setting field property in row »%1«").arg(m_row + 1));
@ -316,7 +324,7 @@ EntryModelModifyRowsCommand::~EntryModelModifyRowsCommand()
*/
bool EntryModelModifyRowsCommand::insert()
{
if (Entry *parentEntry = entryFromPathCpy(m_model, m_parentPath)) {
if (Entry *const parentEntry = entryFromPathCpy(m_model, m_parentPath)) {
if (m_model->insertEntries(m_row, m_model->index(parentEntry), m_values)) {
m_values.clear();
return true;
@ -334,7 +342,7 @@ bool EntryModelModifyRowsCommand::insert()
*/
bool EntryModelModifyRowsCommand::remove()
{
if (Entry *parentEntry = entryFromPathCpy(m_model, m_parentPath)) {
if (Entry *const parentEntry = entryFromPathCpy(m_model, m_parentPath)) {
m_values = m_model->takeEntries(m_row, m_count, m_model->index(parentEntry));
return !m_values.isEmpty();
}
@ -433,29 +441,28 @@ bool EntryModelMoveRowsCommand::internalRedo()
bool EntryModelMoveRowsCommand::internalUndo()
{
if (m_count) {
Entry *sourceParentEntry = entryFromPathCpy(m_model, m_sourceParentPath);
Entry *destParentEntry = entryFromPathCpy(m_model, m_destParentPath);
if (sourceParentEntry && destParentEntry) {
int sourceRow = m_destChild;
int destChild = m_sourceRow;
// moves within the same parent needs special consideration
if (sourceParentEntry == destParentEntry) {
// move entry down
if (m_sourceRow < m_destChild) {
sourceRow -= m_count;
// move entry up
} else if (m_sourceRow > m_destChild) {
destChild += m_count;
// keep entry were it is
} else {
return true;
}
}
return m_model->moveRows(m_model->index(destParentEntry), sourceRow, m_count, m_model->index(sourceParentEntry), destChild);
}
return false;
if (!m_count) {
return true;
}
return true;
Entry *const sourceParentEntry = entryFromPathCpy(m_model, m_sourceParentPath);
Entry *const destParentEntry = entryFromPathCpy(m_model, m_destParentPath);
if (sourceParentEntry && destParentEntry) {
auto sourceRow = m_destChild, destChild = m_sourceRow;
// moves within the same parent needs special consideration
if (sourceParentEntry == destParentEntry) {
// move entry down
if (m_sourceRow < m_destChild) {
sourceRow -= m_count;
// move entry up
} else if (m_sourceRow > m_destChild) {
destChild += m_count;
// keep entry were it is
} else {
return true;
}
}
return m_model->moveRows(m_model->index(destParentEntry), sourceRow, m_count, m_model->index(sourceParentEntry), destChild);
}
return false;
}
} // namespace QtGui

View File

@ -1,4 +1,5 @@
#include "./cli/cli.h"
#ifdef PASSWORD_MANAGER_GUI_QTWIDGETS
#include "./gui/initiategui.h"
#endif
@ -9,13 +10,12 @@
#include "resources/config.h"
#include "resources/qtconfig.h"
#include <passwordfile/util/openssl.h>
#include <c++utilities/application/argumentparser.h>
#include <c++utilities/application/commandlineutils.h>
#include <c++utilities/misc/parseerror.h>
#if defined(PASSWORD_MANAGER_GUI_QTWIDGETS) || defined(PASSWORD_MANAGER_GUI_QTQUICK)
#define PASSWORD_MANAGER_GUI_QTWIDGETS_OR_QTQUICK
#include <QCoreApplication>
#include <QString>
#include <qtutilities/resources/qtconfigarguments.h>
@ -24,33 +24,39 @@ ENABLE_QT_RESOURCES_OF_STATIC_DEPENDENCIES
#include <c++utilities/application/fakeqtconfigarguments.h>
#endif
#include <cstdlib>
#include <iostream>
// force (preferably Qt Quick) GUI under Android
#ifdef Q_OS_ANDROID
#if defined(PASSWORD_MANAGER_GUI_QTWIDGETS) || defined(PASSWORD_MANAGER_GUI_QTQUICK)
#ifdef PASSWORD_MANAGER_GUI_QTWIDGETS_OR_QTQUICK
#define PASSWORD_MANAGER_FORCE_GUI
#else
#error "Must build at least one kind of GUI under Android."
#error "Must configure building at least one kind of GUI under Android."
#endif
#endif
using namespace std;
using namespace CppUtilities;
using namespace Util;
#ifndef PASSWORD_MANAGER_FORCE_GUI
static int fail(std::string_view error)
{
CMD_UTILS_START_CONSOLE;
std::cerr << error << std::endl;
return EXIT_FAILURE;
}
#endif
int main(int argc, char *argv[])
{
CMD_UTILS_CONVERT_ARGS_TO_UTF8;
SET_APPLICATION_INFO;
QT_CONFIG_ARGUMENTS qtConfigArgs;
int returnCode = 0;
// parse CLI arguments
auto qtConfigArgs = QT_CONFIG_ARGUMENTS();
#ifndef PASSWORD_MANAGER_FORCE_GUI
// setup argument parser
ArgumentParser parser;
// file argument
Argument fileArg("file", 'f', "specifies the file to be opened (or created when using --modify)");
auto parser = ArgumentParser();
auto fileArg = Argument("file", 'f', "specifies the file to be opened (or created when using --modify)");
fileArg.setValueNames({ "path" });
fileArg.setRequiredValueCount(1);
fileArg.setCombinable(true);
@ -58,63 +64,44 @@ int main(int argc, char *argv[])
fileArg.setImplicit(true);
qtConfigArgs.qtWidgetsGuiArg().addSubArgument(&fileArg);
qtConfigArgs.qtQuickGuiArg().addSubArgument(&fileArg);
// cli argument
Argument cliArg("interactive-cli", 'i', "starts the interactive command line interface");
auto cliArg = Argument("interactive-cli", 'i', "starts the interactive command line interface");
cliArg.setDenotesOperation(true);
cliArg.setSubArguments({ &fileArg });
// help argument
HelpArgument helpArg(parser);
auto helpArg = HelpArgument(parser);
parser.setMainArguments({ &qtConfigArgs.qtWidgetsGuiArg(), &qtConfigArgs.qtQuickGuiArg(), &cliArg, &helpArg });
// parse the specified arguments
parser.parseArgs(argc, argv);
#endif
#ifndef PASSWORD_MANAGER_FORCE_GUI
// start either interactive CLI or GUI
// run CLI if CLI-argument is present
if (cliArg.isPresent()) {
// init OpenSSL
OpenSsl::init();
return Cli::InteractiveCli().run(fileArg.isPresent() ? std::string(fileArg.firstValue()) : std::string());
}
Cli::InteractiveCli cli;
if (fileArg.isPresent()) {
cli.run(fileArg.firstValue());
} else {
cli.run();
}
// clean OpenSSL
OpenSsl::clean();
} else if (qtConfigArgs.areQtGuiArgsPresent()) {
#if defined(PASSWORD_MANAGER_GUI_QTWIDGETS) || defined(PASSWORD_MANAGER_GUI_QTQUICK)
const auto file(fileArg.isPresent() ? QString::fromLocal8Bit(fileArg.firstValue()) : QString());
// run GUI depending on which GUI-argument is present
if (qtConfigArgs.areQtGuiArgsPresent()) {
#ifdef PASSWORD_MANAGER_GUI_QTWIDGETS_OR_QTQUICK
const auto file = fileArg.isPresent() ? QString::fromLocal8Bit(fileArg.firstValue()) : QString();
#endif
if (qtConfigArgs.qtWidgetsGuiArg().isPresent()) {
#ifdef PASSWORD_MANAGER_GUI_QTWIDGETS
returnCode = QtGui::runWidgetsGui(argc, argv, qtConfigArgs, file);
return QtGui::runWidgetsGui(argc, argv, qtConfigArgs, file);
#else
CMD_UTILS_START_CONSOLE;
cerr << "The application has not been built with Qt widgets support." << endl;
return fail("The application has not been built with Qt Widgets GUI support.");
#endif
} else if (qtConfigArgs.qtQuickGuiArg().isPresent()) {
#ifdef PASSWORD_MANAGER_GUI_QTQUICK
returnCode = QtGui::runQuickGui(argc, argv, qtConfigArgs, file);
return QtGui::runQuickGui(argc, argv, qtConfigArgs, file);
#else
CMD_UTILS_START_CONSOLE;
cerr << "The application has not been built with Qt quick support." << endl;
return fail("The application has not been built with Qt Quick GUI support.");
#endif
} else {
CMD_UTILS_START_CONSOLE;
cerr << "See --help for usage." << endl;
}
}
return fail("See --help for usage.");
#else // PASSWORD_MANAGER_FORCE_GUI
#ifdef PASSWORD_MANAGER_GUI_QTQUICK
returnCode = QtGui::runQuickGui(argc, argv, qtConfigArgs, QString());
return QtGui::runQuickGui(argc, argv, qtConfigArgs, QString());
#else
returnCode = QtGui::runWidgetsGui(argc, argv, qtConfigArgs, QString());
return QtGui::runWidgetsGui(argc, argv, qtConfigArgs, QString());
#endif
#endif
return returnCode;
}

View File

@ -347,7 +347,7 @@ bool FieldModel::moveRows(const QModelIndex &sourceParent, int sourceRow, int co
// reserve space for temporary copies (FIXME: possible to avoid this?)
m_fields->reserve(m_fields->size() + static_cast<std::size_t>(count));
auto tmp = vector<Io::Field>(static_cast<std::size_t>(count));
auto tmp = std::vector<Io::Field>(static_cast<std::size_t>(count));
// move rows to temporary array
std::move(m_fields->begin() + sourceRow, m_fields->begin() + sourceRow + count, tmp.begin());
// erase slots of rows to be moved
@ -394,10 +394,7 @@ QMimeData *FieldModel::mimeData(const QModelIndexList &indices) const
*/
const Field *FieldModel::field(size_t row) const
{
if (m_fields && row < m_fields->size()) {
return &(*m_fields)[row];
}
return nullptr;
return m_fields && row < m_fields->size() ? &(*m_fields)[row] : nullptr;
}
} // namespace QtGui