Is there an undo command? If not, would it be possible? #165

Closed
tzarskyz opened this Issue Nov 27, 2012 · 17 comments

Comments

Projects
None yet
5 participants

One thing I think would be useful in Slate is an undo command. Say you resize a window to the top left quadrant of your screen, pressing the undo command's binding would revert the window to it's previous state (size, position, maybe even monitor eventually.)

What do you all think? Heck, it is probably already implemented I just haven't found it but if it is not I am curious to know how much work would be necessary to implement this feature? Unfortunately I can't code to save my life so I am counting on one of you keyboard ninjas to solve this one, as usual.

Anybody else think they would use an "Undo" binding?

Owner

jigish commented Nov 27, 2012

I should be able to do this very easily by creating a before snapshot when any binding is run. Wouldn't work for focus bindings but any sort of move would be able to be undone. I suppose for focus I can keep track of the previously focused window. I'm getting a bit more time now so I'll look at this tomorrow.

jigish was assigned Nov 27, 2012

gimbo commented Nov 27, 2012

I'd just like to +1 this - I've been using SizeUp for quite a while, and I'm attempting to migrate to Slate, and as far as I can see this is the only thing missing for me (and it won't prevent the migration). For me a very common pattern in SizeUp is to maximise a window for a short time, then revert to its previous state. Would love to see this feature. Thanks!

+1 also. Having the ability to restore a window to its previous state would make for some very interesting chained functionality.

Thanks for showing the interest guys and thanks jigish for looking into this. This feature would be extremely useful imo. I find something new with Slate everyday. It's amazing! Thanks again for looking into it.

@jigish jigish pushed a commit that referenced this issue Nov 28, 2012

Jigish Patel [Issue #165] initial undo operation 2ce1c47
Owner

jigish commented Nov 28, 2012

implemented an initial version of this. It keeps a stack of snapshots and updates the stack every time you activate a movement related binding in slate. by default the max stack size is 10, but this is configurable through undoMaxStackSize. I'll upload a new version later today with this. let me know if there are any issues.

jigish closed this Nov 28, 2012

Owner

jigish commented Nov 28, 2012

also, just to reiterate - this will not work for focus or visibility related operations and may very well screw up the focus ordering of windows.

I just downloaded the source to try undo but after compiling Slate and running the new version it immediately gives me an error of: "Unrecognized Operation: Unrecognized operation 'undo' in 'undo" This message is displayed in a pop-up with the buttons/options to either "skip" or "quit" Slate.

I put your example undo in my config file. bind 1:ctrl undo

Anyone else have any ideas?

If anyone gets it working, please let me know so I can try and troubleshoot my problems. Thanks.

Owner

jigish commented Nov 28, 2012

Just pushed an update. try updating your release version and let me know if that doesn't work

Owner

jigish commented Nov 28, 2012

btw - it is version 1.0.13 that has the change

Works perfect! Jumped the gun there, anyhow thanks again for implementing undo. No problems at all.

Contributor

jangxyz commented Dec 13, 2012

I couldn't figure how to perform "full screen => back to original", on version 1.0.16
Tried giving the following:

config undoMaxStackSize 1
bind f:ctrl;alt;cmd chain move screenOriginX;screenOriginY screenSizeX;screenSizeY | undo

Fullscreen seem to work, but undo doesn't. Any reason why?

Owner

jigish commented Dec 13, 2012

Yah. take a look at the undoOps config. You need to set it such that chain is not in undoOps because if it is, it'll just take a new snapshot right before it tries to undo which result in slate doing absolutely nothing.

Contributor

jangxyz commented Dec 13, 2012

@jigish Still didn't achieve what I wanted. After trying some options in undoOps, I found something weird happening.

  1. When chain is removed from undoOps, the following 'chained' command does not save snapshot at all:
    bind f:ctrl;alt;cmd chain move screenOriginX;screenOriginY screenSizeX;screenSizeY | undo
  2. However, normal move options do save snapshot:
    bind f:shift;ctrl;alt;cmd move screenOriginX;screenOriginY screenSizeX;screenSizeY

Tried this with 'move only'(undoOps move) and 'everything except chain'(undoOps activate-snapshot,grid,layout,move,resize,sequence,shell).
Adding chain results in saving the snapshot, but fails to activate it -- perhaps due to what you said, as it saves another snapshot.

From the observation, my guess is that
undoOps without chain makes it fail early when chain is seen, preventing the inner move save snapshots..?
I've tested it by reading the Application Support/com.slate.Slate/snapshots file.

.. or maybe I've made some stupid mistake somewhere in the config file..

Owner

jigish commented Dec 13, 2012

oh no you're right. if chain isn't in undoOps then it won't make the snapshot the first time around. yah that just won't work lol. I actually don't know if this is even possible with the current implementation of undo/chain. I wonder if anyone else has implemented this sort of functionality?

Contributor

jangxyz commented Dec 14, 2012

But what does undoing a chain mean?

Since chain is combining multiple operations to one, I'm not sure whether letting the chain operation be undo'ed or not makes sense at all. It also conflicts with the phrase "you can undo only movement-based operations" because chain itself has nothing to do with movement.
You can't undo the whole chained operations at one either, because it's circular.

How about removing chain from undoOps completely and
checking whether there are any other undoOps inside the command?

gimbo commented Dec 14, 2012

I haven't been compeltely paying attention to this, or to how undo is implementing, but I just wanted to butt in with some general thoughts, which may or may not be helpful...

The usual way to handle undo is to build a stack of undo-able operations as they're performed, and then when you undo it simply pops operation items from the stack and applies their inverse (so you can only undo operations whose inverse you can compute (perhaps by explicitly storing prior state) and it's not in general possible to undo operations which have side effects, such as performing shell commands). I don't think chaining has any special effect here, because as you progress through a chain performing operations, it would just push commands onto the stack in the usual fashion? Or am I missing something?

sequence is a bit more interesting than chain because here we have multiple operations performed in aggregate; but that's OK, you just push onto the stack a special sequence container with each of the composite operations referenced from it, and if you undo/pop that operation, it just goes through the operations in the sequence in reverse, applying their inverses in the usual manner - with the usual caveats that certain operations have no well-defined inverse/undo.

I realise I might be talking about completely reimplementing how you do undo - but this is how I'd do it, anyway. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment