On Wed, Mar 11, 2015 at 10:49:17PM +0300, Grace Karanja wrote:
>
> diff --git a/qt-ui/downloadfromdivecomputer.cpp
> b/qt-ui/downloadfromdivecomputer.cpp
> index c57aa1e..994f67d 100644
> --- a/qt-ui/downloadfromdivecomputer.cpp
> +++ b/qt-ui/downloadfromdivecomputer.cpp
> @@ -3,6 +3,7 @@
> #include "mainwindow.h"
> #include "divelistview.h"
> #include "display.h"
> +#include "undocommands.h"
>
> #include <QTimer>
> #include <QFileDialog>
> @@ -426,11 +427,14 @@ void DownloadFromDCWidget::on_ok_clicked()
> return;
>
> // record all the dives in the 'real' dive_table
> + QList<struct dive*> diveList;
> for (int i = 0; i < downloadTable.nr; i++) {
> if (diveImportedModel->data(diveImportedModel->index(i,
> 0),Qt::CheckStateRole) == Qt::Checked)
> - record_dive(downloadTable.dives[i]);
> + diveList.append(downloadTable.dives[i]);
> downloadTable.dives[i] = NULL;
> }
> + UndoDownloadDives *undoCommand = new UndoDownloadDives(diveList);
I know this is nitpicking... but "UndoDownloadDives" seems like the wrong
name. It doesn't undo the downloading, it undoes the recording of the
downloaded dives, right?
> + MainWindow::instance()->undoStack->push(undoCommand);
> downloadTable.nr = 0;
>
> int uniqId, idx;
> diff --git a/qt-ui/undocommands.cpp b/qt-ui/undocommands.cpp
> index 012faaf..758b5eb 100644
> --- a/qt-ui/undocommands.cpp
> +++ b/qt-ui/undocommands.cpp
> @@ -154,3 +154,38 @@ void UndoMergeDives::redo()
> mark_divelist_changed(true);
> MainWindow::instance()->refreshDisplay();
> }
> +
> +
> +UndoDownloadDives::UndoDownloadDives(QList<dive *> downloadedDives)
> +{
> + dives = downloadedDives;
> + setText("download dives");
> +}
> +
> +void UndoDownloadDives::undo()
> +{
> + QList<struct dive*> newList;
> + for (int i = 0; i < dives.count(); i++) {
> + //make a copy of the dive before deleting it
WHY? They are already in the dives list, aren't they?
> + struct dive* d = alloc_dive();
> + copy_dive(dives.at(i), d);
> + newList.append(d);
> + //delete the dive
> + delete_single_dive(get_divenr(dives.at(i)));
> + }
> + mark_divelist_changed(true);
> + MainWindow::instance()->refreshDisplay();
> + dives.clear();
> + dives = newList;
So here you throw away your perfectly good list, only to replace it with
the new one...
> +}
> +
> +void UndoDownloadDives::redo()
> +{
> + for (int i = 0; i < dives.count(); i++) {
> + struct dive* d = alloc_dive();
> + copy_dive(dives.at(i), d);
> + record_dive(d);
see? The dives in dives (your variable names sometimes make it hard to
talk about the code :-) ) are copied before being recorded, so there's no
reason in undo to create that new list.
/D
_______________________________________________
subsurface mailing list
[email protected]
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
