Re: track-changes and undo

[Stefan Monnier <monnier@iro.umontreal.ca>]

[-- Attachment #1: Type: text/plain, Size: 1466 bytes --]

> The short version: The package makes it easier to add CriticMarkup to a
> document, i.e., markup to indicate additions, deletions, etc. There are
> keybindings for the various markups (`{++...++}` for additions, `{--...--}` for
> deletions, etc.).
>
> The follow-changes mode tries to do that automatically: if you delete a word,
> instead of actually deleting it, the word is enclosed in deletion markup.

Thanks,

Yes, that makes a lot of sense and checking `undo-in-progress` is
indispensable to make undo work.  With the current API (if you want to
use `track-changes`, that is) the best you can do is to use the
`:immediate` option so that your code gets called directly from the
`after-change-functions` where you can (still) see the
`undo-in-progress`.

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).

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 tend to do "destructive reads", so the patch below is the result of
  reading that part of your code.  ]


        Stefan

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: criticmarkup.diff --]
[-- Type: text/x-diff, Size: 2963 bytes --]

diff --git a/cm-mode.el b/cm-mode.el
index 43c705a..882f77d 100644
--- a/cm-mode.el
+++ b/cm-mode.el
@@ -862,43 +862,31 @@ substitutions, `d' for comments and highlights."
 
 ;;; Follow Changes
 
-(defvar cm-follow-changes nil
-  "Flag indicating whether follow changes mode is active.")
-(make-variable-buffer-local 'cm-follow-changes)
-
 (defvar cm-current-deletion nil
   "The deleted text in follow changes mode.
 The value is actually a list consisting of the text and a flag
 indicating whether the deletion was done with the backspace
 key.")
 
-(defun cm-follow-changes (&optional arg)
-  "Activate follow changes mode.
-If ARG is positive, activate follow changes mode, if ARG is 0 or
-negative, deactivate it.  If ARG is `toggle', toggle follow
-changes mode."
-  (interactive (list (or current-prefix-arg 'toggle)))
-  (let ((enable (if (eq arg 'toggle)
-                    (not cm-follow-changes)
-                  (> (prefix-numeric-value arg) 0))))
-    (if enable
-        (progn
-          (add-to-list 'before-change-functions 'cm-before-change t)
-          (add-to-list 'after-change-functions 'cm-after-change)
-          (setq cm-follow-changes t)
-          (message "Follow changes mode activated."))
-      (setq before-change-functions (delq 'cm-before-change before-change-functions))
-      (setq after-change-functions (delq 'cm-after-change after-change-functions))
-      (setq cm-follow-changes nil)
-      (message "Follow changes mode deactivated."))))
+(define-minor-mode cm-follow-changes  ;FIXME: Shouldn't it end in `mode'?
+  "Minor mode to follow changes."
+  :global nil
+  (if cm-follow-changes
+      (progn
+        (add-hook 'before-change-functions #'cm-before-change t t)
+        (add-hook 'after-change-functions #'cm-after-change nil t))
+    (remove-hook 'before-change-functions #'cm-before-change t)
+    (remove-hook after-change-functions #'cm-after-change t)))
 
 (defun cm-before-change (beg end)
   "Function to execute before a buffer change.
 BEG and END are the beginning and the end of the region to be
 changed."
   (unless (or undo-in-progress
+              ;; FIXME: What do you mean by "buffer switches"?
               (and (= beg (point-min)) (= end (point-max))))  ; This happens on buffer switches.
     (if (= beg end)  ; Addition.
+        ;; FIXME: There can be corner cases where point is not at beg/end.
         (cm-make-addition (cm-markup-at-point))
       ;; When the deletion was done with backspace, point is at end. We record
       ;; this in `cm-current-deletion' so we can position point correctly.
@@ -906,7 +894,7 @@ changed."
 
 (defun cm-after-change (beg end length)
   "Function to execute after a buffer change.
-This function marks deletions.  See cm-before-change for details.
+This function marks deletions.  See `cm-before-change' for details.
 BEG and END mark the region to be changed, LENGTH is the length
 of the affected text."
   (unless (or undo-in-progress