Request For Comments - Undo/Redo framework
Dirk Hohndel
dirk at hohndel.org
Thu Feb 26 09:11:04 PST 2015
Hi Grace...
On Tue, Feb 24, 2015 at 07:54:15PM +0300, Grace Karanja wrote:
> diff --git a/qt-ui/simplewidgets.cpp b/qt-ui/simplewidgets.cpp
> index f7944c9..7de067d 100644
> --- a/qt-ui/simplewidgets.cpp
> +++ b/qt-ui/simplewidgets.cpp
> @@ -142,8 +142,19 @@ void RenumberDialog::renumberOnlySelected(bool selected)
>
> void RenumberDialog::buttonClicked(QAbstractButton *button)
> {
> - if (ui.buttonBox->buttonRole(button) == QDialogButtonBox::AcceptRole)
> + if (ui.buttonBox->buttonRole(button) == QDialogButtonBox::AcceptRole) {
> + QMap<int,int> renumberedDives;
> + int i;
> + struct dive *dive = NULL;
> + for_each_dive (i, dive) {
> + if (dive->selected) {
Careful - you are ignoring the selectedOnly flag... so the condition
should be if (!selected_only || dive->selected)
> + renumberedDives.insert(dive->id, dive->number);
> + }
> + }
> + UndoRenumberDives *undoCommand = new UndoRenumberDives(renumberedDives, ui.spinBox->value());
> + MainWindow::instance()->undoStack->push(undoCommand);
> renumber_dives(ui.spinBox->value(), selectedOnly);
Oh, so this time you ARE calling the activity function from here instead
of having the redo registration take care of that. That feels a little
inconsistent with your last patch...
> diff --git a/qt-ui/undocommands.cpp b/qt-ui/undocommands.cpp
> index 316def4..a075e99 100644
> --- a/qt-ui/undocommands.cpp
> +++ b/qt-ui/undocommands.cpp
> @@ -62,3 +62,41 @@ void UndoShiftTime::redo()
> mark_divelist_changed(true);
> MainWindow::instance()->refreshDisplay();
> }
> +
> +
> +UndoRenumberDives::UndoRenumberDives(QMap<int, int> originalNumbers, int startNumber)
> +{
> + oldNumbers = originalNumbers;
> + start = startNumber;
> + isFirstRedo = true;
And because of that (previous comment) you need this weird flag...
> + setText("renumber dive");
> + if (oldNumbers.count() > 1)
> + setText(QString("renumber %1 dives").arg(QString::number(oldNumbers.count())));
> +}
> +
> +void UndoRenumberDives::undo()
> +{
> + foreach (int key, oldNumbers.keys()) {
> + struct dive* d = get_dive_by_uniq_id(key);
> + d->number = oldNumbers.value(key);
> + }
> + mark_divelist_changed(true);
> + MainWindow::instance()->refreshDisplay();
> +}
> +
> +void UndoRenumberDives::redo()
> +{
> + if (isFirstRedo) {
> + isFirstRedo = false;
> + return; //Skip the first redo. Qt calls redo() when the QUndoCommand is pushed
> + //to the QUndoStack
Yeah, that's just silly - I liked what you did last time better. Put a
coment where the current call to renumber_dives() is so that no one gets
confused, but then have just one place where you do this operation,
Would you mind redoing / resubmitting the patch?
Thanks
/D
More information about the subsurface
mailing list