Re: track-changes and undo

[Eli Zaretskii <eliz@gnu.org>]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: "'Help-Gnu-Emacs (help-gnu-emacs@gnu.org)'" <help-gnu-emacs@gnu.org>
> Date: Mon, 22 Apr 2024 19:04:10 -0400
> 
> >> The gain from `track-changes` is just to provide you with the "before"
> >> string for deletions so it takes care of reading it in
> >> `before-change-functions` and then providing it to you in the
> >> `after-change-functions` (with the advantage that it detects/handles the
> >> various corner cases where that pairing fails).
> > I think I'll try and see if I can make it work with the `:immediate` option.
> > It would of course mean that the buffer is modified inside
> > `after-change-functions`, which you warn against,
> 
> It wouldn't be worse than what you have now since you also modify the
> buffer from `before/after-change-functions`.
> 
> > but it looks like that's the only way.
> 
> In the general case it's tricky to postpone the buffer change to a safer
> time, indeed.  In practice, tho, you should be able to distinguish "undo
> commands" from all other commands (basically depending on whether they
> do all their modifications with `undo-in-progress` or not) and then
> ignore only the changes of undo commands: the result should be
> good enough.
> 
> IOW something like:
> 
>     (track-changes-register #'cm-change-signal :immediate t)
> 
>     (defvar-local cm-change-pending nil)
> 
>     (defun cm-change-signal (id)
>       (cond
>        (cm-change-pending nil) ;; Nothing to do, we're already waiting.
>        (undo-in-progress
>         ;; Just ignore this undo change.
>         (track-changes-fetch id #'ignore))
>        (t
>         (setq cm-change-pending
>               (run-with-timer 0 nil #'cm-change-do id)))))
>        
>     (defun cm-change-do (id)
>       (track-changes-fetch
>        (lambda (beg end before)
>          ..DO THE CriticalMarkup THING..
>          ;; Ignore the changes we just made.
>          (track-changes-fetch id #'ignore)
>          (setq cm-change-pending nil))))
> 
> >> One other thing that you might have trouble to reproduce with
> >> `track-changes` is the following test:
> >>
> >>     (and (= beg (point-min)) (= end (point-max)))
> >>
> >> that you have in `cm-before-change`.  I'm not completely sure what this
> >> is for, tho.  Is it for `revert-buffer`?
> >
> > I honestly don't remember... Based on the comment, it looks like
> > `switch-to-buffer` triggers `before-change-functions`, but a) that doesn't make
> > much sense; and b) the code seems to work just fine without that line. (I even
> > fired up a Vagrant box with an old Ubuntu release with Emacs 24, which would be
> > the most recent version when I wrote that code).
> 
> Have you tried `M-x revert-buffer RET` (assuming the file and the
> buffer aren't equal)?
> 
> > Making it a minor mode makes sense, of course.
> 
> It was already a minor mode, IMO, just one defined by hand instead of
> using the dedicated macro.

From the peanut gallery: if anything in this discussion should be
mentioned in the ELisp manual, where track-changes is documented,
please add those nits there.  For example, the whole issue of using
track-changes with undo should perhaps be mentioned there.

Thanks.