bug#36006: electric-pair-mode fails to balance in certain cases

bug#36006: electric-pair-mode fails to balance in certain cases

[Dario Gjorgjevski]

Consider the following text in html-mode with electric-pair-mode turned
on and electric-pair-skip-self set to electric-pair-default-skip-self:

         <|>

(Take | to denote point.)  In this situation, the expected result of
attempting to insert > is to skip over the one immediately after point,
i.e., end up with <>|.  However, what we get instead is <>|>.  Only if
we *now* attempt to insert >, we end up with <>>|, which doesn't make
any sense.

The same behavior is observed with ordinary parentheses in html-mode,
too.

From my debugging, the culprit seems to be an unnecessary (or mistaken?)
use of electric-pair--with-uncached-syntax.  I am not sure *why* exactly
it happens, and would appreciate further insight.

The patch shown below fixes the issue.

diff --git a/lisp/elec-pair.el b/lisp/elec-pair.el
index 5fb9d751e2..6450d02c9e 100644
--- a/lisp/elec-pair.el
+++ b/lisp/elec-pair.el
@@ -325,11 +325,9 @@ electric-pair--balance-info
                         (cond ((< direction 0)
                                (condition-case nil
                                    (eq (char-after pos)
-                                       (electric-pair--with-uncached-syntax
-                                           (table)
-                                         (matching-paren
-                                          (char-before
-                                           (scan-sexps (point) 1)))))
+                                       (matching-paren
+                                        (char-before
+                                         (scan-sexps (point) 1))))
                                  (scan-error nil)))
                               (t
                                ;; In this case, no need to use

-- 
Dario Gjorgjevski :: <dario.gjorgjevski@gmail.com>
                  :: 744A 4F0B 4F1C 9371
                  :: +48 1525 8666837

bug#36006: electric-pair-mode fails to balance in certain cases

[npostavs]

Dario Gjorgjevski <dario.gjorgjevski@gmail.com> writes:

> Consider the following text in html-mode with electric-pair-mode turned
> on and electric-pair-skip-self set to electric-pair-default-skip-self:
>
>          <|>
>
> (Take | to denote point.)  In this situation, the expected result of
> attempting to insert > is to skip over the one immediately after point,
> i.e., end up with <>|.  However, what we get instead is <>|>.  Only if
> we *now* attempt to insert >, we end up with <>>|, which doesn't make
> any sense.

I'm having trouble reproducing this; what's the exact sequence of input
starting from 'emacs -Q'?  And what version of Emacs is this?

bug#36006: electric-pair-mode fails to balance in certain cases

[Dario Gjorgjevski]

> I'm having trouble reproducing this; what's the exact sequence of input
> starting from 'emacs -Q'?

Run 'emacs -Q'; it opens up with the *scratch* buffer.  Now:

1. M-x electric-pair-mode <RET>
2. M-x html-mode <RET>
3. <div>

(The order of 1. and 2. is irrelevant.)  What I expect to see in the
buffer is:

<div>|

What I see instead is:

<div>|>

If I type another '>', I end up with:

<div>>|

> And what version of Emacs is this?

In GNU Emacs 27.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.8)
 of 2019-05-30 built on dario-XPS-13-9370
Repository revision: cc71a82fc705a73fa3ef6cda3ec6bee1cb654d7e
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12004000
System Description: Ubuntu 19.04

-- 
Dario Gjorgjevski :: <dario.gjorgjevski@gmail.com>
                  :: 744A 4F0B 4F1C 9371
                  :: +48 1525 8666837

bug#36006: electric-pair-mode fails to balance in certain cases

[Noam Postavsky]

found 36006 27.0.50
tags 36006 + confirmed
quit

Dario Gjorgjevski <dario.gjorgjevski@gmail.com> writes:

>> I'm having trouble reproducing this; what's the exact sequence of input
>> starting from 'emacs -Q'?
>
> Run 'emacs -Q'; it opens up with the *scratch* buffer.  Now:
>
> 1. M-x electric-pair-mode <RET>
> 2. M-x html-mode <RET>
> 3. <div>

Ah, got it now, I think I was inserting before starting html-mode before.

> From my debugging, the culprit seems to be an unnecessary (or mistaken?)
> use of electric-pair--with-uncached-syntax.  I am not sure *why* exactly
> it happens, and would appreciate further insight.

It doesn't happen in emacs-26, which has the same
electric-pair--with-uncached-syntax call, so it's not just that alone.
I suspect this could be related to the recent changes to mark ">"
outside of tags with punctuation syntax (technically, <foo>a > b</foo>
is valid, only "<" needs to be escaped as &lt;).

This is kind of a pain to debug, when I stepped with edebug, it worked,
I think because stepping gives time for syntax-propertize to run.

bug#36006: electric-pair-mode fails to balance in certain cases

[Dario Gjorgjevski]

[-- Attachment #1: Type: text/plain, Size: 721 bytes --]

(It’s been so long since I last looked into the bug...)

> It doesn't happen in emacs-26, which has the same
> electric-pair--with-uncached-syntax call, so it's not just that alone.
> I suspect this could be related to the recent changes to mark ">"
> outside of tags with punctuation syntax (technically, <foo>a > b</foo>
> is valid, only "<" needs to be escaped as &lt;).

You’re right.  The problem is that electric-pair--with-uncached-syntax
let-binds syntax-propertize-function to ignore, causing the syntax of
">" to not be updated.

Applying the attached patch fixes the issue.  However, in what cases
would we want syntax-propertize-function to be let-bound to ignore?

Best regards,
Dario


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Do not let-bind syntax-propertize-function to ignore in electric-pair--with-uncached-syntax --]
[-- Type: text/x-diff, Size: 1008 bytes --]

From 6de30e0cdc97475b3aac2a35bec1fcf1ac0af84f Mon Sep 17 00:00:00 2001
From: Dario Gjorgjevski <dario.gjorgjevski+git@gmail.com>
Date: Fri, 22 Nov 2019 13:19:39 +0100
Subject: [PATCH] Leave syntax-propertize-function unchanged

* lisp/elec-pair.el (electric-pair--with-uncached-syntax): Do not
let-bind syntax-propertize-function.
---
 lisp/elec-pair.el | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lisp/elec-pair.el b/lisp/elec-pair.el
index f3cbee7048..6e528b2dfd 100644
--- a/lisp/elec-pair.el
+++ b/lisp/elec-pair.el
@@ -246,8 +246,7 @@ electric-pair--with-uncached-syntax
 cache is flushed from position START, defaulting to point."
   (declare (debug ((form &optional form) body)) (indent 1))
   (let ((start-var (make-symbol "start")))
-    `(let ((syntax-propertize-function #'ignore)
-           (,start-var ,(or start '(point))))
+    `(let ((,start-var ,(or start '(point))))
        (unwind-protect
            (with-syntax-table ,table
              ,@body)
-- 
2.17.1

bug#36006: electric-pair-mode fails to balance in certain cases

[Gregory Heytings via Bug reports for GNU Emacs, the Swiss army knife of text editors]

I don't know if the following information is useful, but on the "<div>" 
example given by the OP, the difference between Emacs 26 and 27 is that, 
in `electric-pair--balance-info', in the expression

(eq (char-after pos) (electric-pair--with-uncached-syntax (table) (matching-paren (char-before (scan-sexps (point) 1)))))

the second argument fails with "Unbalanced parentheses" in Emacs 27, which 
means that the whole expression fails, whereas in Emacs 26 both arguments 
evaluate to 60 and the expression returns t.

Because of that `electric-pair--balance-info' returns ((nil . 60) nil . 
60) in Emacs 27, instead of ((t . 60) t) in Emacs 26.

Because of that `electric-pair-skip-if-helps-balance' and 
`electric-pair-default-skip-self' return nil in Emacs 27, instead of t in 
Emacs 26.

Because of that `electric-pair-post-self-insert-function', called by 
`post-self-insert-hook', does not skip the insertion of the right angle 
bracket character.

The following snippet evaluates to "60" on both Emacs 26 and 27, so I 
don't understand where the "Unbalanced parentheses" error could come from.

----
<div>
(progn
   (electric-pair-mode 1)
   (html-mode)
   (let ((table (syntax-table))) (electric-pair--with-uncached-syntax (table) (matching-paren (char-before (scan-sexps 0 1))))))
----

Gregory

bug#36006: electric-pair-mode fails to balance in certain cases

[Gregory Heytings via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Another note: this bug might in fact be specific to html-mode/sgml-mode. 
As far as I can see electric-pair-mode works fine with emacs-lisp-mode, 
c-mode, tex-mode, ...  And with

(defun sgml-syntax-propertize (start end &optional rules-function))

the bug disappears.

bug#36006: electric-pair-mode fails to balance in certain cases

[Gregory Heytings via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>
> Another note: this bug might in fact be specific to html-mode/sgml-mode. 
> As far as I can see electric-pair-mode works fine with emacs-lisp-mode, 
> c-mode, tex-mode, ...  And with
>
> (defun sgml-syntax-propertize (start end &optional rules-function))
>
> the bug disappears.
>

A more specific note: removing `(put-text-property ... (string-to-syntax 
"."))' in `sgml--syntax-propertize-ppss' also fixes the bug.  So this bug 
is indeed specific to html-mode/sgml-mode.

bug#36006: electric-pair-mode fails to balance in certain cases

[Lars Ingebrigtsen]

Gregory Heytings <ghe@sdf.org> writes:

> A more specific note: removing `(put-text-property
> ... (string-to-syntax "."))' in `sgml--syntax-propertize-ppss' also
> fixes the bug.  So this bug is indeed specific to html-mode/sgml-mode.

This problem is still present in Emacs 29.

I guess the issue here is that electric-pair-mode works by letting the
user insert the '>' before the final <div>, and then fixes things up (by
removing the extra '>' and moving point), but this defeats that:

(defun sgml--syntax-propertize-ppss (pos)
  "Return PPSS at POS, fixing the syntax of any lone `>' along the way."
  (cl-assert (>= pos (car sgml--syntax-propertize-ppss)))
  (let ((ppss (parse-partial-sexp (car sgml--syntax-propertize-ppss) pos -1
                                  nil (cdr sgml--syntax-propertize-ppss))))
    (while (eq -1 (car ppss))
      (put-text-property (1- (point)) (point)
                         'syntax-table (string-to-syntax "."))
      ;; Hack attack: rather than recompute the ppss from
      ;; (car sgml--syntax-propertize-ppss), we manually "fix it".
      (setcar ppss 0)
      (setq ppss (parse-partial-sexp (point) pos -1 nil ppss)))
    (setcdr sgml--syntax-propertize-ppss ppss)
    (setcar sgml--syntax-propertize-ppss pos)
    ppss))

Because it's marking the extra '>' as punctuation?  

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no