* Added a new section "Field-level undo functionality" to correct

issue 33.

* Added functions `yas/field-undo-before-hook',
  `yas/field-undo-after-hook', `yas/field-undo-group-text-change' and
  variables `yas/field-undo-history',
  `yas/field-undo-forbidden-commands' to implement field-level undo.

* Modified `yas/cleanup-snippet' to record the actual snippet end
  in the snippet before cleaning up the main overlay.

* New hooks `yas/field-undo-before-hook' and
  `yas/field-undo-after-hook' added to `pre-command-hook' and
  `post-command-hook', respectively.

* Slightly changed the commentary of the "Snippet register and
  unregister routines section"

* Modified step 12 to call `yas/undo-expand-snippet' without the
  `(point-min)' argument, which led to incorrect undo behaviour.

* Added a new step in `yas/expand-snippet' describing the local
  restore and save of undo infomation in a new slot of the
  `yas/snippet' structure. This is step 14, previous 14 was moved to 15.

* Modified `yas/undo-expand-snippet' to kill text up to the the
  correct snippet end, which was not being calculated correctly. It is
  now a slot of the snippet itself and no longer a parameter.

* Added functions `yas/current-field-text',
  `yas/replace-fields-with-value',
  `yas/remove-recent-undo-from-history' to refactor some code used
  twice or more.

* Modified `yas/check-cleanup-snippet' to detect trigger cleanup
  if point exits any primary snippet field, of any
  group. Previously cleanup would only be triggered on exiting
  all groups. This is a follow-up to issue 28.

* Added two slot to the `yas/snippet' structure to allow for
  field-level undo and correct small bug in the previous fix of issue
  28.

* Modified `yas/cleanup-snippet' to call
  `yas/after-exit-snippet-hook' as a follow-up to feedback on
  issue 28.

* (Accidentaly :) changed (but corrected!) indentation of most
  existing functions. This was done by a buffer global indent-region.

* (Accidentaly :) ran `checkdoc' but then gave up, it's
  uncessary. Didn't change much stuff though.

yasnippet.el

@@ -113,7 +113,7 @@ a window system.")
 (defvar yas/extra-mode-hooks
   '()
   "A list of mode-hook that should be hooked to enable yas/minor-mode.
-Most modes need no special consideration. Some mode (like ruby-mode)
+Most modes need no special consideration.  Some mode (like `ruby-mode')
 doesn't call `after-change-major-mode-hook' need to be hooked explicitly.")
 (mapc '(lambda (x)
          (add-to-list 'yas/extra-mode-hooks
@@ -218,7 +218,7 @@ to expand.
           "${\\(?:\\([0-9]+\\):\\)?\\([^}]*\\)}"))
 
 (defvar yas/snippet-id-seed 0
-  "Contains the next id for a snippet")
+  "Contains the next id for a snippet.")
 (defun yas/snippet-next-id ()
   (let ((id yas/snippet-id-seed))
     (incf yas/snippet-id-seed)
@@ -287,11 +287,29 @@ set to t."
   name
   condition)
 (defstruct (yas/snippet (:constructor yas/make-snippet ()))
-  "A snippet."
+  "A snippet.
+
+Description of some fields:
+
+`yas/snippet-saved-buffer-undo-list' saves the value of
+`buffer-undo-list' just after the snippet has been expanded. This
+is to be restored when the snippet is cleaned up. Thus the
+snippet expansion can still be undone after
+`yas/cleanup-snippet', even if field-level undo steps were
+recorded.
+
+`yas/snippet-end-marker' saves the actual end position of the
+snippets main overlay, at the time the snippet was cleaned
+up. Thus `yas/undo-expand-snippet' can clean it up properly.
+
+TODO: describe the rest of the fields"
   (groups nil)
   (exit-marker nil)
   (id (yas/snippet-next-id) :read-only t)
-  (overlay nil))
+  (overlay nil)
+  (saved-buffer-undo-list nil)
+  (end-marker nil))
+
 (defstruct (yas/group (:constructor yas/make-group (primary-field snippet)))
   "A group contains a list of field with the same number."
   primary-field
@@ -338,11 +356,11 @@ set to t."
        (yas/group-primary-field group))
       ""))
 (defun yas/group-number (group)
-  "Get the number of the field group."
+  "Get the number of the field GROUP."
   (yas/field-number
    (yas/group-primary-field group)))
 (defun yas/group-add-field (group field)
-  "Add a field to the field group. If the value of the primary 
+  "Add a FIELD to the field GROUP. If the value of the primary
 field is nil and that of the field is not nil, the field is set
 as the primary field of the group."
   (push field (yas/group-fields group))
@@ -513,27 +531,24 @@ the template of a snippet in the current snippet-table."
     (save-excursion
       (let* ((inhibit-modification-hooks t)
              (primary (yas/group-primary-field field-group))
-	     (primary-overlay (yas/field-overlay primary))
+             (text (yas/current-field-text primary)))
-	     (text (buffer-substring-no-properties (overlay-start primary-overlay)
+        ;; For all fields except the primary, replace their text
-						   (overlay-end primary-overlay))))
+        (yas/replace-fields-with-value (remove-if #'(lambda (field)
-	(dolist (field (yas/group-fields field-group))
+                                                      (equal field primary))
-	  (let* ((field-overlay (yas/field-overlay field))
+                                                  (yas/group-fields field-group))
-		 (original-length (- (overlay-end field-overlay)
+                                       text)))))
-				     (overlay-start field-overlay))))
+(defun yas/current-field-text (field)
-	    (unless (eq field-overlay primary-overlay)
+  (let ((primary-overlay (yas/field-overlay field)))
-	      (goto-char (overlay-start field-overlay))
+    (when primary-overlay
-	      (insert (yas/calculate-field-value field text))
+      (buffer-substring-no-properties (overlay-start primary-overlay)
-	      (if (= (overlay-start field-overlay)
+                                      (overlay-end primary-overlay)))))
-		     (overlay-end field-overlay))
+
-		  (move-overlay field-overlay
-				(overlay-start field-overlay)
-				(point))
-		(delete-char original-length)))))))))
 
 (defun yas/overlay-modification-hook (overlay after? beg end &optional length)
   "Modification hook for snippet field overlay."
   (when (and after? (not undo-in-progress))
     (yas/synchronize-fields (overlay-get overlay 'yas/group))))
+
 (defun yas/overlay-insert-in-front-hook (overlay after? beg end &optional length)
   "Hook for snippet overlay when text is inserted in front of a snippet field."
   (when after?
@@ -546,6 +561,7 @@ the template of a snippet in the current snippet-table."
             (goto-char end)
             (delete-char (- (overlay-end overlay) end)))))
       (yas/synchronize-fields field-group))))
+
 (defun yas/overlay-maybe-insert-behind-hook (overlay after? beg end &optional length)
   "Insert behind hook sometimes doesn't get called. I don't know why.
 So I add modification hook in the big overlay and try to detect `insert-behind'
@@ -584,21 +600,41 @@ event manually."
                          (yas/synchronize-fields group)
                          (setq done t)))))))))))
 
-(defun yas/undo-expand-snippet (start end key snippet)
+(defun yas/remove-recent-undo-from-history ()
-  "Undo a snippet expansion. Delete the overlays. This undo can't be
-redo-ed."
   (let ((undo (car buffer-undo-list)))
     (while (null undo)
       (setq buffer-undo-list (cdr buffer-undo-list))
       (setq undo (car buffer-undo-list)))
     ;; Remove this undo operation record
-    (setq buffer-undo-list (cdr buffer-undo-list))
+    (setq buffer-undo-list (cdr buffer-undo-list))))
+
+(defun yas/undo-expand-snippet (start key snippet)
+  "Undo a snippet expansion. Delete the overlays. This undo can't be
+redo-ed."
+  (yas/remove-recent-undo-from-history)
   (let ((inhibit-modification-hooks t)
         (buffer-undo-list t))
     (yas/exit-snippet snippet)
     (goto-char start)
-    (delete-char (- end start))
+    (delete-char (- (yas/snippet-end-marker snippet)
-    (insert key))))
+                    start))
+    (insert key)))
+
+(defun yas/replace-fields-with-value (fields text)
+  "In all of the fields of the snippet group GROUP fields, delete
+whatever value (string) existed and insert TEXT instead.
+
+The string to insert is calculated according to
+`yas/calculate-field-value', which might insert different text
+for each field."
+  (dolist (field fields)
+    (let* ((overlay (yas/field-overlay field))
+           (start (overlay-start overlay))
+           (end (overlay-end overlay))
+           (length (- end start)))
+      (goto-char start)
+      (insert (yas/calculate-field-value field text))
+      (delete-char length))))
 
 (defun yas/expand-snippet (start end template)
   "Expand snippet at current point. Text between START and END
@@ -608,14 +644,14 @@ will be deleted before inserting template."
   (goto-char start)
 
   (let ((key (buffer-substring-no-properties start end))
-	(original-undo-list buffer-undo-list)
+        (original-undo-list buffer-undo-list) ;; save previous undo information
         (inhibit-modification-hooks t)
         (length (- end start))
         (column (current-column)))
     (save-restriction
       (narrow-to-region start start)
 
-      (setq buffer-undo-list t)
+      (setq buffer-undo-list t) ;; disable undo for a short while
       (insert template)
 
       ;; Step 1: do necessary indent
@@ -702,19 +738,13 @@ will be deleted before inserting template."
                        yas/keymap-overlay-modification-hooks)
           (overlay-put overlay 'keymap yas/keymap)
           (overlay-put overlay 'yas/snippet-reference snippet)
-	  (setf (yas/snippet-overlay snippet) overlay))
+          (setf (yas/snippet-overlay snippet) overlay)
+          (setf (yas/snippet-end-marker snippet) (overlay-end overlay)))
 
         ;; Step 8: Replace fields with default values
         (dolist (group (yas/snippet-groups snippet))
-	  (let ((value (yas/group-value group)))
+          (yas/replace-fields-with-value (yas/group-fields group)
-	    (dolist (field (yas/group-fields group))
+                                         (yas/group-value group)))
-	      (let* ((overlay (yas/field-overlay field))
-		     (start (overlay-start overlay))
-		     (end (overlay-end overlay))
-		     (length (- end start)))
-		(goto-char start)
-		(insert (yas/calculate-field-value field value))
-		(delete-char length)))))
 
         ;; Step 9: restore all escape characters
         (yas/replace-all yas/escape-dollar "$")
@@ -747,7 +777,6 @@ will be deleted before inserting template."
           (add-to-list 'original-undo-list
                        `(apply yas/undo-expand-snippet
                                ,(point-min)
-			       ,(point-max)
                                ,key
                                ,snippet)))
 
@@ -755,9 +784,11 @@ will be deleted before inserting template."
         (widen)
         (delete-char length)
 
+        ;; Step 14: Restore undo information, and also save it for future use.
+        (setf (yas/snippet-saved-buffer-undo-list snippet) original-undo-list)
         (setq buffer-undo-list original-undo-list)
 
-	;; Step 14: place the cursor at a proper place
+        ;; Step 15: place the cursor at a proper place
         (let ((groups (yas/snippet-groups snippet))
               (exit-marker (yas/snippet-exit-marker snippet)))
           (if groups
@@ -773,6 +804,8 @@ will be deleted before inserting template."
   (let ((point (or point (point)))
         (snippet-overlay nil))
     (dolist (overlay (overlays-at point))
+      ;; appending and removing-duplicates fixes a bug when overlays
+      ;; are not recognized because point is really at the end
       (when (overlay-get overlay 'yas/snippet)
         (if (null snippet-overlay)
             (setq snippet-overlay overlay)
@@ -1247,11 +1280,11 @@ when the condition evaluated to non-nil."
           (message "Not in a snippet field."))))))
 
 (defun yas/exit-snippet (snippet)
-  "Goto exit-marker of SNIPPET and delete the snippet."
+  "Goto exit-marker of SNIPPET and cleanup the snippe.  Cleaning
+up the snippet does not delete it!"
   (interactive)
   (goto-char (yas/snippet-exit-marker snippet))
-  (yas/cleanup-snippet snippet)
+  (yas/cleanup-snippet snippet))
-  (run-hooks 'yas/after-exit-snippet-hook))
 
 ;; Snippet register and unregister routines.
 ;;
@@ -1269,20 +1302,12 @@ when the condition evaluated to non-nil."
 ;; They were introduced to fix bug 28
 ;; "http://code.google.com/p/yasnippet/issues/detail?id=28".  Whenever
 ;; point exits a snippet or a snippet field, *all* snippets are
-;; destroyed. I think this scheme can also fix other bugs or
+;; destroyed.
-;; introduce new features. It can also be used with `pre-command-hook'
-;; for instance.
 ;;
-;; For example, to only allow one snippet at a time, add a similar
+;; Also, this scheme have been reused to fix bug 33
-;; check to `pre-command-hook' that immediately destroys all
+;; "http://code.google.com/p/yasnippet/issues/detail?id=33", which
-;; snippets if `this-command' equals `yas/snippet-expand'.
+;; deals with undoing changes when part of the snippet's field have
-;;
+;; been filled out already.  See commentary on "Field-level undo" below
-;; Or also using `pre-command-hook', exterminate all snippets if
-;; `this-command' is `undo' and some snippet fields have already been
-;; partially filled out. This would at least work around issue 33,
-;; which complains about undo.
-;;
-;; Tell me what you think!
 ;;
 
 (defvar yas/registered-snippets nil
@@ -1296,7 +1321,9 @@ when the condition evaluated to non-nil."
 `post-command-hook' that should exist while at least one
 registered snippet exists in the current buffer.  Return snippet"
   (puthash (yas/snippet-id snippet) snippet yas/registered-snippets)
+  (add-hook 'pre-command-hook  'yas/field-undo-before-hook            'append 'local)
   (add-hook 'post-command-hook 'yas/check-cleanup-snippet        'append 'local)
+  (add-hook 'post-command-hook 'yas/field-undo-after-hook             'append 'local)
   snippet)
 
 (defun yas/unregister-snippet (snippet)
@@ -1307,6 +1334,8 @@ current buffer."
   (remhash (yas/snippet-id snippet) yas/registered-snippets)
   (when (eq 0
             (hash-table-count yas/registered-snippets))
+    (remove-hook 'pre-command-hook  'yas/field-undo-before-hook     'local)
+    (remove-hook 'post-command-hook 'yas/field-undo-after-hook      'local)
     (remove-hook 'post-command-hook 'yas/check-cleanup-snippet 'local)))
 
 (defun yas/exterminate-snippets ()
@@ -1322,11 +1351,18 @@ snippet as ordinary text"
   (let* ((overlay (yas/snippet-overlay snippet))
          (yas/snippet-beg (overlay-start overlay))
          (yas/snippet-end (overlay-end overlay)))
-    (delete-overlay overlay)
+    ;; save the end of the moribund snippet in case we need to undo
+    ;; its original expansion.  This is used by `yas/undo-expand-snippet'
+    (when (and overlay
+               (overlay-buffer overlay))
+      (setf (yas/snippet-end-marker snippet) yas/snippet-end)
+      (delete-overlay overlay))
     (dolist (group (yas/snippet-groups snippet))
       (dolist (field (yas/group-fields group))
-	(delete-overlay (yas/field-overlay field)))))
+        (delete-overlay (yas/field-overlay field))))
-    (yas/unregister-snippet snippet))
+    (run-hooks 'yas/after-exit-snippet-hook))
+  (yas/unregister-snippet snippet)
+  (setq buffer-undo-list (yas/snippet-saved-buffer-undo-list snippet)))
 
 (defun yas/check-cleanup-snippet ()
   "Checks if point exited any of the fields of the snippet, if so
@@ -1335,34 +1371,140 @@ clean it up.
 This function is part of `post-command-hook' while
 registered snippets last."
   (let ((snippet (yas/snippet-of-current-keymap)))
-    (cond (;;
+    (cond ( ;;
            ;; No snippet at point, cleanup *all* snippets
            ;;
            (null snippet)
            (yas/exterminate-snippets))
-	  (;;
+          ( ;;
            ;; A snippet exits at point, but point is out of any
-	   ;; snippet field.
+           ;; primary snippet field.
-	   ;;
-	   ;; XXX: `every' and `notany' should be much slower than a
-	   ;; couple of `while' loops...
-	   ;; 
            (and snippet
-		(every #'(lambda (group)
+                (notany #'(lambda (group)
-			   (notany #'(lambda (field)
+                            (let ((primary-overlay (yas/field-overlay (yas/group-primary-field group))))
-				       (let ((overlay (yas/field-overlay field)))
+                              (and (>= (point) (overlay-start primary-overlay))
-					 (and (>= (point) (overlay-start overlay))
+                                   (<= (point) (overlay-end primary-overlay)))))
-					      (<= (point) (overlay-end overlay)))))
-				   (yas/group-fields group)))
                         (yas/snippet-groups snippet)))
            (yas/cleanup-snippet snippet))
-	  (;;
+          ( ;;
            ;; Snippet at point, and point inside a snippet field,
            ;; everything is normal
            ;;
            t
            nil))))
 
+;; Field-level undo functionality
+;;
+;; XXX: Commentary on this section by joaot.
+;;
+;; "Field-level undo" means undoing for bits of snippet fields that have
+;; already been filled out.  Because this is kind of experimental, I
+;; have called it "field-undo", to distinguish it from regular undo
+;; like the one used by `yas/undo-expand-snippet' to undo the original
+;; snippet expansion.
+;;
+;; Field level undo allows no redos.  Also, field level undo undoes any
+;; change, even if it is only one character long.  This might be
+;; implemented in the future.
+;;
+;; Field level undo cooperates with normal undo and seems transparet
+;; to the `undo' command.  The basic idea is the same as with snippet
+;; registration/unregistration.  The undo history is saved in
+;; `yas/field-undo-original-history' before each command and rewritten
+;; if appropriate at the end.
+;;
+;; This is done by registering `yas/field-undo-before-hook' and
+;; `yas/field-undo-after-hook' in the `pre-command-hook' and
+;; `post-command-hook', respectively.
+;;
+;; Also, the `value' slot of the primary field of each group is used
+;; to keep track of the most recently inserted text of that snippet
+;; field.  This could be seen as a hack, but that slot wasn't being
+;; used anyway and its new meaning is actually quite reasonable.
+;;
+;; Another detail is that undo informatino shoulnd't be recorded for
+;; some commands, most notably `undo' itself.  Therefore, a variable
+;; `yas/field-undo-forbidden-commands' has been introduced, to be
+;; tested agains `this-command'.
+;;
+
+(defvar yas/field-undo-history nil
+  "Saves the value of `buffer-undo-list' when undo information is
+to be recorded by `yas/field-undo-after-hook'.  A new piece of undo
+is pushed into this variable and it then replaces
+`buffer-undo-list' if appropriate.")
+
+(defvar yas/field-undo-forbidden-commands '(undo aquamacs-undo redo aquamacs-redo)
+  "A list of commands executed while a snippet is active that
+should not trigger any undo-recording action")
+
+(defun yas/field-undo-before-hook ()
+  "Saves the field-level undo history, `buffer-undo-list' into a global
+`yas/field-undo-history' variable just before a command is
+performed.  It will come in handy in case the command is to be undone"
+  (setq yas/field-undo-history buffer-undo-list))
+
+(defun yas/field-undo-after-hook ()
+  "Compares the value (a string) of the currently active snippet
+group with a previously saved one.  If these are different, undo
+information is added to `buffer-undo-list'
+
+This function is added to the `post-command-hook' and should
+be a part of that list while registered snippets last."
+  (let* ((overlay (or (yas/current-snippet-overlay)
+                      (yas/current-snippet-overlay (1- (point)))))
+         (group (when overlay
+                  (overlay-get overlay 'yas/group))))
+    (when group
+      (let ((new-text (yas/current-field-text (yas/group-primary-field group)))
+            (old-text (yas/field-value (yas/group-primary-field group))))
+        ;;
+        ;; Unless extended undo forbids `this-command', or the old and
+        ;; new field strings are the same, rewrite the undo history
+        ;; with a call to `yas/field-undo-group-text-change'
+        ;; instead of whatever was placed there by the currently
+        ;; finishing `this-command' command. This call receives the id
+        ;; of the currently active snippet, the group to be undone and
+        ;; the old text.
+        ;;
+        (unless (or (memq this-command yas/field-undo-forbidden-commands)
+                    (string= new-text
+                             old-text))
+          ;;
+          ;; Push a separator onto the history list, if one wasn't
+          ;; there first. Have no clue why sometimes one is and one
+          ;; isn't.
+          ;;
+          (unless (null (car yas/field-undo-history))
+            (push nil yas/field-undo-history))
+          (push `(apply yas/field-undo-group-text-change
+                        ,group
+                        ,old-text)
+                yas/field-undo-history)
+          (setq buffer-undo-list yas/field-undo-history))
+        ;;
+        ;; Then, in any case, save the new text into the value slot of
+        ;; the primary this is because some "forbidden" commands might
+        ;; really have changed the field value, most notably `undo'
+        ;; itself! This was a hard bug to track down!
+        ;;
+        (setf (yas/field-value (yas/group-primary-field group)) new-text)))))
+
+(defun yas/field-undo-group-text-change (group old-text)
+  "Undoes one step of field-level undo history, in the snippet
+  field group GROUP, replacing its text with OLD-TEXT, but
+  respecting any transforms."
+  (yas/remove-recent-undo-from-history)
+  (let ((inhibit-modification-hooks t)  ; otherwise an additional
+                                        ; `yas/replace-fields-with-value'
+                                        ; is called
+        (buffer-undo-list t))
+    (yas/replace-fields-with-value
+     (yas/group-fields group)
+     old-text)))
+
+;; Debug functions.  Use (or change) at will whenever needed.
+
 (defun yas/debug-some-vars ()
   (interactive)
   (with-output-to-temp-buffer "*YASnippet trace*"
@@ -1375,12 +1517,27 @@ registered snippets last."
                         (princ (format "\t key %s for snippet %s with %s  groups\n"
                                        key
                                        (yas/snippet-id snippet)
-				       (length (yas/snippet-groups snippet)))))
+                                       (length (yas/snippet-groups snippet))))
+                        (dolist (group (yas/snippet-groups snippet))
+                          (princ (format "\t   group with %s fields.  Primary field is value is \"%s\"\n"
+                                         (length (yas/group-fields group))
+                                         (yas/field-value (yas/group-primary-field group))))))
                     yas/registered-snippets)))
-    (princ (format "\nPost command hook: %s\n" post-command-hook))
-    (princ (format "\nPre  command hook: %s\n" pre-command-hook))))
 
-;;(run-hooks 'yas/after-exit-snippet-hook)))) ;;; XXX: why was this here at top level?
+    (princ (format "\nPost command hook: %s\n" post-command-hook))
+    (princ (format "\nPre  command hook: %s\n" pre-command-hook))
+
+    (princ (format "\nUndo is %s. Undolist has %s elements. First 10 elements follow:\n"
+                   (if (eq buffer-undo-list t)
+                       "DISABLED"
+                     "ENABLED")
+                   (length buffer-undo-list)))
+    (let ((undo-list buffer-undo-list))
+      (dotimes (i 10)
+        (when undo-list
+          (princ (format "%s:  %s\n" i (car undo-list)))
+          (setq undo-list (cdr undo-list)))))))
+
 
 (provide 'yasnippet)