Slightly improve on fix for #515: Refactor code and add tests

While I agree with the solution for #515 in commit 17fe790 for now,
other approaches might be interesting in the future, like clearing on
other types of insertion, not just self-insertion.

So add some tests and refactor a new function that could be used for
that purpose later on. For now, change no behaviour.

* yasnippet-tests.el (dont-clear-on-partial-deletion-issue-515,
  dont-clear-on-yank-issue-515): New tests. (yas-mock-yank): New
  function.

* yasnippet.el (yas--skip-and-clear-field-p): New function.
  (yas--on-field-overlay-modification): Use it.

yasnippet-tests.el

@@ -142,6 +142,39 @@
 ;;     (should (string= (yas--buffer-contents)
 ;;                      "brother from another mother!"))))
 
+(ert-deftest dont-clear-on-partial-deletion-issue-515 ()
+  "Ensure fields are not cleared when user doesn't really mean to."
+  (with-temp-buffer
+    (yas-minor-mode 1)
+    (yas-expand-snippet "my ${1:kid brother} from another ${2:mother}")
+
+    (ert-simulate-command '(kill-word 1))
+    (ert-simulate-command '(delete-char 1))
+
+    (should (string= (yas--buffer-contents)
+                     "my brother from another mother"))
+    (should (looking-at "brother"))
+
+    (ert-simulate-command '(yas-next-field))
+    (should (looking-at "mother"))
+    (ert-simulate-command '(yas-prev-field))
+    (should (looking-at "brother"))))
+
+(ert-deftest dont-clear-on-yank-issue-515 ()
+  "A yank shouldn't clear and unmodified field." ; or should it? -- jt
+  (with-temp-buffer
+    (yas-minor-mode 1)
+    (yas-expand-snippet "my ${1:kid brother} from another ${2:mother}")
+
+    (yas-mock-yank "little")
+    (yas-mock-insert " ")
+
+    (should (string= (yas--buffer-contents)
+                     "my little kid brother from another mother"))
+    (ert-simulate-command '(yas-next-field))
+    (ert-simulate-command '(yas-prev-field))
+    (should (looking-at "little kid brother"))))
+
 
 ;;; Snippet expansion and character escaping
 ;;; Thanks to @zw963 (Billy) for the testing
@@ -814,6 +847,10 @@ add the snippets associated with the given mode."
     (let ((last-command-event (aref string i)))
       (ert-simulate-command '(self-insert-command 1)))))
 
+(defun yas-mock-yank (string)
+  (let ((interprogram-paste-function (lambda () string)))
+    (ert-simulate-command '(yank nil))))
+
 (defun yas-make-file-or-dirs (ass)
   (let ((file-or-dir-name (car ass))
         (content (cdr ass)))

yasnippet.el

@@ -3385,7 +3385,14 @@ Move the overlay, or create it if it does not exit."
     (overlay-put yas--active-field-overlay 'insert-behind-hooks
                  '(yas--on-field-overlay-modification))))
 
-(defun yas--on-field-overlay-modification (overlay after? _beg _end &optional _length)
+(defun yas--skip-and-clear-field-p (field _beg _end &optional _length)
+  "Tell if newly modified FIELD should be cleared and skipped.
+BEG, END and LENGTH like overlay modification hooks."
+  (and (eq this-command 'self-insert-command)
+       (not (yas--field-modified-p field))
+       (= (point) (yas--field-start field))))
+
+(defun yas--on-field-overlay-modification (overlay after? beg end &optional length)
   "Clears the field and updates mirrors, conditionally.
 
 Only clears the field if it hasn't been modified and point is at
@@ -3401,9 +3408,7 @@ field start.  This hook does nothing if an undo is in progress."
                (yas--field-update-display field))
              (yas--update-mirrors snippet))
             (field
-             (when (and (eq this-command 'self-insert-command)
+             (when (yas--skip-and-clear-field-p field beg end)
-                        (not (yas--field-modified-p field))
-                        (= (point) (yas--field-start field)))
                (yas--skip-and-clear field))
              (setf (yas--field-modified-p field) t))))))