bug#50236: 27.2; electric-pair-mode is inconvenient in comint
Andrew Hyatt <[email protected]> Wed, 05 Aug 2026 21:59:51 -0400
| Newsgroups | gmane.emacs.bugs |
|---|---|
| Message-ID | <[email protected]> |
João Távora <[email protected]> writes: > Andrew Hyatt <[email protected]> writes: > >> On Mon, Aug 3, 2026 at 5:37 AM João Távora <[email protected]> wrote: >> >> Did someone test the candidate code with SLY's current e-p-m and comint integration? Is there any reason to think it >> might break? I hope not. >> >> I think as long as SLY doesn't use fields in some strange way, it should be OK. I haven't tested with SLY but if you let >> me know what e-p-m is and how to test it, I can test it out. > > e-p-m is electric-pair-mode. To test with SLY you need a Common Lisp > implementation, such as sbcl. Then it could (should?) be as easy as > > sudo pacman -S sbcl # or however you install packages, this is for ArchLinux > git clone https://github.com/joaotavora/sly.git > cd sly > path/to/patched/emacs -Q -L . -l sly-autoloads -f eletric-pair-mode -f sly > > which should land you in a SLY Lisp REPL (not unlike Elisp's IELM). > > CL-USER> (prin1 "blabla ( forgot to close") > "blabla ( forgot to close" > "blabla ( forgot to close" > CL-USER> (closing a parenthesis here should skip, despite the output) > > That's it. Got it. I've performed the test, and indeed closing the parens correctly skipped past the closing parens that electric-pair inserted, instead of inserting a new one. >> >> >> If SLY decides to migrate to the new style of comint integration (presuming there is one, at least that's where I saw >> the discussion headed) is there a manual or example to follow? >> >> I don't think there's anything SLY would need to do, as long as it is using comint in the normal way, which automatically >> uses different fields for non-user input and prompts, and user-inputs >> are not in a field at all. > > That's nice :-) > > As far as I understand, you are confident your patch is 100% > backward-compatible, right? Also, do you think you could code it such > that the highlights only the restriction/wrapping part? I'm somewhat but not completely confident. It passes the tests, and appears to do the correct thing and take into account how electric-pair works on regions. It's certainly possible there's something I overlooked; it's my first time working with electric-pair. > > In other words, can you patch be expressed in term of a diff such as > this one? > > @@ -597,6 +597,11 @@ electric-pair-default-inhibit > (defun electric-pair-post-self-insert-function () > + (with-suitable-restriction-thingies () > + (let ((foo 42)...) > + (electric-pair--post-self-insert-function-1)))) > + > +(defun electric-pair--post-self-insert-function-1 () > "Do main work for `electric-pair-mode'. > This function is added to `post-self-insert-hook' when > `electric-pair-mode' is enabled. > > If so, it'll be much easier to review and to understand by future > maintainers. Yes, I can create a macro that does the restriction, which may help readability. I've done so in a new patch which is now attached. > > João
0001-electric-pair-fix-v3.patch
(text/x-patch, 13.1 KB)
diff --git a/doc/emacs/programs.texi b/doc/emacs/programs.texi
index 7cf7b2ff96f..5f492c22970 100644
--- a/doc/emacs/programs.texi
+++ b/doc/emacs/programs.texi
@@ -1129,7 +1129,10 @@ Matching
buffer. And typing a closing delimiter immediately before another
closing delimiter of the same type does not insert that character but
moves point as described above, even when there is an unpaired matching
-opening delimiter earlier in the buffer.
+opening delimiter earlier in the buffer. Matching does not consider the
+whole buffer, only the current field (if the field is less than size
+@code{electric-pair-max-field-size}), so for example, previous input or
+non-user input in comint buffers will usually not be included.
If there is an active region, this variable has no effect.
diff --git a/lisp/elec-pair.el b/lisp/elec-pair.el
index 54fbc960b0f..38fa6bb57e0 100644
--- a/lisp/elec-pair.el
+++ b/lisp/elec-pair.el
@@ -228,6 +228,16 @@ electric-pair-skip-whitespace-chars
(const :tag "Newline" ?\n))
(repeat (character :value " "))))
+(defvar-local electric-pair-max-field-size 1000
+ "Maximum number of characters to search for a field boundary.
+
+This is used to limit how far we search the buffer to find a field
+boundary to restrict the search for matching pairs to. If there is no
+field boundary within this many characters, then we will not attempt to
+restrict the matching, otherwise we only match within the field.
+
+This is buffer local because different modes may want to tweak this.")
+
(defvar-local electric-pair-skip-whitespace-function
#'electric-pair--skip-whitespace
"Function to use to move point forward over whitespace.
@@ -596,6 +606,26 @@ electric-pair-default-inhibit
(electric-pair-inhibit-if-helps-balance char)
(electric-pair-conservative-inhibit char)))
+(defmacro electric-pair--restricted (&rest body)
+ "Restrict the action of BODY to the appropriate region.
+
+This is either the selected region, or, if the region isn't selected,
+the current field if one is found within `electric-pair-max-field-size',
+or else the entire buffer."
+ (declare (indent 0) (debug t))
+ (let ((subtracted-point-sym (gensym 'subtracted-point))
+ (beginning-sym (gensym 'beginning-sym))
+ (added-point-sym (gensym 'added-point))
+ (end-sym (gensym 'end)))
+ `(with-restriction
+ (let* ((,subtracted-point-sym (- (point) electric-pair-max-field-size))
+ (,beginning-sym (field-beginning nil nil (max (point-min) ,subtracted-point-sym))))
+ (if (or (use-region-p) (= ,beginning-sym ,subtracted-point-sym)) (point-min) ,beginning-sym))
+ (let* ((,added-point-sym (+ (point) electric-pair-max-field-size))
+ (,end-sym (field-end nil nil (min (point-max) ,added-point-sym))))
+ (if (or (use-region-p) (= ,end-sym ,added-point-sym)) (point-max) ,end-sym))
+ ,@body)))
+
(defun electric-pair-post-self-insert-function ()
"Do main work for `electric-pair-mode'.
This function is added to `post-self-insert-hook' when
@@ -620,88 +650,94 @@ electric-pair-post-self-insert-function
(both as defined by the major mode's syntax table). This is
done by looking up the variables
`electric-pair-inhibit-predicate', `electric-pair-skip-self'
- and `electric-pair-skip-whitespace' (which see)."
- (let* ((pos (and electric-pair-mode (electric--after-char-pos)))
- (num (when pos (prefix-numeric-value current-prefix-arg)))
- (beg (when num (- pos num)))
- (skip-whitespace-info))
- (pcase (electric-pair-syntax-info last-command-event)
- (`(,syntax ,pair ,unconditional ,space)
- (cond
- ((null pos) nil)
- ((zerop num) nil)
- ;; Wrap a pair around the active region.
- ;;
- ((and (memq syntax '(?\( ?\) ?\" ?\$)) (use-region-p))
- ;; FIXME: To do this right, we'd need a post-self-insert-function
- ;; so we could add-function around it and insert the closer after
- ;; all the rest of the hook has run.
- (if (or (eq syntax ?\")
- (and (eq syntax ?\))
- (>= (point) (mark)))
- (and (not (eq syntax ?\)))
- (>= (mark) (point))))
- (save-excursion
- (goto-char (mark))
- (electric-pair--insert pair num))
- (delete-region beg pos)
- (electric-pair--insert pair num)
- (goto-char (mark))
- (electric-pair--insert last-command-event num)))
- ;; Backslash-escaped: no pairing, no skipping.
- ((save-excursion
- (goto-char beg)
- (not (evenp (skip-syntax-backward "\\"))))
- (let ((current-prefix-arg (1- num)))
- (electric-pair-post-self-insert-function)))
- ;; Skip self.
- ((and (memq syntax '(?\) ?\" ?\$))
- (and (or unconditional
- (if (functionp electric-pair-skip-self)
- (electric-pair--save-literal-point-excursion
- (goto-char pos)
- (funcall electric-pair-skip-self
- last-command-event))
- electric-pair-skip-self))
- (save-excursion
- (when (and
- (not (and unconditional (eq syntax ?\")))
- (setq skip-whitespace-info
- (if (and
- (not
- (eq electric-pair-skip-whitespace
- 'chomp))
- (functionp electric-pair-skip-whitespace))
- (funcall electric-pair-skip-whitespace)
- electric-pair-skip-whitespace)))
- (funcall electric-pair-skip-whitespace-function))
- (eq (char-after) last-command-event))))
- ;; This is too late: rather than insert&delete we'd want to only
- ;; skip (or insert in overwrite mode). The difference is in what
- ;; goes in the undo-log and in the intermediate state which might
- ;; be visible to other post-self-insert-hook. We'll just have to
- ;; live with it for now.
- (when skip-whitespace-info
- (funcall electric-pair-skip-whitespace-function))
- (delete-region beg (if (eq skip-whitespace-info 'chomp)
- (point)
- pos))
- (forward-char num))
- ;; Insert matching pair.
- ;; String pairs
- ((and (eq syntax 'str) (not overwrite-mode))
- (if space (insert " "))
- (save-excursion
- (insert pair)))
- ;; Char pairs
- ((and (memq syntax '(?\( ?\" ?\$))
- (not overwrite-mode)
- (or unconditional
- (not (electric-pair--save-literal-point-excursion
- (goto-char pos)
- (funcall electric-pair-inhibit-predicate
- last-command-event)))))
- (save-excursion (electric-pair--insert pair num))))))))
+ and `electric-pair-skip-whitespace' (which see).
+
+If possible, this function will attempt to match delimiters for the
+current field only. However, we limit how much we scan for the
+beginning and end of the field to limit possible computation, so in
+large fields this falls back to using the entire buffer for matching."
+ (electric-pair--restricted
+ (let* ((pos (and electric-pair-mode (electric--after-char-pos)))
+ (num (when pos (prefix-numeric-value current-prefix-arg)))
+ (beg (when num (- pos num)))
+ (skip-whitespace-info))
+ (pcase (electric-pair-syntax-info last-command-event)
+ (`(,syntax ,pair ,unconditional ,space)
+ (cond
+ ((null pos) nil)
+ ((zerop num) nil)
+ ;; Wrap a pair around the active region.
+ ;;
+ ((and (memq syntax '(?\( ?\) ?\" ?\$)) (use-region-p))
+ ;; FIXME: To do this right, we'd need a post-self-insert-function
+ ;; so we could add-function around it and insert the closer after
+ ;; all the rest of the hook has run.
+ (if (or (eq syntax ?\")
+ (and (eq syntax ?\))
+ (>= (point) (mark)))
+ (and (not (eq syntax ?\)))
+ (>= (mark) (point))))
+ (save-excursion
+ (goto-char (mark))
+ (electric-pair--insert pair num))
+ (delete-region beg pos)
+ (electric-pair--insert pair num)
+ (goto-char (mark))
+ (electric-pair--insert last-command-event num)))
+ ;; Backslash-escaped: no pairing, no skipping.
+ ((save-excursion
+ (goto-char beg)
+ (not (evenp (skip-syntax-backward "\\"))))
+ (let ((current-prefix-arg (1- num)))
+ (electric-pair-post-self-insert-function)))
+ ;; Skip self.
+ ((and (memq syntax '(?\) ?\" ?\$))
+ (and (or unconditional
+ (if (functionp electric-pair-skip-self)
+ (electric-pair--save-literal-point-excursion
+ (goto-char pos)
+ (funcall electric-pair-skip-self
+ last-command-event))
+ electric-pair-skip-self))
+ (save-excursion
+ (when (and
+ (not (and unconditional (eq syntax ?\")))
+ (setq skip-whitespace-info
+ (if (and
+ (not
+ (eq electric-pair-skip-whitespace
+ 'chomp))
+ (functionp electric-pair-skip-whitespace))
+ (funcall electric-pair-skip-whitespace)
+ electric-pair-skip-whitespace)))
+ (funcall electric-pair-skip-whitespace-function))
+ (eq (char-after) last-command-event))))
+ ;; This is too late: rather than insert&delete we'd want to only
+ ;; skip (or insert in overwrite mode). The difference is in what
+ ;; goes in the undo-log and in the intermediate state which might
+ ;; be visible to other post-self-insert-hook. We'll just have to
+ ;; live with it for now.
+ (when skip-whitespace-info
+ (funcall electric-pair-skip-whitespace-function))
+ (delete-region beg (if (eq skip-whitespace-info 'chomp)
+ (point)
+ pos))
+ (forward-char num))
+ ;; Insert matching pair.
+ ;; String pairs
+ ((and (eq syntax 'str) (not overwrite-mode))
+ (if space (insert " "))
+ (save-excursion
+ (insert pair)))
+ ;; Char pairs
+ ((and (memq syntax '(?\( ?\" ?\$))
+ (not overwrite-mode)
+ (or unconditional
+ (not (electric-pair--save-literal-point-excursion
+ (goto-char pos)
+ (funcall electric-pair-inhibit-predicate
+ last-command-event)))))
+ (save-excursion (electric-pair--insert pair num)))))))))
(defun electric-pair-open-newline-between-pairs-psif ()
"Honor `electric-pair-open-newline-between-pairs'.
diff --git a/test/lisp/electric-tests.el b/test/lisp/electric-tests.el
index 9209e1739fd..256fcd0e7dd 100644
--- a/test/lisp/electric-tests.el
+++ b/test/lisp/electric-tests.el
@@ -703,7 +703,30 @@ autowrapping-multi-2
(goto-char (point-max))
(skip-chars-backward "\"")
(mark-sexp -1)))
-
+
+(define-electric-pair-test preserve-balance
+ "\"\n\n" "---\""
+ :expected-string "\"\n\n\""
+ :expected-point 5
+ :modes '(fundamental-mode)
+ :test-in-comments nil
+ :test-in-strings nil
+ :bindings '((electric-pair-preserve-balance . t))
+ :fixture-fn (lambda ()
+ (electric-pair-mode 1)))
+
+(define-electric-pair-test preserve-balance-not-across-fields
+ "\"\n\n" "---\""
+ :expected-string "\"\n\n\"\""
+ :expected-point 5
+ :modes '(fundamental-mode)
+ :test-in-comments nil
+ :test-in-strings nil
+ :bindings '((electric-pair-preserve-balance . t))
+ :fixture-fn (lambda ()
+ (electric-pair-mode 1)
+ (add-text-properties (point-min) (+ (point-min) 2)
+ '(field t))))
;;; Electric quotes
(define-electric-pair-test electric-quote-string
@@ -904,7 +927,6 @@ electric-quote-markdown-in-code
nil :local))
:bindings '((comment-start . "<!--") (comment-use-syntax . t))
:test-in-comments nil :test-in-strings nil)
-
;;; tests for `electric-layout-mode'