Fixing undo operations of graphs and images. Changing undo names.

sources/QPropertyUndoCommand/qpropertyundocommand.cpp

@@ -128,6 +128,18 @@ bool QPropertyUndoCommand::mergeWith(const QUndoCommand *other)
 	QPropertyUndoCommand const *undo = static_cast<const QPropertyUndoCommand *>(other);
 	if (m_object != undo->m_object
 			|| m_property_name != undo->m_property_name) return false;
+	// Same object and property name alone isn't enough: two entirely
+	// separate, deliberate actions (say, cropping an image and then
+	// mirroring it) both go through the same "pixmap" property and
+	// would otherwise silently coalesce into one undo entry, carrying
+	// only the first action's label -- the second vanishes from the
+	// undo list with no way to undo just it. Legitimate merging (a
+	// slider or spinbox pushing one command per tick while being
+	// dragged, e.g. ArcEditor's angle editors) always reuses the exact
+	// same text() across the whole sequence, so requiring a match here
+	// keeps that working unchanged while refusing to merge anything
+	// that isn't actually a continuation of the same action.
+	if (text() != other->text()) return false;
 	m_new_value = undo->m_new_value;
 	return true;
 }

sources/qet.cpp

@@ -297,11 +297,19 @@ QString QET::ElementsAndConductorsSentence(
 
 	if (images_count) {
 		if (!text.isEmpty()) text += ", ";
-		text += QObject::tr(
-			"%n image(s)",
-			"part of a sentence listing the content of a diagram",
-			images_count
-		);
+		// Qt's %n only selects a grammatical singular/plural form (the
+		// "(s)" convention used by every other count here) -- it never
+		// spells the number out as a word, so getting "une image"
+		// instead of the literal "1 image" for the single-item case
+		// means handling that count outside %n entirely, with its own
+		// fixed string.
+		text += images_count == 1
+				? QObject::tr("une image", "part of a sentence listing the content of a diagram")
+				: QObject::tr(
+					"%n images",
+					"part of a sentence listing the content of a diagram",
+					images_count
+				);
 	}
 
 	if (shapes_count) {

sources/qetgraphicsitem/diagramimageitem.cpp

@@ -815,11 +815,16 @@ void DiagramImageItem::handlerMouseReleaseEvent(int index)
 					else
 						undo = new QPropertyUndoCommand(this, "scaleFactorY", m_original_transform.scaleY, m_transform.scaleY);
 				}
+				if (undo)
+					undo->setText(tr("Redimensionner une image"));
 				break;
 
 			case HandleRole::Rotate:
 				if (!qFuzzyCompare(m_transform.rotation, m_original_transform.rotation))
+				{
 					undo = new QPropertyUndoCommand(this, "rotationAngle", m_original_transform.rotation, m_transform.rotation);
+					undo->setText(tr("Faire pivoter une image"));
+				}
 				break;
 
 			case HandleRole::SkewEdge:
@@ -827,6 +832,8 @@ void DiagramImageItem::handlerMouseReleaseEvent(int index)
 					undo = new QPropertyUndoCommand(this, "skewX", m_original_transform.skewX, m_transform.skewX);
 				else if (!qFuzzyCompare(m_transform.skewY, m_original_transform.skewY))
 					undo = new QPropertyUndoCommand(this, "skewY", m_original_transform.skewY, m_transform.skewY);
+				if (undo)
+					undo->setText(tr("Incliner une image"));
 				break;
 
 			case HandleRole::Pivot:
@@ -841,8 +848,28 @@ void DiagramImageItem::handlerMouseReleaseEvent(int index)
 
 		if (undo)
 		{
+			// Defensive fallback only -- every role above now sets its
+			// own, distinct label directly (Resize/Rotate/SkewEdge used
+			// to all fall through to this same generic text, making the
+			// undo list unable to tell three completely different edits
+			// apart); this only still matters if some future role is
+			// ever added without setting one of its own.
 			if (undo->text().isEmpty())
 				undo->setText(tr("Modifier une image"));
+
+			// Every push here is one complete, finished gesture (press,
+			// drag, release) -- never a continuation of an earlier one.
+			// QPropertyUndoCommand::mergeWith() already treats any
+			// command with children as never mergeable; a dummy child
+			// guarantees that here regardless of which role produced
+			// undo. Without this, a single-property change (Rotate and
+			// SkewEdge always are; Resize sometimes is, when only one
+			// axis actually changed) would silently coalesce into
+			// whatever identically-labelled edit came right before it
+			// -- even after a deselect/reselect proved they were two
+			// separate actions, since the shared label alone is
+			// otherwise indistinguishable from a genuine continuation.
+			new QUndoCommand(undo);
 			diagram()->undoStack().push(undo);
 		}
 	}
@@ -1635,6 +1662,15 @@ void DiagramImageItem::replaceImage()
 
 	auto *undo = new QPropertyUndoCommand(this, "pixmap", oldPixmap, newPixmap);
 	undo->setText(tr("Remplacer une image"));
+	// Every call here is a separate, deliberate menu action with no
+	// compound child of its own (unlike crop(), which always chains a
+	// pos/rawPivot change and is naturally immune) -- two of them in a
+	// row would carry the exact same object, property, and text, which
+	// is indistinguishable from a legitimate merge to
+	// QPropertyUndoCommand::mergeWith(). A dummy child (already treated
+	// as "never merge" by that check) keeps each one its own, separate
+	// undo step regardless.
+	new QUndoCommand(undo);
 	diagram()->undoStack().push(undo);
 }
 
@@ -1678,6 +1714,11 @@ void DiagramImageItem::mirror(bool horizontal)
 
 	auto *undo = new QPropertyUndoCommand(this, "pixmap", oldPixmap, newPixmap);
 	undo->setText(horizontal ? tr("Miroir horizontal d'une image") : tr("Miroir vertical d'une image"));
+	// See replaceImage()'s identical comment: a separate, deliberate
+	// action with no compound child of its own, so a dummy one is
+	// needed to stop two consecutive same-direction mirrors (identical
+	// object, property, and text) from silently merging into one.
+	new QUndoCommand(undo);
 	diagram()->undoStack().push(undo);
 }
 
@@ -1727,6 +1768,11 @@ void DiagramImageItem::setTransparentColor()
 
 	auto *undo = new QPropertyUndoCommand(this, "pixmap", oldPixmap, newPixmap);
 	undo->setText(tr("Définir une couleur transparente"));
+	// See replaceImage()'s identical comment: a separate, deliberate
+	// action with no compound child of its own, so a dummy one is
+	// needed to stop two consecutive transparency edits (identical
+	// object, property, and text) from silently merging into one.
+	new QUndoCommand(undo);
 	diagram()->undoStack().push(undo);
 }
 

sources/qetgraphicsitem/qetshapeitem.cpp

@@ -2477,6 +2477,8 @@ void QetShapeItem::handlerMousePressEvent(int handlerIndex)
 	m_old_polygon = m_polygon;
 	m_old_xRadius = m_xRadius;
 	m_old_yRadius = m_yRadius;
+	m_old_startAngle = m_startAngle;
+	m_old_endAngle = m_endAngle;
 	m_old_transform = m_transform;
 	m_old_pos = pos();
 	m_old_nodes = m_nodes;
@@ -2555,11 +2557,16 @@ void QetShapeItem::handlerMouseReleaseEvent(int handlerIndex)
 			{
 				undo = new QPropertyUndoCommand(this, "rect", QRectF(m_old_P1, m_old_P2), QRectF(m_P1, m_P2).normalized());
 			}
+			if (undo)
+				undo->setText(tr("Redimensionner %1").arg(name()));
 			break;
 
 		case HandleRole::Rotate:
 			if (!qFuzzyCompare(m_transform.rotation, m_old_transform.rotation))
+			{
 				undo = new QPropertyUndoCommand(this, "rotation", m_old_transform.rotation, m_transform.rotation);
+				undo->setText(tr("Faire pivoter %1").arg(name()));
+			}
 			break;
 
 		case HandleRole::SkewEdge:
@@ -2567,6 +2574,8 @@ void QetShapeItem::handlerMouseReleaseEvent(int handlerIndex)
 				undo = new QPropertyUndoCommand(this, "skewX", m_old_transform.skewX, m_transform.skewX);
 			else if (!qFuzzyCompare(m_transform.skewY, m_old_transform.skewY))
 				undo = new QPropertyUndoCommand(this, "skewY", m_old_transform.skewY, m_transform.skewY);
+			if (undo)
+				undo->setText(tr("Incliner %1").arg(name()));
 			break;
 
 		case HandleRole::Pivot:
@@ -2583,20 +2592,37 @@ void QetShapeItem::handlerMouseReleaseEvent(int handlerIndex)
 			{
 				undo = new QPropertyUndoCommand(this, "xRadius", m_old_xRadius, m_xRadius);
 				new QPropertyUndoCommand(this, "yRadius", m_old_yRadius, m_yRadius, undo);
+				undo->setText(tr("Arrondir les coins d'%1").arg(name()));
 			}
 			break;
 
 		case HandleRole::ArcEndpoint:
-			// startAngle/endAngle changes are cosmetic-cost enough (and
-			// re-derived from each other on snap-to-full-ellipse) that
-			// they are intentionally not wrapped in undo here yet -- flag
-			// for a follow-up once ArcEndpoint dragging ships in the UI.
+			// The snap-to-full-ellipse behaviour in setStartAngle()/
+			// setEndAngle() (see anglesGeometricallyAdjacent()) can
+			// reset BOTH angles at once even though only one endpoint
+			// was actually dragged, so both are checked here regardless
+			// of which handle (slot 0 or 1) triggered this -- the same
+			// reasoning as CornerRadius checking both xRadius and
+			// yRadius above.
+			if (!qFuzzyCompare(m_startAngle, m_old_startAngle))
+			{
+				undo = new QPropertyUndoCommand(this, "startAngle", m_old_startAngle, m_startAngle);
+				if (!qFuzzyCompare(m_endAngle, m_old_endAngle))
+					new QPropertyUndoCommand(this, "endAngle", m_old_endAngle, m_endAngle, undo);
+			}
+			else if (!qFuzzyCompare(m_endAngle, m_old_endAngle))
+			{
+				undo = new QPropertyUndoCommand(this, "endAngle", m_old_endAngle, m_endAngle);
+			}
+			if (undo)
+				undo->setText(tr("Modifier l'angle d'un arc"));
 			break;
 
 		case HandleRole::PathAnchor:
 			if (m_shapeType == Polygon && m_polygon != m_old_polygon)
 			{
 				undo = new QPropertyUndoCommand(this, "polygon", m_old_polygon, m_polygon);
+				undo->setText(tr("Modifier la forme d'%1").arg(name()));
 			}
 			else if (m_shapeType == Path && m_nodes != m_old_nodes)
 			{
@@ -2611,6 +2637,7 @@ void QetShapeItem::handlerMouseReleaseEvent(int handlerIndex)
 				m_nodes = after;
 				const QDomElement afterXml = snapshotXml();
 				undo = new PromoteShapeCommand(this, before, afterXml);
+				undo->setText(tr("Modifier la forme d'%1").arg(name()));
 			}
 			break;
 
@@ -2624,14 +2651,38 @@ void QetShapeItem::handlerMouseReleaseEvent(int handlerIndex)
 				m_nodes = after;
 				const QDomElement afterXml = snapshotXml();
 				undo = new PromoteShapeCommand(this, before, afterXml);
+				undo->setText(tr("Modifier la courbure d'%1").arg(name()));
 			}
 			break;
 	}
 
 	if (undo)
 	{
+		// Defensive fallback only -- every role above now sets its own,
+		// distinct label directly (Resize/Rotate/SkewEdge/CornerRadius/
+		// PathAnchor/PathControlIn/PathControlOut used to all fall
+		// through to this same generic text, making the undo list
+		// unable to tell completely different edits apart); this only
+		// still matters if some future role is ever added without
+		// setting one of its own.
 		if (undo->text().isEmpty())
 			undo->setText(tr("Modifier %1").arg(name()));
+
+		// Every push here is one complete, finished gesture (press,
+		// drag, release) -- never a continuation of an earlier one, the
+		// way ArcEditor's slider deliberately pushes many commands
+		// during a single ongoing drag and wants them to merge.
+		// QPropertyUndoCommand::mergeWith() already treats any command
+		// with children as never mergeable; a dummy child guarantees
+		// that here regardless of which role produced undo. Without
+		// this, a single-property change (Rotate and SkewEdge always
+		// are; Resize and ArcEndpoint sometimes are, when only one of
+		// their two properties actually changed) would silently
+		// coalesce into whatever identically-labelled edit came right
+		// before it -- even after a deselect/reselect proved they were
+		// two separate actions, since the shared label is otherwise
+		// indistinguishable from a genuine continuation.
+		new QUndoCommand(undo);
 		diagram()->undoStack().push(undo);
 	}
 }

sources/qetgraphicsitem/qetshapeitem.h

@@ -312,7 +312,9 @@ class QetShapeItem : public QetGraphicsItem
 		qreal		 m_xRadius = 0,
 				 m_yRadius = 0,
 				 m_old_xRadius,
-				 m_old_yRadius;
+				 m_old_yRadius,
+				 m_old_startAngle,
+				 m_old_endAngle;
 
 		ShapeTransform   m_transform;
 		ShapeTransform   m_old_transform;