Re: An idea: combine-change-calls

[Alan Mackenzie]

Hello, Stefan.

On Thu, Mar 29, 2018 at 15:10:47 -0400, Stefan Monnier wrote:
> > Right, I think I've got it now.  The macro combine-change-calls would
> > maybe first push some sort of sentinal element onto buffer-undo-list,
> > then let ,@forms run, then massage the list so that the 2000 elements
> > created by ,@forms would become part of the `apply' element.

> Right.  Now you can re-read the pseudo-code I had sent:

>     (let ((elem-apply `(apply 0 ,beg ,end ,#'my-undo-combining nil)))
>       (push elem-apply buffer-undo-list)
>       (funcall body)
>       (let ((new-bul (memq elem-apply buffer-undo-list)))
>         (when new-bul
>           (let ((undo-elems buffer-undo-list))
>             (setf (nthcdr (- (length undo-elems) (length new-bul))
>                           undo-elems)
>                   nil)
>             (setf (nth 1 elem-apply) (- end-marker end))
>             (setf (nth 3 elem-apply) (marker-position end-marker))
>             (setf (nth 5 elem-apply) undo-elems)
>             (setq buffer-undo-list new-bul)))))

> and then

>     (defun my-undo-combining (undo-elems)
>       (let ((inhibit-modification-hooks t))
>         (while t
>           (primitive-undo 1 undo-elems))))

I've got a first version of some real working code.  Not to be forgotten
is the need for my-undo-combining (which I've called
wrap-and-run-primitive-undo) itself to generate an (apply ...
my-undo-combining ...) for the use of redo.

Here's that code.  It badly needs doc strings and commenting, and there
are one or two safety checks not yet there.  But it is working code.
What do you think?



(defun wrap-and-run-primitive-undo (beg end list)
  (let ((old-bul buffer-undo-list)
        (end-marker (copy-marker end)))
    (if (not inhibit-modification-hooks)
        (run-hook-with-args 'before-change-functions beg end))
    (let ((inhibit-modification-hooks t))
      (funcall #'primitive-undo 1 list))
    (unless (eq buffer-undo-list t)
      (let ((ap-elt
             (list 'apply
                   (- end end-marker)
                   beg
                   (marker-position end-marker)
                   #'wrap-and-run-primitive-undo
                   beg (marker-position end-marker) buffer-undo-list))
            (ptr buffer-undo-list))
        (if (not (eq buffer-undo-list old-bul))
            (progn
              (while (not (eq (cdr ptr) old-bul))
                (setq ptr (cdr ptr)))
              (setcdr ptr nil)
              (push ap-elt buffer-undo-list)
              (setcdr buffer-undo-list old-bul)))))
    (if (not inhibit-modification-hooks)
        (run-hook-with-args 'after-change-functions
                            beg (marker-position end-marker)
                            (- end beg)))))
          
(defmacro combine-change-calls (beg end &rest forms)
  `(let ((-beg- ,beg)
         (-end- ,end)
         (body (lambda () ,@forms))
         (old-bul buffer-undo-list)
         end-marker)
     (setq end-marker (copy-marker -end-))
     (if (not inhibit-modification-hooks)
         (run-hook-with-args 'before-change-functions -beg- -end-))
     (if (eq buffer-undo-list t)
         (progn
           (funcall body)
           (if (not inhibit-modification-hooks)
               (run-hook-with-args 'after-change-functions
                                   -beg- (marker-position end-marker)
                                   (- -end- -beg-))))
       (let ((inhibit-modification-hooks t))
         (funcall body))
       (let ((ap-elt
              (list 'apply
                    (- -end- end-marker) ; DELTA
                    -beg-
                    (marker-position end-marker)
                    #'wrap-and-run-primitive-undo
                    -beg- (marker-position end-marker) buffer-undo-list))
             (ptr buffer-undo-list))
         (if (not (eq buffer-undo-list old-bul))
             (progn
               (while (not (eq (cdr ptr) old-bul))
                 (setq ptr (cdr ptr)))
               (setcdr ptr nil)
               (push ap-elt buffer-undo-list)
               (setcdr buffer-undo-list old-bul)))))
     (if (not inhibit-modification-hooks)
         (run-hook-with-args 'after-change-functions
                             -beg- (marker-position end-marker)
                             (- -end- -beg-)))
     (setq end-marker nil)))


As usual, comments and criticism of the above would be welcome.

[ .... ]

> I could imagine a macro `with-inhibit-read-only` which would bind
> inhibit-read-only to t and then tweak the buffer-undo-list similarly to
> what you're doing such that the undo is also performed with
> inhibit-read-only bound to t.

OK, thanks.  The above code hasn't taken this into account

[ .... ]

> I'm not interested in destroying this information.  I'm just trying to
> solve the problem at hand and it's a simple and efficient solution, and
> neither you nor I can come up with a scenario where this simpler
> solution is worse, apparently.

Well, I've wanted to see this info whilst writing c-c-c.

> > Its continued existence does not cause any disadvantage.  The run time
> > which would be saved in `undo' is minimal and inconsequential.

> Agreed, the gain is in the simplicity of combine-change-calls.

I can definitely see that.  ;-)

[ .... ]

> >> IIUC we agree that this is considered an unimportant use-case and it's
> >> OK to just ignore such boundaries.
> > I suppose so.  What would such a boundary mean anyway?  We're talking
> > about a scenario where all the change hook invocations are amalgamated.
> > How could it make sense to split that up?

> Some commands explicitly push undo boundaries in the middle of their
> execution, so that the user can easily undo the 2nd half of the
> command's execution, for example.

> This is rare enough that I think it's perfectly OK to say that such
> use-cases are not supported by combine-change-calls.  [ It's also the
> only use-case I could think of where using a simple pair of
> "delete+insert" undo entries would have made a difference.  ]

To be mulled over.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).