bug#23906: 25.0.95; Undo boundary after process output is not consistent

[phillip.lord@russet.org.uk (Phillip Lord)]

Markus Triska <triska@metalevel.at> writes:

>> IOW, maybe we should we aim to provide that behavior not just in
>> ediprolog.el but also in comint, based on some user config.
>
> Yes! Thank you for taking this general perspective. I must say at this
> point that I have no experience with how such a "transactional" undo
> would actually work out in practice. I can imagine that it is a bit
> unusual at first. For ediprolog though, I am quite certain that I would
> prefer it because, as explained above, the interactions are typically
> short, and breaking up a short interaction into two or more parts only 1
> out of N times is definitely worse than always treating it as a unit.

Emacs undo is already transactional in the sense that it inserts undo's
between commands. If Emacs is doing stuff, running functions, and so
forth, it will not add an undo boundary unless you tell it to. This is
even true for the timer based boundaries, IIRC. Timers can only run in
one point of the command loop.


>> Using Viper's approach, you could for example throw away the
>> undo-boundaries added since the last process-send-string.  I'd expect
>> you'd do it right when you see Prolog's prompt.  So if you undo while
>> Prolog is still in the middle of responding you might only undo parts of
>> the current response (like now), but once a response is complete it will
>> always be undone as an indivisible step.
>
> This sounds like a great approach, thank you! I will try this.

So, I had a quick play, using this code (hey, my first piece of prolog
in a decade, and I copied it!).

rosetta_sleep(Time) :-
	writeln('Sleeping...'),
	sleep(Time),
	writeln('Awake!').

%?- rosetta_sleep(10).
%@ Sleeping...
%@ Awake!
%@ true.


This generalises the problem -- in this case, you are pretty much
guaranteed to get an undo-boundary between "Sleeping..." and "Awake!"
which you presumably want together.

You can actually get this behaviour -- this patch achieves it.


@@ -307,21 +307,24 @@
 for `ediprolog-consult' with a new process. With other prefix
 arguments, equivalent to `ediprolog-remove-interactions'."
   (interactive "P")
-  (cond ((eq arg 0)
-         (unless (ediprolog-running)
-           (error "No Prolog process running"))
-         (ediprolog-kill-prolog)
-         (message "Prolog process killed."))
-        ((eq arg 1) (ediprolog-consult))
-        ((eq arg 2) (ediprolog-consult t))
-        ((eq arg 7)
-         (unless (ediprolog-more-solutions)
-           (error "No query in progress"))
-         (ediprolog-toplevel))
-        ((equal arg '(4)) (ediprolog-consult) (ediprolog-query))
-        ((equal arg '(16)) (ediprolog-consult t) (ediprolog-query))
-        ((null arg) (unless (ediprolog-query) (ediprolog-consult)))
-        (t (ediprolog-remove-interactions))))
+  (cancel-timer undo-auto-current-boundary-timer)
+  (setq undo-auto-current-boundary-timer nil)
+  (let ((undo-auto-current-boundary-timer t))
+    (cond ((eq arg 0)
+           (unless (ediprolog-running)
+             (error "No Prolog process running"))
+           (ediprolog-kill-prolog)
+           (message "Prolog process killed."))
+          ((eq arg 1) (ediprolog-consult))
+          ((eq arg 2) (ediprolog-consult t))
+          ((eq arg 7)
+           (unless (ediprolog-more-solutions)
+             (error "No query in progress"))
+           (ediprolog-toplevel))
+          ((equal arg '(4)) (ediprolog-consult) (ediprolog-query))
+          ((equal arg '(16)) (ediprolog-consult t) (ediprolog-query))
+          ((null arg) (unless (ediprolog-query) (ediprolog-consult)))
+          (t (ediprolog-remove-interactions)))))
 
 (defun ediprolog-process-ready ()
   "Error if the previous query is still in progress."


Of course, this is pretty clunky and has global effect for the duration
of the let binding. Also easy to get wrong (as I did first time I tried
it).

But, if this is the behaviour you want, I think it can be added. I'll
just add a new buffer-local variable to disable the effect of the timer
(rather than the timer itself, as I have done here).

Phil