undo traceback

[Greg Couch]

Reproduce with:

open 1gcn
show
split atoms :1-12 atoms :13-24
undo

The traceback is:

Traceback (most recent call last):
  File "/home/chimera/chimerax_daily/lib/python3.6/site-packages/chimerax/core/undo.py", line 149, in undo
    inst.undo()
  File "/home/chimera/chimerax_daily/lib/python3.6/site-packages/chimerax/core/undo.py", line 342, in undo
    self._consistency_check()
  File "/home/chimera/chimerax_daily/lib/python3.6/site-packages/chimerax/core/undo.py", line 369, in _consistency_check
    (owner_length, value_length))
ValueError: undo action with different number of owners and old values: 0 != 246

Discussion:

Putting the logic in the split command to clear the undo stack would work, but there are probably lots of unknown places where that would need to be done. Instead, this should be a "UserError" -- ideally with the word 'atoms' instead of 'owners'.

[Conrad Huang]

The whole design of undo is that the mechanism doesn't care what types of items are being added. In this case, it's "atoms". In other cases, it may be "residues" or "molecules" or something else (surfaces, volumes?). A generic message like "Undo failed, probably because structures have been modified." might be okay?

[Greg Couch]

Fine by me.

[Conrad Huang]

Obviously the bad undo action should be popped off the stack. Should the entire undo stack be cleared?

[Greg Couch]

Since the undo actions are not classified, eg., camera actions vs. structure actions, it is not possible to tell which previous actions are still valid, so clearing the undo stack is the reasonable thing to do. That should be part of the message: "Undo failed, .... Clearing undo stack"

[goddard@…]

I think it is not necessary to clear the undo stack.  Possibly other actions that can be undone will work.  Why prevent the user from trying?  Of course failed undo actions need to give a sane user error rather than a traceback.

[Conrad Huang]

Fixed in 81635c182.

UserError exception is thrown now, resulting in an error message, not a traceback. Only the offending undo action is removed, leaving the remainder of the stack alone. Users can find out what's there via "undo list".