From cd8ded4b37eb37f9f25e8422c05c18a145666352 Mon Sep 17 00:00:00 2001 From: Gerald Britton Date: Thu, 20 May 2010 18:32:08 +0000 Subject: [PATCH] Updates to undo/redo logic: 1. Replace single transaction list with separate undo/redo deques 2. Update UndoHistory GUI to work with new queue setup 3. Add test to txn.py for empty transaction list svn: r15427 --- src/UndoHistory.py | 21 ++++---- src/gen/db/txn.py | 3 ++ src/gen/db/undoredo.py | 114 ++++++++++++++--------------------------- 3 files changed, 50 insertions(+), 88 deletions(-) diff --git a/src/UndoHistory.py b/src/UndoHistory.py index 23d2f46ac..159502ee9 100644 --- a/src/UndoHistory.py +++ b/src/UndoHistory.py @@ -111,7 +111,6 @@ class UndoHistory(ManagedWindow.ManagedWindow): self.show() def _selection_changed(self, obj): - assert self.undodb.undo_count == self.undodb.undoindex + 1 (model, node) = self.selection.get_selected() if not node: return @@ -119,18 +118,18 @@ class UndoHistory(ManagedWindow.ManagedWindow): start = min(path[0], self.undodb.undo_count) end = max(path[0], self.undodb.undo_count) - self._paint_rows(0, len(self.model)-1, False) + self._paint_rows(0, len(self.model) - 1, False) self._paint_rows(start, end, True) if path[0] < self.undodb.undo_count: # This transaction is an undo candidate self.redo_button.set_sensitive(False) - self.undo_button.set_sensitive(self.undodb.undo_available()) + self.undo_button.set_sensitive(self.undodb.undo_count) else: # path[0] >= self.undodb.undo_count: # This transaction is an redo candidate self.undo_button.set_sensitive(False) - self.redo_button.set_sensitive(self.undodb.redo_available()) + self.redo_button.set_sensitive(self.undodb.redo_count) def _paint_rows(self, start, end, selected=False): if selected: @@ -144,7 +143,6 @@ class UndoHistory(ManagedWindow.ManagedWindow): self.model.set(the_iter, 3, bg) def _response(self, obj, response_id): - assert self.undodb.undo_count == self.undodb.undoindex + 1 if response_id == gtk.RESPONSE_CLOSE: self.close(obj) @@ -154,7 +152,7 @@ class UndoHistory(ManagedWindow.ManagedWindow): if not node: return path = self.model.get_path(node) - nsteps = path[0]-self.undodb.undo_count-1 + nsteps = path[0] - self.undodb.undo_count - 1 self._move(nsteps or -1) elif response_id == gtk.RESPONSE_ACCEPT: @@ -163,7 +161,7 @@ class UndoHistory(ManagedWindow.ManagedWindow): if not node: return path = self.model.get_path(node) - nsteps = path[0]-self.undodb.undo_count + nsteps = path[0] - self.undodb.undo_count self._move(nsteps or 1) elif response_id == gtk.RESPONSE_APPLY: @@ -191,7 +189,7 @@ class UndoHistory(ManagedWindow.ManagedWindow): self.db.redo_callback(None) def _move(self, steps=-1): - if steps == 0 : + if steps == 0: return func = self.db.undo if steps < 0 else self.db.redo @@ -201,14 +199,13 @@ class UndoHistory(ManagedWindow.ManagedWindow): def _update_ui(self): self._paint_rows(0, len(self.model)-1, False) - self.undo_button.set_sensitive(self.undodb.undo_available()) - self.redo_button.set_sensitive(self.undodb.redo_available()) + self.undo_button.set_sensitive(self.undodb.undo_count) + self.redo_button.set_sensitive(self.undodb.redo_count) self.clear_button.set_sensitive( - self.undodb.undo_available() or self.undodb.redo_available() + self.undodb.undo_count or self.undodb.redo_count ) def _build_model(self): - assert self.undodb.undoindex+1 == len(self.undodb.undoq) self.model.clear() fg = bg = None diff --git a/src/gen/db/txn.py b/src/gen/db/txn.py index 3ec6f2a58..00c912580 100644 --- a/src/gen/db/txn.py +++ b/src/gen/db/txn.py @@ -169,6 +169,9 @@ class DbTxn(defaultdict): While the list is an arbitrary index of integers, it can be used to indicate record numbers for a database. """ + + if self.first is None or self.last is None: + return [] if not reverse: return xrange(self.first, self.last+1) else: diff --git a/src/gen/db/undoredo.py b/src/gen/db/undoredo.py index d24b7f31e..a21408a5c 100644 --- a/src/gen/db/undoredo.py +++ b/src/gen/db/undoredo.py @@ -54,8 +54,8 @@ import Errors DBERRS = (db.DBRunRecoveryError, db.DBAccessError, db.DBPageNotFoundError, db.DBInvalidArgError) -_SIGBASE = ('person', 'family', 'source', 'event', 'media', 'place', - 'repository', 'reference', 'note', 'undoq', 'redoq') +_SIGBASE = ('person', 'family', 'source', 'event', 'media', + 'place', 'repository', 'reference', 'note') #------------------------------------------------------------------------- # # DbUndo class @@ -67,15 +67,18 @@ class DbUndo(object): for use with a real backend. """ - __slots__ = ['undodb', 'db', 'mapbase', 'translist', 'undoindex', - 'undo_history_timestamp', 'txn'] + __slots__ = ('undodb', 'db', 'mapbase', 'undo_history_timestamp', + 'txn', 'undoq', 'redoq') def __init__(self, grampsdb): """ Class constructor. Set up main instance variables """ self.db = grampsdb - self.clear() + self.undoq = deque() + self.redoq = deque() + self.undo_history_timestamp = time.time() + self.txn = None self.mapbase = ( self.db.person_map, self.db.family_map, @@ -92,10 +95,8 @@ class DbUndo(object): """ Clear the undo/redo list (but not the backing storage) """ - self.undoq = deque() - self.redoq = deque() - self.translist = [] - self.undoindex = -1 + self.undoq.clear() + self.redoq.clear() self.undo_history_timestamp = time.time() self.txn = None @@ -162,78 +163,47 @@ class DbUndo(object): """ txn.set_description(msg) txn.timestamp = time.time() - - # If we're within our undo limit, add this transaction self.undoq.append(txn) - self.undoindex += 1 - if self.undoindex < DBUNDO: - if self.undoindex >= len(self.translist): - self.translist.append(txn) - else: - self.translist[self.undoindex] = txn - del self.translist[self.undoindex+1:] - self.redoq.clear() - # Otherwise, we've exceeded our undo limit - else: - self.db.abort_possible = False - self.undo_history_timestamp = time.time() - self.translist[-1] = txn - self.redoq.clear() - - def undo_available(self): - """ - Return boolean of whether or not there's a possibility of undo. - """ - #print "Undo available:", bool(self.undoq) - return len(self.undoq) - if 0 <= self.undoindex < len(self.translist): - return True - return False - - def redo_available(self): - """ - Return boolean of whether or not there's a possibility of redo. - """ - #print "Redo available:", bool(self.redoq) - return len(self.redoq) - if 0 <= self.undoindex+1 < len(self.translist): - return True - return False - def undo(self, update_history=True): """ Undo a previously committed transaction """ - if self.db.readonly or not self.undo_available(): + if self.db.readonly or self.undo_count == 0: return False - return self.__undoredo(update_history, self.__undo) + return self.__undo(update_history) def redo(self, update_history=True): """ Redo a previously committed, then undone, transaction """ - if self.db.readonly or not self.redo_available(): + if self.db.readonly or self.redo_count == 0: return False - return self.__undoredo(update_history, self.__redo) + return self.__redo(update_history) - def __undoredo(self, update_history, func): + def undoredo(func): """ - Helper method used by both undo and redo methods. + Decorator function to wrap undo and redo operations within a bsddb + transaction. It also catches bsddb errors and raises an exception + as appropriate """ - try: - with BSDDBTxn(self.db.env) as txn: - self.txn = self.db.txn = txn.txn - status = func(update_history) - if not status: - txn.abort() - self.db.txn = None - return status + def try_(self, *args, **kwargs): + try: + with BSDDBTxn(self.db.env) as txn: + self.txn = self.db.txn = txn.txn + status = func(self, *args, **kwargs) + if not status: + txn.abort() + self.db.txn = None + return status - except DBERRS, msg: - self.db._log_error() - raise Errors.DbError(msg) + except DBERRS, msg: + self.db._log_error() + raise Errors.DbError(msg) + return try_ + + @undoredo def __undo(self, update_history=True): """ Access the last committed transaction, and revert the data to the @@ -241,11 +211,8 @@ class DbUndo(object): """ txn = self.undoq.pop() self.redoq.append(txn) - #transaction = self.translist[self.undoindex] - #assert transaction == txn transaction = txn db = self.db - self.undoindex -= 1 subitems = transaction.get_recnos(reverse=True) # Process all records in the transaction @@ -260,7 +227,7 @@ class DbUndo(object): db.emit, _SIGBASE[key]) # Notify listeners if db.undo_callback: - if self.undo_available(): + if self.undo_count > 0: db.undo_callback(_("_Undo %s") % transaction.get_description()) else: @@ -274,6 +241,7 @@ class DbUndo(object): db.undo_history_callback() return True + @undoredo def __redo(self, db=None, update_history=True): """ Access the last undone transaction, and revert the data to the state @@ -281,9 +249,6 @@ class DbUndo(object): """ txn = self.redoq.pop() self.undoq.append(txn) - self.undoindex += 1 - #transaction = self.translist[self.undoindex] - #assert transaction == txn transaction = txn db = self.db subitems = transaction.get_recnos() @@ -304,8 +269,7 @@ class DbUndo(object): % transaction.get_description()) if db.redo_callback: - if len(self.redoq) > 1: - #new_transaction = self.translist[self.undoindex+1] + if self.redo_count > 1: new_transaction = self.redoq[-2] db.redo_callback(_("_Redo %s") % new_transaction.get_description()) @@ -351,10 +315,8 @@ class DbUndo(object): self.db._log_error() raise Errors.DbError(msg) - @property - def undo_count(self): - """Number of undo requests in the queue""" - return len(self.undoq) + undo_count = property(lambda self:len(self.undoq)) + redo_count = property(lambda self:len(self.redoq)) class DbUndoList(DbUndo): """