On Thu, Mar 12, 2015 at 09:13:06AM +0300, Grace Karanja wrote:
> diff --git a/qt-ui/divelistview.cpp b/qt-ui/divelistview.cpp
> index bded95e..d1c5260 100644
> --- a/qt-ui/divelistview.cpp
> +++ b/qt-ui/divelistview.cpp
> @@ -598,15 +598,13 @@ void DiveListView::removeFromTrip()
> //TODO: move this to C-code.
> int i;
> struct dive *d;
> + QMap<struct dive*, dive_trip*> dives;
So sometimes "dives" is a list of dives. Sometimes you use "diveList" for
a list of dives (I like that better), and now "dives" is a map from dive
to trip (I really dislike this). Can you come up with a consistent naming
scheme for your variables and try to stick with it? I know we are not very
good rolemodels in the overall code, but here with you as the only author,
this would be good.
> for_each_dive (i, d) {
> if (d->selected)
> - remove_dive_from_trip(d, false);
> + dives.insert(d, d->divetrip);
> }
> - rememberSelection();
> - reload(currentLayout, false);
> - fixMessyQtModelBehaviour();
> - restoreSelection();
> - mark_divelist_changed(true);
You remove the selection handling here...
> + UndoRemoveDivesFromTrip *undoCommand = new
> UndoRemoveDivesFromTrip(dives);
> + MainWindow::instance()->undoStack->push(undoCommand);
> }
>
> void DiveListView::newTripAbove()
> diff --git a/qt-ui/undocommands.cpp b/qt-ui/undocommands.cpp
> index 758b5eb..cd776e0 100644
> --- a/qt-ui/undocommands.cpp
> +++ b/qt-ui/undocommands.cpp
> @@ -189,3 +189,32 @@ void UndoDownloadDives::redo()
> mark_divelist_changed(true);
> MainWindow::instance()->refreshDisplay();
> }
> +
> +
> +UndoRemoveDivesFromTrip::UndoRemoveDivesFromTrip(QMap<dive *, dive_trip *>
> movedDives)
> +{
> + dives = movedDives;
See above. "dives" is not a good name
> + setText("remove dives from trip");
> +}
> +
> +void UndoRemoveDivesFromTrip::undo()
> +{
> + QMapIterator<dive*, dive_trip*> i(dives);
> + while (i.hasNext()) {
> + i.next();
> + add_dive_to_trip(i.key(), i.value());
> + }
> + mark_divelist_changed(true);
> + MainWindow::instance()->refreshDisplay();
> +}
> +
> +void UndoRemoveDivesFromTrip::redo()
> +{
> + QMapIterator<dive*, dive_trip* > i(dives);
> + while (i.hasNext()) {
> + i.next();
> + remove_dive_from_trip(i.key(), false);
> + }
> + mark_divelist_changed(true);
> + MainWindow::instance()->refreshDisplay();
You don't do the selection handling here - which I think is correct.
We shouldn't mess with the selection in the undo/redo cycle. But that
means we shouldn't have removed it above...
/D
_______________________________________________
subsurface mailing list
[email protected]
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
