undo/redo Funktion

[Rekrul]

Hallo,

ich versuche gerade für mein Projekt einen Undo-Manager zu implementieren. Hier könnt ihr einen ersten Entwurf dafür sehen:

Code: Alles auswählen

class UndoManager(object):
    def __init__(self):
        """
        Constructor for UndoManager.
        """
        self.clear()
        
        
    def add(self, undoCommand, undoArgs=(), \
                         redoCommand=None, redoArgs=()):
        """
        Add a undo/redo command to the undo stack.
        """
        if self.LOCK:
            return
        command = {
                   'Undo': undoCommand,
                   'UndoArgs': undoArgs,
                   'Redo': redoCommand,
                   'RedoArgs': redoArgs 
                   }
        
        self._undoStack.append(command)
        self._redoStack = []
    
    def undo(self):
        """
        Pop latest command from _undoStack and call it. If there es a redo 
        action, then add it to the _redoStack
        """
        command = self._undoStack.pop()
        
        self.LOCK = True
        command['Undo'](*command['UndoArgs'])
        self.LOCK = False
        
        if command['Redo'] is not None:
            self._redoStack.append(command)
    
    def redo(self):
        """
        Pop latest command from _redoStack and call it. Readd the action to the 
        _undoStack.
        """
        command = self._redoStack.pop()

        self.LOCK = True
        command['Redo'](*command['RedoArgs'])
        self.LOCK = False
        
        self._undoStack.append(command)
    
    def clear(self):
        """
        Clear the _undoStack and _redoStack.
        """
        self._undoStack = []
        self._redoStack = []
        self.LOCK = False

Hier ein Beispiel, wie das ganze verwendet werden soll:

Code: Alles auswählen

from undo import UndoManager

class A:
    def __init__(self):
        self.x = 0
        self.undoManager = UndoManager()
        
    def add(self, value):
        self.x += value
        #inform undoManager
        self.undoManager.add(self.subtract, (value,), self.add, (value,))
        
    def subtract(self, value):
        self.x -= value
        #inform undoManager
        self.undoManager.add(self.add, (value,), self.subtract, (value,))

Ich zeige dies hier, weil ich mir nicht vorstellen kann, dass ich eine elegante Lösung habe und würde mich daher auch über Kritiken, Fehler bzw. Anregungen freuen. Vielleicht kennt auch jemand ein Modul der macht, was ich mir wünsche (z.Z. wäre auch gutes Wetter wünschenswert).

Gruß, Rekrul

[cofi]

Zumindest ueber das `LOCK` solltest du nachdenken, da der momentane Ansatz nicht atomar ist. Mit `threading.Lock` solltest du besser bedient sein. V.a. kann man damit eine elegante Loesung mit `with` basteln.

[DasIch]

Optimal wäre ein Baum statt einem Stack, dann kannst du, wie in vim, für Änderungen nach einem Undo einen neuen Ast erzeugen.

Damit hat man den Vorteil zu jedem Status gelangen zu können.

Dein `LOCK` ist übrigens völlig sinnlos und ein einfaches PEP 8 konformes `lock` würde es als Name auch tun.

[BlackJack]

@DasIch: Warum ist das Lock sinnlos? Man muss doch verhindern das die Aktion selbst wieder ihre gegenteilige Aktion bei dem Manager registriert.

[Rekrul]

@cofi: Vielen Dank für deinen Tip. Ich habe LOCK durch threading.Lock ersetzt:

In undo und redo:

Code: Alles auswählen

        self.lock.acquire(False)
        command['Redo'](*command['RedoArgs'])
        self.lock.release()

Und in add:

Code: Alles auswählen

        if self.lock.locked():
            return

Wie ich allerdings with einbauen soll ist mir unklar, da ich ja eigentlich nicht mehrere Threads habe, sondern lediglich verhindern möchte, dass sich beim Ausführen der undo-/redo-Funktionen diese nicht wieder beim UndoManager registrieren. Und wenn ich nur einen Thread habe, so sollte ich den vielleicht besser nicht blockieren . Vielleicht habe ich da aber auch was missverstanden.

@DasIch: Eine Baumstruktur wäre in der Tat interessant. Habe ich noch garnicht in Betracht gezogen. Werde ich jetzt aber nachholen.

[DasIch]

@DasIch: Warum ist das Lock sinnlos? Man muss doch verhindern das die Aktion selbst wieder ihre gegenteilige Aktion bei dem Manager registriert.

Es ist nicht threadsafe.

[BlackJack]

@DasIch: Das muss es doch auch gar nicht sein. Das wäre vielleicht zusätzlich wünschenswert, aber in single-threaded-Anwendungen erfüllt es auch so eine wichtige Aufgabe, kann also nicht sinnlos sein.

[Rekrul]

Ich glaube ich habe das mit dem with jetzt verstanden. Ich habe den UndoManager um folgendes erweitert/verändert:

Code: Alles auswählen

class UndoManager(object):
    ...
    def __enter__(self):
        self.lock.acquire(False)
        return self
    
    def __exit__(self, type, value, traceback):
        self.lock.release()

    def redo(self):
        ...
        with self:
            command['Redo'](*command['RedoArgs'])

    def redo(self):
        ...
        with self:
            command['Redo'](*command['RedoArgs'])
    ...

Jetzt kann ich mit dem with statement verhindern, dass Aktionenen in den UndoManager aufgenammen werden. Beispielsweise:

Code: Alles auswählen

class A():
    def __init__(self):
        self.x = 0
        self.undoManager = UndoManager()
        
    def add(self, value):
        self.x += value
        self.undoManager.add(self.add, (-value,), self.add, (value,))
        
    def add_twice(self, value):
        with self.undoManager:
            self.add(value)
            self.add(value)
        self.undoManager.add(self.add_twice, (-value,), \
                             self.add_twice, (value,))

War das gemeint?

[Hyperion]

Wozu soll das gut sein:

Code: Alles auswählen

with self:
    ...

?

Du führst den Aufruf doch von außen durch!

[cofi]

Ja. Aber `threading.Lock` unterstuetzt selbst schon das Contextmanager-Protokoll. Allerdings gibt es wohl keinen Weg `Lock`s zu erzeugen die dann non-blocking benutzen.

Ich wuerde allerdings dann nicht den Manager selbst als Lock missbrauchen, sondern eine neue Klasse erstellen:

Code: Alles auswählen

class NonblockingLock(object):
    def __init__(self):
        self.lock = threading.Lock()
    def __enter__(self):
        self.lock.acquire(False)
        return self
    def __exit__(self, *args):
        self.lock.release()

[Rekrul]

@cofi: Danke, hab ich so umgesetzt.

@Hyperion: Ich mache den Aufruf nur von außen, wenn ich eine Methode aufrufe, die sich sonst immer zum UndoManager added, dies aber nicht erwünscht ist (siehe Beispiel ....). Wenn ich undo aufrufe, dann wird der Aufruf nicht gemacht.

[sma]

Hatte neulich einen Undo-Manager für ein JavaScript-Projekt gebaut. Übertragen in Python wäre es:

Code: Alles auswählen

class Command:
    def undo(self): pass
    def redo(self): pass

class UndoManager:
    def __init__(self):
        self.undos, self.redos = [], []
    def undo(self):
        command = self.undos.pop()
        self.redos.append(command)
        command.undo()
    def redo(self):
        command = self.redos.pop()
        self.undos.append(command)
        command.redo()
    def execute(self, command):
        self.undos.append(command)
        self.redos = []
        command.redo()

Das erscheint mir einfacher als der ursprünglich gezeigte Code. Ein Befehl kann mit `UndoManager.execute` registriert und ausgeführt werden. Danach kann man ihn mit `UndoManager.undo` widerrufen und mit `UndoManager.redo` wiederholen.

Ich sehe keine Notwendigkeit für eine LOCK-Variable. Ich halte es auch für kein gutes Design, wenn sich eine Funktion als Seiteneffekt beim UndoManager registriert. Befehle sollten komplett unabhängig davon funktionieren, schon damit das testen einfach bleibt. Daher:

Code: Alles auswählen

x = 0
class Add(Command):
    def __init__(self, v): self.v = v
    def redo(self): global x; x += self.v
    def undo(self): global x; x -= self.v
undoManager.execute(Add(1))

Stefan

[Rekrul]

@sma: Vielen Dank für deinen Beitrag. Deine Idee scheint mir sinnvoll und wesentlich einfacher. Was mich erstmal nicht so überzeugt ist, dass ich von einer Klasse command erben muss und die Methoden undo und redo heissen müssen. Was wenn ich in einer Klasse zwei oder mehr Methoden habe, die ich rückgängig machen möchte? Evtl. wäre deine Idee sehr schön in Kombination mit Dekoratoren.

Code: Alles auswählen

class A(object):
    def __init__(self, value):
        self.value = value
    @undo
    def add(self, x):
        self.value += x
    @redo(add)
    def subtract(self, x):
        self.value -= x

[sedi]

Kann man das dann mit einer Tkinter-GUI verbinden, so dass der UndoManager gleich die Buttons 'Rückgängig' und 'Wiederholen' im Menü (normalerweise) Bearbeiten passend einstellt?