View Issue Details

IDProjectCategoryView StatusLast Update
0017955ScribusGuidespublic2026-08-29 14:50
Reporterqirat Assigned To 
PrioritynormalSeverityminorReproducibilityalways
Status newResolutionopen 
PlatformLinuxOSFedora WorkstationOS Version44
Product Version1.7.4.svn 
Summary0017955: [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();
 }
PatchYes

Activities

Issue History

Date Modified Username Field Change
2026-08-29 14:50 qirat New Issue
2026-08-29 14:50 qirat Tag Attached: #please_test
2026-08-29 14:50 qirat File Added: guide-manager-undo-actions-v1.0.patch