View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0017955 | Scribus | Guides | public | 2026-08-29 14:50 | 2026-08-29 14:50 |
| Reporter | qirat | Assigned To | |||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | new | Resolution | open | ||
| Platform | Linux | OS | Fedora Workstation | OS Version | 44 |
| Product Version | 1.7.4.svn | ||||
| Summary | 0017955: [patch] Fix Guide Editing and History | ||||
| Description | #Issue * Follows none. Written against pristine r27791. * Guide Manager rebuilt the complete standard-guide list after edits using `clearHorizontals()` / `clearVerticals()` followed by bulk `add*()`. * This produced incorrect Undo history: * first added guide could create no Undo entry; * later additions could record removal of existing guides; * moves were recorded as 'Remove' actions; * deleting one guide could record removals for unrelated guides; * entering and leaving an unchanged guide spinbox could still create an Undo entry and mark the document modified. * Undo/Redo could therefore restore an incorrect guide set, not just show the wrong action label. #Fix * Apply the actual guide delta instead of clearing/rebuilding the full list. * Use existing `GuideManagerCore` primitives: * `addHorizontal()` / `addVertical()` for additions; * `moveHorizontal()` / `moveVertical()` for edits; * `deleteHorizontal()` / `deleteVertical()` for removals. * Suppress unchanged model commits so no Undo state or modified flag is generated for no-op edits. * Fix the `GuidesModel::insertRow()` reset/0-position edge. * Reject edits that would move a guide onto an already occupied guide position. #Result * Guide Manager now records the same Add/Move/Remove semantics as direct canvas guide operations. * Undo/Redo affects only the guide actually changed. * No-op edits no longer create Action History entries or modify the document state. | ||||
| Tags | #please_test | ||||
| Attached Files | guide-manager-undo-actions-v1.0.patch (8,910 bytes)
Index: scribus/guidesmodel.h
===================================================================
--- scribus/guidesmodel.h (revision 27791)
+++ scribus/guidesmodel.h (working copy)
@@ -42,7 +42,7 @@
// these removeRows() and insertRows() does not handle margin items correctly
// bool removeRows(int row, int count, const QModelIndex & parent = QModelIndex());
// bool insertRows( int row, int count, const QModelIndex & parent = QModelIndex());
- void insertRow();
+ int insertRow();
//! \brief Set new values into the model.
void setValues(const Guides& values);
@@ -58,10 +58,11 @@
signals:
/*! \brief Signal emitted when the user finishes editing one value.
*/
- void valueChanged();
+ void valueChanged(double oldValue, double newValue, bool inserted);
private:
Guides m_values;
+ int m_pendingInsertRow {-1};
int m_docUnitIndex {0};
int m_docUnitDecimals {0};
double m_rule {0.0};
Index: scribus/guidesmodel.cpp
===================================================================
--- scribus/guidesmodel.cpp (revision 27791)
+++ scribus/guidesmodel.cpp (working copy)
@@ -6,6 +6,7 @@
*/
#include <algorithm>
+#include <cmath>
#include <QLocale>
#include "guidesmodel.h"
@@ -57,16 +58,47 @@
bool GuidesModel::setData(const QModelIndex & index, const QVariant & value, int role)
{
- if (role != Qt::EditRole || !index.isValid())
+ if (role != Qt::EditRole || !index.isValid() || index.row() >= m_values.count())
return false;
+
bool ok;
double newVal = value.toDouble(&ok);
if (!ok)
return false;
- m_values[index.row()] = value2pts(newVal, m_docUnitIndex) + m_rule;
+
+ const int row = index.row();
+ const bool inserted = (row == m_pendingInsertRow);
+ const double oldValue = m_values.at(row);
+ const double oldDisplayValue = pts2value(oldValue - m_rule, m_docUnitIndex);
+ const double tolerance = 0.5 * std::pow(10.0, -m_docUnitDecimals);
+
+ if (!inserted && std::abs(newVal - oldDisplayValue) <= tolerance)
+ return true;
+
+ for (int i = 0; i < m_values.count(); ++i)
+ {
+ if (i == row)
+ continue;
+ const double displayValue = pts2value(m_values.at(i) - m_rule, m_docUnitIndex);
+ if (std::abs(newVal - displayValue) <= tolerance)
+ {
+ if (inserted)
+ {
+ beginRemoveRows(QModelIndex(), row, row);
+ m_values.removeAt(row);
+ endRemoveRows();
+ m_pendingInsertRow = -1;
+ }
+ return false;
+ }
+ }
+
+ const double newValue = value2pts(newVal, m_docUnitIndex) + m_rule;
+ m_values[row] = newValue;
+ m_pendingInsertRow = -1;
std::sort(m_values.begin(), m_values.end());
emit dataChanged(index, index);
- emit valueChanged();
+ emit valueChanged(oldValue, newValue, inserted);
return true;
}
@@ -111,21 +143,24 @@
// return true;
// }
-void GuidesModel::insertRow()
+int GuidesModel::insertRow()
{
-// insertRows(rowCount(), 1);
- beginResetModel();
- if (m_values.contains(0.0))
- return;
+ if (m_pendingInsertRow >= 0)
+ return m_pendingInsertRow;
+
+ const int row = m_values.count();
+ beginInsertRows(QModelIndex(), row, row);
m_values.append(0.0);
- std::sort(m_values.begin(), m_values.end());
- endResetModel();
+ endInsertRows();
+ m_pendingInsertRow = row;
+ return row;
}
void GuidesModel::setValues(const Guides& values)
{
beginResetModel();
m_values = values;
+ m_pendingInsertRow = -1;
std::sort(m_values.begin(), m_values.end());
endResetModel();
}
Index: scribus/ui/guidemanager.h
===================================================================
--- scribus/ui/guidemanager.h (revision 27791)
+++ scribus/ui/guidemanager.h (working copy)
@@ -132,8 +132,8 @@
private slots:
//! Wrapper slot for drawGuides()
void forceDrawGuides(const QItemSelection &, const QItemSelection &);
- void verticalModel_valueChanged();
- void horizontalModel_valueChanged();
+ void verticalModel_valueChanged(double oldValue, double newValue, bool inserted);
+ void horizontalModel_valueChanged(double oldValue, double newValue, bool inserted);
void addHorButton_clicked();
void delHorButton_clicked();
void addVerButton_clicked();
Index: scribus/ui/guidemanager.cpp
===================================================================
--- scribus/ui/guidemanager.cpp (revision 27791)
+++ scribus/ui/guidemanager.cpp (working copy)
@@ -88,8 +88,8 @@
connect(lockCheck, SIGNAL(stateChanged(int)), this, SLOT(lockCheck_stateChanged(int)));
- connect(horizontalModel, SIGNAL(valueChanged()), this, SLOT(horizontalModel_valueChanged()));
- connect(verticalModel, SIGNAL(valueChanged()), this, SLOT(verticalModel_valueChanged()));
+ connect(horizontalModel, SIGNAL(valueChanged(double,double,bool)), this, SLOT(horizontalModel_valueChanged(double,double,bool)));
+ connect(verticalModel, SIGNAL(valueChanged(double,double,bool)), this, SLOT(verticalModel_valueChanged(double,double,bool)));
connect(tabWidget, SIGNAL(currentChanged(int)), this, SLOT(tabWidget_currentChanged(int)));
}
@@ -267,50 +267,62 @@
void GuideManager::delHorButton_clicked()
{
const QModelIndexList indexes = horizontalView->selectionModel()->selectedRows(0);
- Guides v;
-
+ if (indexes.isEmpty())
+ return;
+
+ Guides pageGuides = currentPage->guides.horizontals(GuideManagerCore::Standard);
+ bool changed = false;
for (const QModelIndex& ix : indexes)
- // here *must* go EditRole due truncations in DisplayRole
- // see GuidesModel::data()
- v.append(horizontalModel->data(ix, Qt::UserRole).toDouble());
-
- horizontalModel->removeValues(v);
-
- currentPage->guides.clearHorizontals(GuideManagerCore::Standard);
- currentPage->guides.addHorizontals(horizontalModel->values(), GuideManagerCore::Standard);
- drawGuides();
+ {
+ const double guide = horizontalModel->data(ix, Qt::UserRole).toDouble();
+ if (!pageGuides.contains(guide))
+ continue;
+ currentPage->guides.deleteHorizontal(guide, GuideManagerCore::Standard);
+ pageGuides.removeAll(guide);
+ changed = true;
+ }
+ if (!changed)
+ return;
+
+ clearRestoreHorizontalList();
m_doc->changed();
}
void GuideManager::delVerButton_clicked()
{
const QModelIndexList indexes = verticalView->selectionModel()->selectedRows(0);
- Guides v;
-
+ if (indexes.isEmpty())
+ return;
+
+ Guides pageGuides = currentPage->guides.verticals(GuideManagerCore::Standard);
+ bool changed = false;
for (const QModelIndex& ix : indexes)
- // here *must* go EditRole due truncations in DisplayRole
- // see GuidesModel::data()
- v.append(verticalModel->data(ix, Qt::UserRole).toDouble());
-
- verticalModel->removeValues(v);
-
- currentPage->guides.clearVerticals(GuideManagerCore::Standard);
- currentPage->guides.addVerticals(verticalModel->values(), GuideManagerCore::Standard);
- drawGuides();
+ {
+ const double guide = verticalModel->data(ix, Qt::UserRole).toDouble();
+ if (!pageGuides.contains(guide))
+ continue;
+ currentPage->guides.deleteVertical(guide, GuideManagerCore::Standard);
+ pageGuides.removeAll(guide);
+ changed = true;
+ }
+ if (!changed)
+ return;
+
+ clearRestoreVerticalList();
m_doc->changed();
}
void GuideManager::addHorButton_clicked()
{
- horizontalModel->insertRow();
- horizontalView->setCurrentIndex(horizontalModel->index(0, 0, QModelIndex()));
+ const int row = horizontalModel->insertRow();
+ horizontalView->setCurrentIndex(horizontalModel->index(row, 0, QModelIndex()));
horizontalView->edit(horizontalView->currentIndex());
}
void GuideManager::addVerButton_clicked()
{
- verticalModel->insertRow();
- verticalView->setCurrentIndex(verticalModel->index(0, 0, QModelIndex()));
+ const int row = verticalModel->insertRow();
+ verticalView->setCurrentIndex(verticalModel->index(row, 0, QModelIndex()));
verticalView->edit(verticalView->currentIndex());
}
@@ -592,18 +604,22 @@
return guides.getAutoHorizontals(page);
}
-void GuideManager::verticalModel_valueChanged()
-{
- currentPage->guides.clearVerticals(GuideManagerCore::Standard);
- currentPage->guides.addVerticals(verticalModel->values(), GuideManagerCore::Standard);
- drawGuides();
- m_doc->changed();
-}
-
-void GuideManager::horizontalModel_valueChanged()
-{
- currentPage->guides.clearHorizontals(GuideManagerCore::Standard);
- currentPage->guides.addHorizontals(horizontalModel->values(), GuideManagerCore::Standard);
+void GuideManager::verticalModel_valueChanged(double oldValue, double newValue, bool inserted)
+{
+ if (inserted)
+ currentPage->guides.addVertical(newValue, GuideManagerCore::Standard);
+ else
+ currentPage->guides.moveVertical(oldValue, newValue, GuideManagerCore::Standard);
+ drawGuides();
+ m_doc->changed();
+}
+
+void GuideManager::horizontalModel_valueChanged(double oldValue, double newValue, bool inserted)
+{
+ if (inserted)
+ currentPage->guides.addHorizontal(newValue, GuideManagerCore::Standard);
+ else
+ currentPage->guides.moveHorizontal(oldValue, newValue, GuideManagerCore::Standard);
drawGuides();
m_doc->changed();
}
| ||||
| Patch | Yes | ||||