From 1d339a8b27cefda89d0f3ff00c312ef9c36ca2bb Mon Sep 17 00:00:00 2001 From: Kellermorph Date: Wed, 30 Sep 2026 11:01:31 +0200 Subject: [PATCH] Settings: only write the prefix rows that were edited Review findings on the prefix editor: - OK without touching anything changed the file: every row was written back, so an explicit , which reads as an empty field and cancels the inheritance, was dropped and that folder started inheriting its parent's prefix again. A row is now written only when it was edited or no longer holds what the file has, and the two kinds of empty field look different: hasPrefix() tells an explicit from a missing one, which is what the field hint shows. - load() copied a broken file aside before the user had chosen anything, so every "Corriger le fichier" left a .bak behind while the message said nothing had been modified. The copy is now made by save(), right before the file is replaced: repairing or cancelling leaves nothing behind, and a copy that cannot be made stops the write rather than destroying the only copy. - The button followed the combo box only for "Parcourir...": put back on "Par defaut" it fell back to the previously saved path instead of the default one. The directory of the entry being displayed is now used, "Par defaut" being dataDir()/elements/. --- sources/ElementsCollection/qetlabelsfile.cpp | 70 +++++++++++++------ sources/ElementsCollection/qetlabelsfile.h | 7 +- .../configpage/generalconfigurationpage.cpp | 23 +++--- sources/ui/prefixconfigurationdialog.cpp | 40 ++++++++++- 4 files changed, 104 insertions(+), 36 deletions(-) diff --git a/sources/ElementsCollection/qetlabelsfile.cpp b/sources/ElementsCollection/qetlabelsfile.cpp index de09f8239..6c2fdd357 100644 --- a/sources/ElementsCollection/qetlabelsfile.cpp +++ b/sources/ElementsCollection/qetlabelsfile.cpp @@ -207,16 +207,13 @@ bool QetLabelsFile::parse(QIODevice &device, QDomDocument &document, QString *re - when the file does not exist yet, an empty \ document is kept in memory and will only be created on the first save() - when the file exists but cannot be read, is not well formed or does - not have \ as its root element, a copy of it is kept as a - backup (@see backupPath()) and an empty document is used instead, - so the broken file is replaced only if the caller saves. isBroken() - and brokenReason() tell what is wrong with it, so the caller can - offer repairing the file rather than silently discarding its - content. - @return false when no collection directory was given, or when the file - is broken and could not be backed up : rebuilding it then would - destroy the only copy of it, so errorString() tells why and - nothing is loaded. + not have \ as its root element, nothing is written : an + empty document is used instead, and a copy of the unusable file is + only made by save(), right before it is replaced. isBroken() and + brokenReason() tell what is wrong with it, so the caller can offer + repairing the file rather than silently discarding its content. + @return false only when no collection directory was given. It is + save() that reports a file it could not copy aside. */ bool QetLabelsFile::load(const QString &collection_dir) { @@ -263,19 +260,11 @@ bool QetLabelsFile::load(const QString &collection_dir) if (well_formed) { m_document = document; } else { - //The file is unreadable or malformed : keep a copy of it, then - //work on an empty document. m_force_save makes sure the broken - //file is replaced as soon as the caller validates, even though - //the empty document we start from serializes just fine. - m_backup_path = backupBrokenFile(); - if (m_backup_path.isEmpty()) { - //Without a backup, rebuilding would destroy the only copy - //of the file : stop here instead of letting the caller - //overwrite it. - m_error = tr("Le fichier %1 est endommagé (%2) mais n'a pas pu être sauvegardé :\nrien n'a été modifié.") - .arg(m_file_path, m_broken_reason); - return false; - } + //The file is unreadable or malformed : leave it alone and work + //on an empty document. m_force_save makes sure the file is + //replaced as soon as the caller validates, even though the + //empty document we start from serializes just fine - and it is + //also what tells save() to copy the file aside first. m_broken = true; createEmptyDocument(); m_force_save = true; @@ -356,6 +345,27 @@ QString QetLabelsFile::prefix(const QStringList &relative_path) const return value.isNull() ? QString() : value; } +/** + @brief QetLabelsFile::hasPrefix + @return true when @a relative_path owns a \ child of its own, + even an empty one. prefix() alone cannot tell that apart from a + category without any \ : both give it no value, while an + empty \ cancels the inheritance where a category + without one inherits. +*/ +bool QetLabelsFile::hasPrefix(const QStringList &relative_path) const +{ + QDomElement node = m_document.documentElement(); + for (const QString &name : relative_path) { + node = directChildCategory(node, name); + if (node.isNull()) { + return false; + } + } + + return !node.firstChildElement(QStringLiteral("prefix")).isNull(); +} + /** @brief QetLabelsFile::directChildCategory @return the \ child of @a parent named @a name, or a null @@ -617,6 +627,20 @@ bool QetLabelsFile::save() return false; } + if (m_force_save && m_file_exists) { + //The file on disk could not be read : copy it aside before it + //is replaced. Doing it here, and not when it is loaded, means + //no copy is left behind when the user chooses to repair the + //file instead of rebuilding it - or simply changes their mind + //and cancels. + m_backup_path = backupBrokenFile(); + if (m_backup_path.isEmpty()) { + m_error = tr("Le fichier %1 n'a pas pu être copié à côté avant d'être remplacé :\nrien n'a été modifié.") + .arg(m_file_path); + return false; + } + } + QSaveFile file(m_file_path); if (!file.open(QIODevice::WriteOnly | QIODevice::Text)) { m_error = file.errorString(); diff --git a/sources/ElementsCollection/qetlabelsfile.h b/sources/ElementsCollection/qetlabelsfile.h index dcf6345b6..2030c2db5 100644 --- a/sources/ElementsCollection/qetlabelsfile.h +++ b/sources/ElementsCollection/qetlabelsfile.h @@ -61,6 +61,7 @@ class QetLabelsFile bool load(const QString &collection_dir); QString prefix(const QStringList &relative_path) const; + bool hasPrefix(const QStringList &relative_path) const; QStringList orphanPaths(const QList &folders) const; void ensureStructure(const QList &folders); void setPrefix(const QStringList &relative_path, const QString &prefix); @@ -70,8 +71,10 @@ class QetLabelsFile QString filePath() const {return m_file_path;} QString backupPath() const {return m_backup_path;} QString errorString() const {return m_error;} - ///true when the existing file was found unusable and had to - ///be backed up before an empty document was used instead + ///true when the existing file was found unusable and an + ///empty document is used instead. Its copy is only made by + ///save(), right before the file is replaced, so repairing + ///the file instead of rebuilding it leaves no copy behind bool isBroken() const {return m_broken;} ///what exactly is wrong with that file (line and column of ///the syntax error for instance), so the caller can tell the diff --git a/sources/ui/configpage/generalconfigurationpage.cpp b/sources/ui/configpage/generalconfigurationpage.cpp index fe4ae99a6..68b86a43a 100644 --- a/sources/ui/configpage/generalconfigurationpage.cpp +++ b/sources/ui/configpage/generalconfigurationpage.cpp @@ -650,13 +650,20 @@ void GeneralConfigurationPage::on_m_user_macros_path_cb_currentIndexChanged(int */ void GeneralConfigurationPage::on_m_prefix_pb_clicked() { - //The directory currently shown in the combo is the one this page - //displays, even when it has not been applied yet, while - //QETApp::customElementsDir() still returns the previously saved - //path : follow what the user sees. + //The directory the page displays, even when the change has not + //been applied yet : QETApp::customElementsDir() still answers with + //the previously saved path, which is not what is shown when the + //combo has been put back on "Par defaut". QString directory; - if (ui->m_custom_elmt_path_cb->currentIndex() == 1) { + switch (ui->m_custom_elmt_path_cb->currentIndex()) { + case 1: //"Parcourir..." : the item itself holds the chosen path directory = ui->m_custom_elmt_path_cb->itemData(1, Qt::DisplayRole).toString(); + break; + case 0: //"Par defaut" : where a default custom collection lives + directory = QETApp::dataDir() + QStringLiteral("/elements/"); + break; + default: + break; } if (directory.isEmpty()) { directory = QETApp::customElementsDir(); @@ -704,9 +711,9 @@ void GeneralConfigurationPage::on_m_prefix_pb_clicked() "Ouvrez le fichier dans un éditeur de texte à l'endroit indiqué, " "corrigez-le puis relancez cette commande.\n\n" "« Reconstruire » : l'arborescence des dossiers est recréée, " - "mais tous les préfixes actuels sont perdus.")); - box.setDetailedText(tr("Fichier : %1\nCopie conservée : %2") - .arg(labels.filePath(), labels.backupPath())); + "mais tous les préfixes actuels sont perdus. Le fichier actuel " + "est conservé sous le nom qet_labels.xml.bak avant d'être remplacé.")); + box.setDetailedText(tr("Fichier : %1").arg(labels.filePath())); box.exec(); if (box.clickedButton() != rebuild_button) { return; diff --git a/sources/ui/prefixconfigurationdialog.cpp b/sources/ui/prefixconfigurationdialog.cpp index 6df3b751d..37eee1608 100644 --- a/sources/ui/prefixconfigurationdialog.cpp +++ b/sources/ui/prefixconfigurationdialog.cpp @@ -155,10 +155,27 @@ void PrefixConfigurationDialog::buildTree() continue; } parent_item->setData(0, Qt::UserRole, folder); + //What the file holds for that folder, kept to tell a row the + //user touched from one they left alone (@see accept()) + parent_item->setData(1, Qt::UserRole, m_labels.prefix(folder).trimmed()); - auto *edit = new QLineEdit(m_labels.prefix(folder), m_tree); + auto *edit = new QLineEdit(m_labels.prefix(folder).trimmed(), m_tree); edit->setClearButtonEnabled(true); - edit->setPlaceholderText(tr("hériter du dossier parent", "placeholder of an empty prefix field")); + //A folder whose file holds an explicit shows an empty + //field too, but does not inherit : say so instead of promising + //an inheritance that will not happen + edit->setPlaceholderText(m_labels.hasPrefix(folder) + ? tr("aucun préfixe : n'hérite pas du parent", + "placeholder of an empty prefix field whose folder explicitly has no prefix, which cancels the inheritance") + : tr("hériter du dossier parent", "placeholder of an empty prefix field")); + connect(edit, &QLineEdit::textEdited, this, [parent_item, edit]() { + //Once the user has typed in the field, whatever it holds + //when OK is pressed is what the folder gets - an emptied + //field then means "inherit" again, even when the file had + //an explicit + parent_item->setData(1, Qt::UserRole + 1, true); + edit->setPlaceholderText(tr("hériter du dossier parent", "placeholder of an empty prefix field")); + }); edit->installEventFilter(this); m_tree->setItemWidget(parent_item, 1, edit); } @@ -184,7 +201,17 @@ void PrefixConfigurationDialog::accept() if (edit == nullptr) { continue; } - m_labels.setPrefix(item->data(0, Qt::UserRole).toStringList(), edit->text().trimmed()); + const QString text = edit->text().trimmed(); + //A row the user did not touch is left exactly as the file has + //it. Writing every field back would turn an explicit + //, which shows empty and cancels the inheritance, + //into no at all, and that folder would silently start + //inheriting its parent's prefix again. + if (!item->data(1, Qt::UserRole + 1).toBool() + && text == item->data(1, Qt::UserRole).toString()) { + continue; + } + m_labels.setPrefix(item->data(0, Qt::UserRole).toStringList(), text); } if (m_remove_orphans) { @@ -199,6 +226,13 @@ void PrefixConfigurationDialog::accept() return; } + if (!m_labels.backupPath().isEmpty()) { + QMessageBox::information(this, + tr("Fichier endommagé remplacé"), + tr("Le fichier %1 était illisible : il a été remplacé.\nSa copie a été conservée sous :\n%2") + .arg(m_labels.filePath(), m_labels.backupPath())); + } + QDialog::accept(); }