casouri / vundo

Visualize the undo tree.
413 stars 20 forks source link

Handle user quit gracefully #20

Closed ideasman42 closed 2 years ago

ideasman42 commented 2 years ago

When the user presses C-g instead of exiting mid-undo, explicitly check for user quit during potentially long running operation.

This prevents undo data being left in an invalid state, with the advantage that a if an operation is taking a long time, it's possible to early exit without any risk of problems.

casouri commented 2 years ago

Thanks. This is quite reasonable. Before I merge this, have you signed the copyright assignment to FSF? You contribution is approaching the threshold of 15 LOC under which no assignment is required. If I publish vundo I'd like to publish it to ELPA, which requires assignment from all contributors with significant contribution.

ideasman42 commented 2 years ago

Hi, yes, I have signed the agreement (needed to do this as part of this patch)

casouri commented 2 years ago

Do you have any particular reason to inhibit quit on all commands? I would only inhibit quit in vundo--move-to-node since that's the only place we modify buffer and alter buffer-undo-list.

casouri commented 2 years ago

Inhibiting quit in all commands pretty much means inhibiting quit in all vundo functions. Then should we check for quit signals in every loop? Vundo is pretty fast even if the undo history is long, so I imagine a user would only quit when something goes wrong and Emacs hangs for seconds. In that case partially drawing the tree (and breakages at roughly the same level of severity) is probably not our biggest concern.

ideasman42 commented 2 years ago

It could be limited only to functions that are known to perform multiple undo steps. Although in principle, any looping function in vundo that could potentially take a long time could inhibit quit while checking the flag.

casouri commented 2 years ago

I merged the change, but I changed it to only protect vundo--move-to-node. If you have other thought we can discuss more.