Re: [Emacs-diffs] master 682ab5d 2/2: Adjust previous electric.el and elec-pair.el change

Re: [Emacs-diffs] master 682ab5d 2/2: Adjust previous electric.el and elec-pair.el change

[Stefan Monnier]

>     This fixes a serious bug introduced previously
>     electric-pair-inhibit-if-helps-balance and
>     electric-pair-skip-if-helps-balance, whereby "innocent" markers were
>     being pushed by those function's new save-change-and-restore
>     semantics.  The fix can probably still be improved.

Could you add corresponding test cases?

> +  ;; FIXME: need this instead of `save-excursion' when functions in
> +  ;; BODY, such as `electric-pair-inhibit-if-helps-balance' and
> +  ;; `electric-pair-skip-if-helps-balance' modify and restore the
> +  ;; buffer in a way that modifies the marker used by save-excursion.

Then maybe a better approach is to use `undo`, as in the 100% untested
(and guaranteed incomplete) patch below,


        Stefan


diff --git a/lisp/elec-pair.el b/lisp/elec-pair.el
index b5ec492930..3be09d87b4 100644
--- a/lisp/elec-pair.el
+++ b/lisp/elec-pair.el
@@ -429,20 +429,25 @@ electric-pair-inhibit-if-helps-balance
 happened."
   (pcase (electric-pair-syntax-info char)
     (`(,syntax ,pair ,_ ,s-or-c)
-     (unwind-protect
-         (progn
-           (delete-char -1)
-           (cond ((eq ?\( syntax)
-                  (let* ((pair-data
-                          (electric-pair--balance-info 1 s-or-c))
-                         (outermost (cdr pair-data)))
-                    (cond ((car outermost)
-                           nil)
-                          (t
-                           (eq (cdr outermost) pair)))))
-                 ((eq syntax ?\")
-                  (electric-pair--unbalanced-strings-p char))))
-       (insert char)))))
+     (catch 'done
+       ;; FIXME: modify+undo is *very* tricky business.  We used to
+       ;; use `delete-char' followed by `insert', but this changed the
+       ;; position of some markers.  The real fix would be to compute the
+       ;; result without having to modify the buffer at all.
+       (atomic-change-group
+         (delete-char -1)
+         (throw
+          'done
+          (cond ((eq ?\( syntax)
+                 (let* ((pair-data
+                         (electric-pair--balance-info 1 s-or-c))
+                        (outermost (cdr pair-data)))
+                   (cond ((car outermost)
+                          nil)
+                         (t
+                          (eq (cdr outermost) pair)))))
+                ((eq syntax ?\")
+                 (electric-pair--unbalanced-strings-p char)))))))))
 
 (defun electric-pair-skip-if-helps-balance (char)
   "Return non-nil if skipping CHAR would benefit parentheses' balance.