From 8ab4e24b61cd9ff823265008000c7bed1b389c60 Mon Sep 17 00:00:00 2001 From: ispyisail Date: Sun, 16 Aug 2026 10:08:16 +1200 Subject: [PATCH] Take the modal dialog out of RotateTextsCommand's constructor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RotateTextsCommand called QDialog::exec() from inside its constructor, so the command could not be built without a human answering a dialog. That made it untestable headlessly, undrivable from any script or test harness, and it is why bugtracker #312 (PR #707) shipped with its save/reload round-trip unverified -- the symptom could not be reproduced without a GUI. The command now takes the angle as a parameter and does no asking. Two statics carry the interactive half: hasSelectedTexts(diagram) -- is there anything to rotate askRotation(rotation) -- open the dialog, false if cancelled The single call site in QETDiagramEditor asks first, then builds the command, so the user-visible behaviour is unchanged: same dialog, same title, same no-dialog-on-empty-selection. Keeping askRotation() in this class also keeps the QObject tr() context, so existing translations of "Orienter les textes sélectionnés" are not invalidated. Also guards undo()/redo() against a null m_anim_group. When nothing is selected the constructor calls setObsolete(true) without ever creating the animation group, and QUndoStack::push() calls redo() before discarding an obsolete command -- a latent null dereference on that path. Verified headlessly, which was the point: driving the command through a scratch --test-ops op on examples/741.qet (67 conductors), rotation attributes written on save go 0 -> 67 with the #707 fix present and stay at 0 with it reverted, while the reverted build instead writes userx on all 67. That is bugtracker #312 reproduced and fixed under test for the first time. --- sources/qetdiagrameditor.cpp | 13 ++++- sources/undocommand/rotatetextscommand.cpp | 62 +++++++++++++++++----- sources/undocommand/rotatetextscommand.h | 27 ++++++++-- 3 files changed, 85 insertions(+), 17 deletions(-) diff --git a/sources/qetdiagrameditor.cpp b/sources/qetdiagrameditor.cpp index 6c23478ad..6db163f87 100644 --- a/sources/qetdiagrameditor.cpp +++ b/sources/qetdiagrameditor.cpp @@ -1856,7 +1856,18 @@ void QETDiagramEditor::selectionGroupTriggered(QAction *action) diagram->undoStack().push(c); } else if (value == "rotate_selected_text") - diagram->undoStack().push(new RotateTextsCommand(diagram)); + { + //Ask for the angle first, then build the command: the command + //itself no longer opens a dialog. Guarding on the selection keeps + //the previous behaviour of showing no dialog when there is + //nothing to rotate. + if (RotateTextsCommand::hasSelectedTexts(diagram)) + { + qreal rotation = 0; + if (RotateTextsCommand::askRotation(rotation)) + diagram->undoStack().push(new RotateTextsCommand(diagram, rotation)); + } + } else if (value == "find_selected_element" && currentElement()) findElementInPanel(currentElement()->location()); else if (value == "edit_selected_element") diff --git a/sources/undocommand/rotatetextscommand.cpp b/sources/undocommand/rotatetextscommand.cpp index 5b19dedac..704b7bcd8 100644 --- a/sources/undocommand/rotatetextscommand.cpp +++ b/sources/undocommand/rotatetextscommand.cpp @@ -25,15 +25,34 @@ #include "../qetgraphicsitem/elementtextitemgroup.h" #include "../qtextorientationspinboxwidget.h" +/** + @brief RotateTextsCommand::hasSelectedTexts + @param diagram + @return true if @p diagram has at least one selected text or text group. + Lets a caller decide whether to ask the user for an angle at all, keeping + the previous behaviour where no dialog appeared for an empty selection. +*/ +bool RotateTextsCommand::hasSelectedTexts(Diagram *diagram) +{ + if(!diagram) + return false; + + DiagramContent dc(diagram); + return (!dc.selectedTexts().isEmpty() || !dc.selectedTextsGroup().isEmpty()); +} + /** @brief RotateTextsCommand::RotateTextsCommand @param diagram : Apply the rotation to the selected texts and group of texts - of diagram at construction time. + of diagram at construction time. + @param rotation : the angle to apply, in degrees. Obtain it from + askRotation() for interactive use; pass it directly from a test or script. @param parent : undo parent */ -RotateTextsCommand::RotateTextsCommand(Diagram *diagram, QUndoCommand *parent) : +RotateTextsCommand::RotateTextsCommand(Diagram *diagram, qreal rotation, QUndoCommand *parent) : QUndoCommand(parent), -m_diagram(diagram) +m_diagram(diagram), +m_rotation(rotation) { DiagramContent dc(m_diagram); QList texts_list; @@ -53,8 +72,6 @@ m_diagram(diagram) if(texts_list.count() || groups_list.count()) { - openDialog(); - QStringList parts; if (texts_list.count()) parts << QObject::tr("%n texte(s)", "", texts_list.count()); @@ -76,9 +93,15 @@ void RotateTextsCommand::undo() if(m_diagram) m_diagram.data()->showMe(); + //Nothing was selected at construction time: there is no animation to + //run. QUndoStack::push() calls redo() before it discards an obsolete + //command, so this has to be survivable rather than assumed away. + if(!m_anim_group) + return; + m_anim_group->setDirection(QAnimationGroup::Backward); m_anim_group->start(); - + for(ConductorTextItem *cti : m_cond_texts.keys()) cti->forceRotateByUser(m_cond_texts.value(cti)); } @@ -88,14 +111,28 @@ void RotateTextsCommand::redo() if(m_diagram) m_diagram.data()->showMe(); + if(!m_anim_group) + return; + m_anim_group->setDirection(QAnimationGroup::Forward); m_anim_group->start(); - + for(ConductorTextItem *cti : m_cond_texts.keys()) cti->forceRotateByUser(true); } -void RotateTextsCommand::openDialog() +/** + @brief RotateTextsCommand::askRotation + Ask the user for an orientation. + @param rotation : set to the chosen angle when the dialog is accepted, + left untouched otherwise. + @return true if the user accepted, false if they cancelled. + + Deliberately static and separate from the command: a QUndoCommand that + blocks on a modal in its constructor cannot be built by a test, a script, + or any headless caller. +*/ +bool RotateTextsCommand::askRotation(qreal &rotation) { //Open the dialog QDialog ori_text_dialog; @@ -120,10 +157,11 @@ void RotateTextsCommand::openDialog() layout_v.addStretch(); layout_v.addWidget(&buttons); - if (ori_text_dialog.exec() == QDialog::Accepted) - m_rotation = ori_widget->orientation(); - else - setObsolete(true); + if (ori_text_dialog.exec() != QDialog::Accepted) + return false; + + rotation = ori_widget->orientation(); + return true; } void RotateTextsCommand::setupAnimation(QObject *target, const QByteArray &propertyName, const QVariant& start, const QVariant& end) diff --git a/sources/undocommand/rotatetextscommand.h b/sources/undocommand/rotatetextscommand.h index 84abe1a51..0ddf58830 100644 --- a/sources/undocommand/rotatetextscommand.h +++ b/sources/undocommand/rotatetextscommand.h @@ -28,19 +28,38 @@ class QParallelAnimationGroup; /** @brief The RotateTextsCommand class - Open a dialog for edit the rotation of the current selected texts and texts group in diagram. - Just instantiate this undo command and push it in a QUndoStack. + Apply @p rotation to the currently selected texts and texts group of a + diagram. Just instantiate this undo command and push it in a QUndoStack. + + This command does not ask the user for anything: obtaining the angle is the + caller's job, via the askRotation() helper below. Keeping the dialog out of + the constructor is what makes the command usable outside an interactive + session — from a test harness, a regression sweep, or a script — and stops + a headless caller from blocking forever on a modal nobody can answer. + + Typical interactive use: + @code + if (RotateTextsCommand::hasSelectedTexts(diagram)) { + qreal rotation = 0; + if (RotateTextsCommand::askRotation(rotation)) + diagram->undoStack().push(new RotateTextsCommand(diagram, rotation)); + } + @endcode */ class RotateTextsCommand : public QUndoCommand { public: - RotateTextsCommand(Diagram *diagram, QUndoCommand *parent=nullptr); + RotateTextsCommand(Diagram *diagram, qreal rotation, QUndoCommand *parent=nullptr); + + /// @return true if @p diagram has at least one selected text or text group to rotate. + static bool hasSelectedTexts(Diagram *diagram); + /// Open the orientation dialog. @return true and set @p rotation if accepted, false if cancelled. + static bool askRotation(qreal &rotation); void undo() override; void redo() override; private: - void openDialog(); void setupAnimation(QObject *target, const QByteArray &propertyName, const QVariant& start, const QVariant& end); private: