bug#5805: 23.3 abbrev-insert needs a limited save-excursion

[Bob Nnamtrop <bobnnamtrop@gmail.com>]

On Thu, Jul 7, 2011 at 3:46 PM, Bob Nnamtrop <bobnnamtrop@gmail.com> wrote:
> On Thu, Jul 7, 2011 at 2:59 PM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>> tags 5805 patch
>> thanks
>>
>>> I agree with the original poster that a save-excursion is needed in
>>> abbrev-expand and by making its scope limited (see below) it will not
>>> interfere with abbrevs that want to change point. The reason I find it
>>> annoying the way it is now is that some modes (e.g., idlwave) use
>>> abbrev's extensively to change keywords (e.g. capitalize them). These
>>> are defined without the leading "\". So if I am any number of lines
>>> with only whitespace below a keyword defined in this way (e.g. endfor)
>>> and run expand-abbrev then the point moves even if no visible change
>>> takes place. This is a VERY common occurrence for me since in viper
>>> mode changing from insert to vi mode runs expand-abbrev! Placing the
>>> save-excursion just around the part of abbrev-insert that actually
>>> expands the abbrev fixes this problem and does not limit abbrev's from
>>> moving the point (idlwave does this extensively as well and I've
>>> tested that it is not impacted).
>>
>> Hmm... sounds like an interesting solution.  I'll take a closer look and
>> get back to you.  Thank you.
>>
>>> Here is the diff against the
>>> abbrev.el in emacs 23.3 (note that whitespace not adjusted).
>>
>> Highly appreciated.
>>
>>
>>        Stefan
>>
>
> (SNIP)
> The rule should be "if a space (or any other non-word character) would
> not cause an abbrev to expand then exiting from viper insert state
> should not cause an abbrev to expand". And vise-versa. This would fix
> all the issues I have with abbrev.
> (SNIP)

Here are two patches that do what I explained above. I am new to elisp
so be kind :-) Note that these apply against the trunk (sorry to be
switching back of forth). I left in the save-excursion fix but it is
not absolutely necessary at this point but I still think it is the
right thing to do. Let me know what you think.

Bob

--- trunk/lisp/emulation/viper-cmd.el   2011-07-07 13:00:22.000000000 -0600
+++ local-fixes/lisp/emulation/viper-cmd.el     2011-07-07
17:45:14.000000000 -0600
@@ -617,7 +617,7 @@
     (or (viper-overlay-p viper-replace-overlay)
       (viper-set-replace-overlay (point-min) (point-min)))
     (viper-hide-replace-overlay)
-    (if abbrev-mode (expand-abbrev))
+    (if abbrev-mode (expand-abbrev t))
     (if (and auto-fill-function (> (current-column) fill-column))
        (funcall auto-fill-function))
     ;; don't leave whitespace lines around

--- trunk/lisp/abbrev.el        2011-07-07 13:00:22.000000000 -0600
+++ local-fixes/lisp/abbrev.el  2011-07-07 17:49:26.000000000 -0600
@@ -752,6 +752,7 @@
     ;; If this abbrev has an expansion, delete the abbrev
     ;; and insert the expansion.
     (when (stringp (symbol-value abbrev))
+      (save-excursion
       (goto-char wordstart)
       ;; Insert at beginning so that markers at the end (e.g. point)
       ;; are preserved.
@@ -780,7 +781,7 @@
               (skip-syntax-forward "^w" (1- end))
               ;; Change just that.
               (upcase-initials-region (point) (1+ (point)))
-              (goto-char end))))))
+              (goto-char end)))))))
     ;; Now point is at the end of the expansion and the beginning is
     ;; in last-abbrev-location.
     (when (symbol-function abbrev)
@@ -804,16 +805,18 @@
 a function that performs the abbrev expansion.  It should return
 the abbrev symbol if expansion took place.")

-(defun expand-abbrev ()
+(defun expand-abbrev (&optional strict)
   "Expand the abbrev before point, if there is an abbrev there.
 Effective when explicitly called even when `abbrev-mode' is nil.
+If strict is t then point must be directly after then end of the
+abbrev for the expansion to take place.
   (interactive)
   (run-hooks 'pre-abbrev-expand-hook)
   (with-wrapper-hook abbrev-expand-functions ()
     (destructuring-bind (&optional sym name wordstart wordend)
         (abbrev--before-point)
-      (when sym
+      (when (and sym (or (not strict) (= wordend (point))))
         (unless (or ;; executing-kbd-macro
                  noninteractive
                  (window-minibuffer-p (selected-window)))