bug#49629: 27.2; electric-pair-mode doesn't work for angle brackets in HTML file

bug#49629: 27.2; electric-pair-mode doesn't work for angle brackets in HTML file

[Allen Li]

To reproduce:

1. emacs -Q
2. C-x C-f /tmp/tmp.html RET
3. M-x electric-pair-local-mode RET
4. < >

Expected:

Buffer contains <>

Actual:

Buffer contains <>>

By default, typing > should skip over the > inserted by
electric-pair-mode when the first < is type (but it doesn't).

I tried stepping through with Edebug, but the bug disappears when using
Edebug.  The difference seems to be in what is returned by
electric-pair--balance-info, but that function is big and unfamiliar to
me.

In GNU Emacs 27.2 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.27, cairo version 1.17.4)
 of 2021-03-26 built on juergen
Windowing system distributor 'The X.Org Foundation', version 11.0.12012000
System Description: Arch Linux

bug#49629: Acknowledgement (27.2; electric-pair-mode doesn't work for angle brackets in HTML file)

[Allen Li]

> To reproduce:
>
> 1. emacs -Q
> 2. C-x C-f /tmp/tmp.html RET
> 3. M-x electric-pair-local-mode RET
> 4. < >
>
> Expected:
>
> Buffer contains <>
>
> Actual:
>
> Buffer contains <>>
>
> By default, typing > should skip over the > inserted by
> electric-pair-mode when the first < is type (but it doesn't).
>
> I tried stepping through with Edebug, but the bug disappears when using
> Edebug.  The difference seems to be in what is returned by
> electric-pair--balance-info, but that function is big and unfamiliar to
> me.

What I've found so far is that using Edebug to step through
electric-pair-skip-if-helps-balance, if I manually step past
(delete-char -1) and then press g (edebug-go-mode) then the bug
disappears.  Pressing g before stepping past (delete-char -1) causes
the bug.

For reference, here is the function:

(defun electric-pair-skip-if-helps-balance (char)
  "Return non-nil if skipping CHAR would benefit parentheses' balance.
Works by first removing the character from the buffer, then doing
some list calculations, finally restoring the situation as if nothing
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))
                         (innermost (car pair-data))
                         (outermost (cdr pair-data)))
                    (and
                     (cond ((car outermost)
                            (car innermost))
                           ((car innermost)
                            (not (eq (cdr outermost) pair)))))))
                 ((eq syntax ?\")
                  (electric-pair--inside-string-p char))))
       (insert char)))))

bug#49629: 27.2; electric-pair-mode doesn't work for angle brackets in HTML file

[Lars Ingebrigtsen]

Allen Li <darkfeline@felesatra.moe> writes:

> What I've found so far is that using Edebug to step through
> electric-pair-skip-if-helps-balance, if I manually step past
> (delete-char -1) and then press g (edebug-go-mode) then the bug
> disappears.  Pressing g before stepping past (delete-char -1) causes
> the bug.

If I remember correctly, electric-pair-mode works by doing a lot of
magic in `post-self-insert-hook'?  In my experience, trying to edebug
through code when that hook is running often leads to peculiar results.

That is, you have to debug via other means when trying to see what's
actually happening here, I think.  (I.e., adding `message's all over the
place and other tedious things...)

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

bug#49629: 27.2; electric-pair-mode doesn't work for angle brackets in HTML file

[Allen Li]

On Thu, Jul 22, 2021 at 11:34 PM Lars Ingebrigtsen <larsi@gnus.org> wrote:
>
> Allen Li <darkfeline@felesatra.moe> writes:
>
> > What I've found so far is that using Edebug to step through
> > electric-pair-skip-if-helps-balance, if I manually step past
> > (delete-char -1) and then press g (edebug-go-mode) then the bug
> > disappears.  Pressing g before stepping past (delete-char -1) causes
> > the bug.
>
> If I remember correctly, electric-pair-mode works by doing a lot of
> magic in `post-self-insert-hook'?  In my experience, trying to edebug
> through code when that hook is running often leads to peculiar results.
>
> That is, you have to debug via other means when trying to see what's
> actually happening here, I think.  (I.e., adding `message's all over the
> place and other tedious things...)

Thanks for the advice, kind of.  I tried some printf debugging and I
noticed that
the bug goes away when I add (message "%S"
(buffer-substring-no-properties (point-min) (point-max)))
to the top of electric-pair--balance-info.  Maybe I'm being dense, but I think
that expression should not have this kind of side effect.

bug#49629: 27.2; electric-pair-mode doesn't work for angle brackets in HTML file

[Lars Ingebrigtsen]

Allen Li <darkfeline@felesatra.moe> writes:

> I tried some printf debugging and I noticed that the bug goes away
> when I add (message "%S" (buffer-substring-no-properties (point-min)
> (point-max))) to the top of electric-pair--balance-info.  Maybe I'm
> being dense, but I think that expression should not have this kind of
> side effect.

It should not, but it sounds like whatever's going on might be redisplay
dependent?  In which case it might be better to say

(push (buffer-string) my-var)

at strategic places in the code instead of messaging, and then examining
my-var afterwards.

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

bug#49629: 27.2; electric-pair-mode doesn't work for angle brackets in HTML file

[Allen Li]

On Wed, Jul 28, 2021 at 3:42 PM Lars Ingebrigtsen <larsi@gnus.org> wrote:
>
> Allen Li <darkfeline@felesatra.moe> writes:
>
> > I tried some printf debugging and I noticed that the bug goes away
> > when I add (message "%S" (buffer-substring-no-properties (point-min)
> > (point-max))) to the top of electric-pair--balance-info.  Maybe I'm
> > being dense, but I think that expression should not have this kind of
> > side effect.
>
> It should not, but it sounds like whatever's going on might be redisplay
> dependent?  In which case it might be better to say
>
> (push (buffer-string) my-var)
>
> at strategic places in the code instead of messaging, and then examining
> my-var afterwards.

I can't reproduce my previous fix of adding `(message "%S"
(buffer-substring-no-properties (point-min) (point-max)))` to the top
of `electric-pair--balance-info`.  At the time, I had many more
`message`s scattered throughout the function, but I could reliably
make the bug disappear and reappear by adding/removing this particular
`message`.  Of course, I don't remember exactly what `message`s I had
at the time.

Thus, I resorted to actually trying to understand the code.  I've
tracked down the bug to unexpected behavior from the `scan-sexps` call
in this part of `electric-pair--balance-info`:

         (ended-prematurely-fn
          ;; called when `scan-sexps' crashed against a parenthesis
          ;; pointing opposite the direction of travel.  After
          ;; traversing that character, the idea is to travel one sexp
          ;; in the opposite direction looking for a matching
          ;; delimiter.
          #'(lambda ()
              (let* ((pos (point))
                     (matched
                      (save-excursion
                        (cond ((< direction 0)
                               (condition-case err
                                   (eq (char-after pos)
                                       (electric-pair--with-uncached-syntax
                                           (table)
                                         (debug) ;; Added by me
                                         (matching-paren
                                          (char-before
                                           (scan-sexps (point) 1)))))
                                 (scan-error nil)))

The state of my test buffer at this point is `^<p>` where ^ is point.
The buffer starts as `<p^>`, and like the comment states, we move
point past the `<` "pointing opposite the direction of travel" and
look for a match for the opening `<`.

However, the `(scan-sexps (point) 1)` here signals `Scan error:
"Unbalanced parentheses", 1, 4`.  Again, the buffer state here is
`^<p>`.  Since `scan-sexps` is C, I haven't dug into it yet, but it
seems like it should work.  I also checked `(matching-paren ?<)` and
`(matching-paren ?>)` here to see if the syntax table recognizes the
angle brackets and it appears to do so, so I don't know why
`scan-sexps` is complaining.

While I don't know how the `syntax-ppss` cache works, I tried added an
extra `syntax-ppss-flush-cache` call to the start of the
`electric-pair--with-uncached-syntax`, but that doesn't help.

My current hypothesis is some odd behavior in how setting the syntax
table works.

bug#49629: 27.2; electric-pair-mode doesn't work for angle brackets in HTML file

[Lars Ingebrigtsen]

Allen Li <darkfeline@felesatra.moe> writes:

> Thus, I resorted to actually trying to understand the code.

Darn, I hate it when that happens.

> I've tracked down the bug to unexpected behavior from the `scan-sexps`
> call in this part of `electric-pair--balance-info`:

I can reproduce exactly what you're seeing -- when edebugging, the
problem goes away, etc.

I put a 

              (redisplay t)

into the function, and that also made the problem go away, but that's as
far as I've gotten so far.  So the problem does indeed seem to be
something related to a cache/table somewhere not having been updated...

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

bug#49629: 27.2; electric-pair-mode doesn't work for angle brackets in HTML file

[Allen Li]

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

I have bisected this regression to 7fff418edf56244a1fcf54718523aa9b5cb3a854

I will cc Stefan on the miniscule chance he still remembers anything about
this and can save me time.
Otherwise, I will see if I can pinpoint the regression (or if I messed up
the bisect).

Author: Stefan Monnier <monnier@iro.umontreal.ca>
Date:   Fri Nov 29 11:51:48 2019 -0500
    * lisp/textmodes/mhtml-mode.el: Fix bug#38372

    The `sgml-syntax-propertize-rules` rely on the
`sgml--syntax-propertize-ppss`
    setup by `sgml-syntax-propertize` so it is not correct/safe to use
    them directly like html used to do.

    Change `sgml-syntax-propertize` so it can be used by mhtml,
    and then adjust mhtml-mode accordingly.

    * lisp/textmodes/mhtml-mode.el: Remove redundant `eval-and-compile`.
    Only require cl-lib at compile-time.
    (mhtml--syntax-propertize): New const, extracted from
mhtml-syntax-propertize.
    (mhtml-syntax-propertize): Use `sgml-syntax-propertize`.

    * lisp/textmodes/sgml-mode.el (sgml--syntax-propertize): New const,
    extracted from sgml-syntax-propertize.
    (sgml-syntax-propertize): Add optional `rules-function` arg.
 lisp/textmodes/mhtml-mode.el | 44
++++++++++++++++++++------------------------
 lisp/textmodes/sgml-mode.el  | 13 ++++++++-----
 2 files changed, 28 insertions(+), 29 deletions(-)

On Sun, Aug 1, 2021 at 10:41 AM Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Allen Li <darkfeline@felesatra.moe> writes:
>
> > Thus, I resorted to actually trying to understand the code.
>
> Darn, I hate it when that happens.
>
> > I've tracked down the bug to unexpected behavior from the `scan-sexps`
> > call in this part of `electric-pair--balance-info`:
>
> I can reproduce exactly what you're seeing -- when edebugging, the
> problem goes away, etc.
>
> I put a
>
>               (redisplay t)
>
> into the function, and that also made the problem go away, but that's as
> far as I've gotten so far.  So the problem does indeed seem to be
> something related to a cache/table somewhere not having been updated...
>
> --
> (domestic pets only, the antidote for overdose, milk.)
>    bloggy blog: http://lars.ingebrigtsen.no
>

[-- Attachment #2: Type: text/html, Size: 2873 bytes --]

bug#49629: 27.2; electric-pair-mode doesn't work for angle brackets in HTML file

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

> I have bisected this regression to 7fff418edf56244a1fcf54718523aa9b5cb3a854
>
> I will cc Stefan on the miniscule chance he still remembers anything about
> this and can save me time.
> Otherwise, I will see if I can pinpoint the regression (or if I messed up
> the bisect).

Hmm... I do vaguely remember the change.  I don't see the immediate
connection, but...

>> I can reproduce exactly what you're seeing -- when edebugging, the
>> problem goes away, etc.
>>
>> I put a
>>
>>               (redisplay t)

I suspect that a call to `syntax-propertize` at that same place (or
nearby) might do the trick as well?


        Stefan

bug#49629: 27.2; electric-pair-mode doesn't work for angle brackets in HTML file

[Allen Li]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> I have bisected this regression to 7fff418edf56244a1fcf54718523aa9b5cb3a854
>>
>> I will cc Stefan on the miniscule chance he still remembers anything about
>> this and can save me time.
>> Otherwise, I will see if I can pinpoint the regression (or if I messed up
>> the bisect).
>
> Hmm... I do vaguely remember the change.  I don't see the immediate
> connection, but...

Stefan, I think I found an unrelated regression in your commit and I'd
like to double-check with you (or anyone else reading).

    modified   lisp/textmodes/sgml-mode.el
    @@ -395,16 +395,19 @@ sgml--syntax-propertize-ppss
                             (car (sgml--syntax-propertize-ppss
                                   (match-beginning 0)))))
                    (string-to-syntax ".")))))
    -     )))
    +     )
    +    "Syntax-propertize rules for sgml text.
    +These have to be run via `sgml-syntax-propertize'"))

    -(defun sgml-syntax-propertize (start end)
    +(defconst sgml--syntax-propertize
    +  (syntax-propertize-rules sgml-syntax-propertize-rules))
    +
    +(defun sgml-syntax-propertize (start end &optional rules-function)
       "Syntactic keywords for `sgml-mode'."
       (setq sgml--syntax-propertize-ppss (cons start (syntax-ppss start)))
       (cl-assert (>= (cadr sgml--syntax-propertize-ppss) 0))
       (sgml-syntax-propertize-inside end)
    -  (funcall
    -   (syntax-propertize-rules sgml-syntax-propertize-rules)
    -   start end)
    +  (funcall (or rules-function sgml--syntax-propertize) (point) end)
       ;; Catch any '>' after the last quote.
       (sgml--syntax-propertize-ppss end))

In the final `funcall`, the `start` argument was changed to `(point)`.
Looking at the overall commit, this seems unintentional to me.

Unless this was intentional, I will upload a patch to fix it (this still
exists in master).  Unfortunately, this does not fix the bug I am
interested in, but may as well fix it while I'm here.

bug#49629: 27.2; electric-pair-mode doesn't work for angle brackets in HTML file

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

>     -(defun sgml-syntax-propertize (start end)
>     +(defconst sgml--syntax-propertize
>     +  (syntax-propertize-rules sgml-syntax-propertize-rules))
>     +
>     +(defun sgml-syntax-propertize (start end &optional rules-function)
>        "Syntactic keywords for `sgml-mode'."
>        (setq sgml--syntax-propertize-ppss (cons start (syntax-ppss start)))
>        (cl-assert (>= (cadr sgml--syntax-propertize-ppss) 0))
>        (sgml-syntax-propertize-inside end)
>     -  (funcall
>     -   (syntax-propertize-rules sgml-syntax-propertize-rules)
>     -   start end)
>     +  (funcall (or rules-function sgml--syntax-propertize) (point) end)
>        ;; Catch any '>' after the last quote.
>        (sgml--syntax-propertize-ppss end))
>
> In the final `funcall`, the `start` argument was changed to `(point)`.
> Looking at the overall commit, this seems unintentional to me.

No, this was intentional: `start` is used by passing it to
`syntax-ppss` which will move point to `start`.  After that some of the
text may/will be handled by `sgml-syntax-propertize-inside` which
indicates it by moving point past the part that it handled (if any) and
the (or rules-function sgml--syntax-propertize) should only apply to the
remaining part.


        Stefan

bug#49629: 27.2; electric-pair-mode doesn't work for angle brackets in HTML file

[Allen Li]

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

On Thu, Dec 9, 2021 at 2:31 AM Allen Li <darkfeline@felesatra.moe> wrote:

> I have bisected this regression to 7fff418edf56244a1fcf54718523aa9b5cb3a854
>
> I will cc Stefan on the miniscule chance he still remembers anything about
> this and can save me time.
> Otherwise, I will see if I can pinpoint the regression (or if I messed up
> the bisect).
>

Posting an update (or non-update) on this.  This regression did seem to be
introduced by 7fff418edf56244a1fcf54718523aa9b5cb3a854, however I'm pretty
sure there's nothing wrong with the commit itself, it's just that jiggling
the code around made this regression reliably occur.


>
> Author: Stefan Monnier <monnier@iro.umontreal.ca>
> Date:   Fri Nov 29 11:51:48 2019 -0500
>     * lisp/textmodes/mhtml-mode.el: Fix bug#38372
>
>     The `sgml-syntax-propertize-rules` rely on the
> `sgml--syntax-propertize-ppss`
>     setup by `sgml-syntax-propertize` so it is not correct/safe to use
>     them directly like html used to do.
>
>     Change `sgml-syntax-propertize` so it can be used by mhtml,
>     and then adjust mhtml-mode accordingly.
>
>     * lisp/textmodes/mhtml-mode.el: Remove redundant `eval-and-compile`.
>     Only require cl-lib at compile-time.
>     (mhtml--syntax-propertize): New const, extracted from
> mhtml-syntax-propertize.
>     (mhtml-syntax-propertize): Use `sgml-syntax-propertize`.
>
>     * lisp/textmodes/sgml-mode.el (sgml--syntax-propertize): New const,
>     extracted from sgml-syntax-propertize.
>     (sgml-syntax-propertize): Add optional `rules-function` arg.
>  lisp/textmodes/mhtml-mode.el | 44
> ++++++++++++++++++++------------------------
>  lisp/textmodes/sgml-mode.el  | 13 ++++++++-----
>  2 files changed, 28 insertions(+), 29 deletions(-)
>
> On Sun, Aug 1, 2021 at 10:41 AM Lars Ingebrigtsen <larsi@gnus.org> wrote:
>
>> Allen Li <darkfeline@felesatra.moe> writes:
>>
>> > Thus, I resorted to actually trying to understand the code.
>>
>> Darn, I hate it when that happens.
>>
>> > I've tracked down the bug to unexpected behavior from the `scan-sexps`
>> > call in this part of `electric-pair--balance-info`:
>>
>> I can reproduce exactly what you're seeing -- when edebugging, the
>> problem goes away, etc.
>>
>> I put a
>>
>>               (redisplay t)
>>
>> into the function, and that also made the problem go away, but that's as
>> far as I've gotten so far.  So the problem does indeed seem to be
>> something related to a cache/table somewhere not having been updated...
>>
>> --
>> (domestic pets only, the antidote for overdose, milk.)
>>    bloggy blog: http://lars.ingebrigtsen.no
>>
>

[-- Attachment #2: Type: text/html, Size: 3681 bytes --]

bug#49629: 27.2; electric-pair-mode doesn't work for angle brackets in HTML file

[Allen Li]

[-- Attachment #1.1: Type: text/plain, Size: 3592 bytes --]

I *think* I've fixed this, but it's complicated.  Also I could be
completely wrong.  For what it's worth, I can reproduce the bug without the
patch and cannot with the patch, which see attached.

`electric-pair--with-uncached-syntax` hides `syntax-propertize-function`,
and `mhtml-mode` uses `syntax-propertize-function`. AFAIU, let-binding
`syntax-propertize-function` may or may not clear the cached syntax applied
by said function, leading to the current heisenbug.

If this sounds sensible, then a slightly different patch is needed, because
`electric-pair--with-uncached-syntax` is used in some contexts where hiding
`syntax-propertize-function` is the correct behavior.

See second attached patch for an attempt at this approach.

+Noam Postavsky <npostavs@gmail.com> since they added
`electric-pair--with-uncached-syntax`

On Sun, Jun 26, 2022 at 1:41 AM Allen Li <darkfeline@felesatra.moe> wrote:

> On Thu, Dec 9, 2021 at 2:31 AM Allen Li <darkfeline@felesatra.moe> wrote:
>
>> I have bisected this regression
>> to 7fff418edf56244a1fcf54718523aa9b5cb3a854
>>
>> I will cc Stefan on the miniscule chance he still remembers anything
>> about this and can save me time.
>> Otherwise, I will see if I can pinpoint the regression (or if I messed up
>> the bisect).
>>
>
> Posting an update (or non-update) on this.  This regression did seem to be
> introduced by 7fff418edf56244a1fcf54718523aa9b5cb3a854, however I'm pretty
> sure there's nothing wrong with the commit itself, it's just that jiggling
> the code around made this regression reliably occur.
>
>
>>
>> Author: Stefan Monnier <monnier@iro.umontreal.ca>
>> Date:   Fri Nov 29 11:51:48 2019 -0500
>>     * lisp/textmodes/mhtml-mode.el: Fix bug#38372
>>
>>     The `sgml-syntax-propertize-rules` rely on the
>> `sgml--syntax-propertize-ppss`
>>     setup by `sgml-syntax-propertize` so it is not correct/safe to use
>>     them directly like html used to do.
>>
>>     Change `sgml-syntax-propertize` so it can be used by mhtml,
>>     and then adjust mhtml-mode accordingly.
>>
>>     * lisp/textmodes/mhtml-mode.el: Remove redundant `eval-and-compile`.
>>     Only require cl-lib at compile-time.
>>     (mhtml--syntax-propertize): New const, extracted from
>> mhtml-syntax-propertize.
>>     (mhtml-syntax-propertize): Use `sgml-syntax-propertize`.
>>
>>     * lisp/textmodes/sgml-mode.el (sgml--syntax-propertize): New const,
>>     extracted from sgml-syntax-propertize.
>>     (sgml-syntax-propertize): Add optional `rules-function` arg.
>>  lisp/textmodes/mhtml-mode.el | 44
>> ++++++++++++++++++++------------------------
>>  lisp/textmodes/sgml-mode.el  | 13 ++++++++-----
>>  2 files changed, 28 insertions(+), 29 deletions(-)
>>
>> On Sun, Aug 1, 2021 at 10:41 AM Lars Ingebrigtsen <larsi@gnus.org> wrote:
>>
>>> Allen Li <darkfeline@felesatra.moe> writes:
>>>
>>> > Thus, I resorted to actually trying to understand the code.
>>>
>>> Darn, I hate it when that happens.
>>>
>>> > I've tracked down the bug to unexpected behavior from the `scan-sexps`
>>> > call in this part of `electric-pair--balance-info`:
>>>
>>> I can reproduce exactly what you're seeing -- when edebugging, the
>>> problem goes away, etc.
>>>
>>> I put a
>>>
>>>               (redisplay t)
>>>
>>> into the function, and that also made the problem go away, but that's as
>>> far as I've gotten so far.  So the problem does indeed seem to be
>>> something related to a cache/table somewhere not having been updated...
>>>
>>> --
>>> (domestic pets only, the antidote for overdose, milk.)
>>>    bloggy blog: http://lars.ingebrigtsen.no
>>>
>>

[-- Attachment #1.2: Type: text/html, Size: 5031 bytes --]

[-- Attachment #2: 0001-Fix-regression.patch --]
[-- Type: text/x-patch, Size: 861 bytes --]

From 5dd42c108b49097624c1637a562e83953752b0cf Mon Sep 17 00:00:00 2001
From: Allen Li <darkfeline@felesatra.moe>
Date: Sun, 26 Jun 2022 01:47:19 -0700
Subject: [PATCH] Fix regression

---
 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 964d21f11c..b94eab066f 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.36.1


[-- Attachment #3: 0001-elec-pair-Fix-bug-incorrectly-hiding-syntax-properti.patch --]
[-- Type: text/x-patch, Size: 3595 bytes --]

From 95c20919bb69d46e6f44a9e330c78e6f1d4a6a8b Mon Sep 17 00:00:00 2001
From: Allen Li <darkfeline@felesatra.moe>
Date: Sun, 26 Jun 2022 02:32:47 -0700
Subject: [PATCH] elec-pair: Fix bug incorrectly hiding
 syntax-propertize-function

Notably, this causes electric-pair-mode to often misbehave in HTML
files when pairing angle brackets.
---
 lisp/elec-pair.el | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/lisp/elec-pair.el b/lisp/elec-pair.el
index 964d21f11c..915a6d6333 100644
--- a/lisp/elec-pair.el
+++ b/lisp/elec-pair.el
@@ -237,16 +237,21 @@ electric-pair--insert
         (electric-layout-allow-duplicate-newlines t))
     (self-insert-command 1)))
 
-(cl-defmacro electric-pair--with-uncached-syntax ((table &optional start) &rest body)
+(cl-defmacro electric-pair--with-uncached-syntax ((table &optional start func) &rest body)
   "Like `with-syntax-table', but flush the `syntax-ppss' cache afterwards.
 Use this instead of (with-syntax-table TABLE BODY) when BODY
 contains code which may update the `syntax-ppss' cache.  This
 includes calling `parse-partial-sexp' and any sexp-based movement
 functions when `parse-sexp-lookup-properties' is non-nil.  The
-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)
+cache is flushed from position START, defaulting to point.
+FUNC is an optional function to set to `syntax-propertize-function'."
+  (declare (debug ((form &optional form form) body)) (indent 1))
+  (let ((start-var (make-symbol "start"))
+        (func-var (make-symbol "func")))
+    `(let ((syntax-propertize-function (let ((,func-var ,func))
+                                         (if ,func-var
+                                             ,func-var
+                                           syntax-propertize-function)))
            (,start-var ,(or start '(point))))
        (unwind-protect
            (with-syntax-table ,table
@@ -272,7 +277,7 @@ electric-pair--syntax-ppss
                               (point)))))
     (if s-or-c-start
         (electric-pair--with-uncached-syntax (electric-pair-text-syntax-table
-                                              s-or-c-start)
+                                              s-or-c-start #'ignore)
           (parse-partial-sexp s-or-c-start pos))
       ;; HACK! cc-mode apparently has some `syntax-ppss' bugs
       (if (memq major-mode '(c-mode c++ mode))
@@ -326,7 +331,7 @@ electric-pair--balance-info
                                (condition-case nil
                                    (eq (char-after pos)
                                        (electric-pair--with-uncached-syntax
-                                           (table)
+                                           (table (point) (if string-or-comment #'ignore))
                                          (matching-paren
                                           (char-before
                                            (scan-sexps (point) 1)))))
@@ -356,7 +361,8 @@ electric-pair--balance-info
     (save-excursion
       (while (not outermost)
         (condition-case err
-            (electric-pair--with-uncached-syntax (table)
+            (electric-pair--with-uncached-syntax
+                (table (point) (if string-or-comment #'ignore))
               (scan-sexps (point) (if (> direction 0)
                                       (point-max)
                                     (- (point-max))))
-- 
2.36.1

bug#49629: 27.2; electric-pair-mode doesn't work for angle brackets in HTML file

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

> I *think* I've fixed this, but it's complicated.  Also I could be
> completely wrong.  For what it's worth, I can reproduce the bug without the
> patch and cannot with the patch, which see attached.

AFAICT you've indeed found the origin of the problem.

> If this sounds sensible, then a slightly different patch is needed, because
> `electric-pair--with-uncached-syntax` is used in some contexts where hiding
> `syntax-propertize-function` is the correct behavior.

I think the code deserves a comment when/where it overrides
`syntax-propertize-function` to explain why it's needed.
AFAICT it was introduced in commit
89cfdbf729bc731331358e0efc69547547aa3ca2 but that commit doesn't explain
why it bound it to nil (which I later changed to `ignore`).

Furthermore, the cache could be filled with entries before `start` while
the syntax-table  (and/or `syntax-propertize-function`) is temporarily
changed, so the flush doesn't seem sufficient.  [ It's unlikely, because
usually the cache will have been pre-filled via font-lock and friends,
but it can still occur in corner cases.  ]

IIUC we use `with-syntax-table` there specifically when we want to
provide text-mode style paren matching within comments and strings.
Maybe a good way to avoid problem with syntax-ppss/properties is to
narrow the buffer to the comment/string at the same time as we
`with-syntax-table` and let-bind `syntax-propertize-function`.


        Stefan

bug#49629: 27.2; electric-pair-mode doesn't work for angle brackets in HTML file

[Allen Li]

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

On Sun, Jun 26, 2022 at 5:17 AM Stefan Monnier <monnier@iro.umontreal.ca>
wrote:

> > I *think* I've fixed this, but it's complicated.  Also I could be
> > completely wrong.  For what it's worth, I can reproduce the bug without
> the
> > patch and cannot with the patch, which see attached.
>
> AFAICT you've indeed found the origin of the problem.
>
> > If this sounds sensible, then a slightly different patch is needed,
> because
> > `electric-pair--with-uncached-syntax` is used in some contexts where
> hiding
> > `syntax-propertize-function` is the correct behavior.
>
> I think the code deserves a comment when/where it overrides
> `syntax-propertize-function` to explain why it's needed.
> AFAICT it was introduced in commit
> 89cfdbf729bc731331358e0efc69547547aa3ca2 but that commit doesn't explain
> why it bound it to nil (which I later changed to `ignore`).
>
> Furthermore, the cache could be filled with entries before `start` while
> the syntax-table  (and/or `syntax-propertize-function`) is temporarily
> changed, so the flush doesn't seem sufficient.  [ It's unlikely, because
> usually the cache will have been pre-filled via font-lock and friends,
> but it can still occur in corner cases.  ]
>
> IIUC we use `with-syntax-table` there specifically when we want to
> provide text-mode style paren matching within comments and strings.
> Maybe a good way to avoid problem with syntax-ppss/properties is to
> narrow the buffer to the comment/string at the same time as we
> `with-syntax-table` and let-bind `syntax-propertize-function`.
>

Thanks.  Two observations:

FIrst, changing `syntax-propertize-function` from nil to `ignore` was wrong
IIUC.  If the function is set, then it is wholly responsible for applying
syntax table.  When set to nil the default behavior is used, but when set
to `ignore`, that should mean that no syntax is applied at all.  In
practice, I don't know what behavior that causes.

Second, since `electric-pair--with-uncached-syntax` appears to be used for
doing text-mode matching (as you've also observed), maybe we should
de-generalize it to do only that.  I think that allows us to make some
simplifying assumptions about the state of the world.

>
>
>         Stefan
>
>

[-- Attachment #2: Type: text/html, Size: 2851 bytes --]

bug#49629: 27.2; electric-pair-mode doesn't work for angle brackets in HTML file

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

> FIrst, changing `syntax-propertize-function` from nil to `ignore` was wrong
> IIUC.  If the function is set, then it is wholly responsible for applying
> syntax table.  When set to nil the default behavior is used, but when set
> to `ignore`, that should mean that no syntax is applied at all.  In
> practice, I don't know what behavior that causes.

The two behave identically (IOW the "default behavior" is the same as
that of `ignore`).

> Second, since `electric-pair--with-uncached-syntax` appears to be used for
> doing text-mode matching (as you've also observed), maybe we should
> de-generalize it to do only that.  I think that allows us to make some
> simplifying assumptions about the state of the world.

Sounds good.


        Stefan

bug#49629: 27.2; electric-pair-mode doesn't work for angle brackets in HTML file

[Allen Li]

[-- Attachment #1.1: Type: text/plain, Size: 1253 bytes --]

How does the attached patch look?

I did not clear the syntax patch for the entire buffer because I was
worried of the performance impact.
We assume that we won't be touching the part of the buffer before the
specified start.
IIUC there is no issue keeping the cached state from before start using the
original table,
as we only want to consider the text succeeding start as text.

On Mon, Jun 27, 2022 at 5:39 AM Stefan Monnier <monnier@iro.umontreal.ca>
wrote:

> > FIrst, changing `syntax-propertize-function` from nil to `ignore` was
> wrong
> > IIUC.  If the function is set, then it is wholly responsible for applying
> > syntax table.  When set to nil the default behavior is used, but when set
> > to `ignore`, that should mean that no syntax is applied at all.  In
> > practice, I don't know what behavior that causes.
>
> The two behave identically (IOW the "default behavior" is the same as
> that of `ignore`).
>
> > Second, since `electric-pair--with-uncached-syntax` appears to be used
> for
> > doing text-mode matching (as you've also observed), maybe we should
> > de-generalize it to do only that.  I think that allows us to make some
> > simplifying assumptions about the state of the world.
>
> Sounds good.
>
>
>         Stefan
>
>

[-- Attachment #1.2: Type: text/html, Size: 1717 bytes --]

[-- Attachment #2: 0001-elec-pair-Fix-bug-incorrectly-hiding-syntax-properti.patch --]
[-- Type: text/x-patch, Size: 8241 bytes --]

From fc3a85fc83fa3fd54a73002575d339e6bb7423b2 Mon Sep 17 00:00:00 2001
From: Allen Li <darkfeline@felesatra.moe>
Date: Sun, 26 Jun 2022 02:32:47 -0700
Subject: [PATCH] elec-pair: Fix bug incorrectly hiding
 syntax-propertize-function

The main bug that this is fixing is `syntax-propertize-function' being
hidden in `electric-pair--balance-info' when the original syntax table
is to be used, not `electric-pair-text-syntax-table'.

Notably, this causes `electric-pair-mode' to often misbehave in HTML
files when pairing angle brackets.

This commit also flushes the cache before installing
`electric-pair-text-syntax-table', to prevent cached syntax for the
original table from affecting things.
---
 lisp/elec-pair.el | 82 +++++++++++++++++++++++++----------------------
 1 file changed, 43 insertions(+), 39 deletions(-)

diff --git a/lisp/elec-pair.el b/lisp/elec-pair.el
index bbed955a39..dab37c5110 100644
--- a/lisp/elec-pair.el
+++ b/lisp/elec-pair.el
@@ -188,6 +188,25 @@ electric-pair-conservative-inhibit
    ;; I also find it often preferable not to pair next to a word.
    (eq (char-syntax (following-char)) ?w)))
 
+(cl-defmacro electric-pair--with-text-syntax ((&optional start) &rest body)
+  "Run BODY with `electric-pair-text-syntax-table' active.
+This ensures that all syntax related values are set properly and the
+`syntax-ppss' cache is cleared before and after.
+In particular, this must be used when BODY contains code which may
+update the `syntax-ppss' cache.  This includes calling
+`parse-partial-sexp' and any sexp-based movement functions when
+`parse-sexp-lookup-properties' is non-nil.  The cache is flushed from
+position START, defaulting to point."
+  (declare (debug ((&optional form) body)) (indent 1))
+  (let ((start-var (make-symbol "start")))
+    `(let ((syntax-propertize-function nil)
+           (,start-var ,(or start '(point))))
+       (syntax-ppss-flush-cache ,start-var)
+       (unwind-protect
+           (with-syntax-table electric-pair-text-syntax-table
+             ,@body)
+         (syntax-ppss-flush-cache ,start-var)))))
+
 (defun electric-pair-syntax-info (command-event)
   "Calculate a list (SYNTAX PAIR UNCONDITIONAL STRING-OR-COMMENT-START).
 
@@ -202,13 +221,12 @@ electric-pair-syntax-info
          (post-string-or-comment (nth 8 (syntax-ppss (point))))
          (string-or-comment (and post-string-or-comment
                                  pre-string-or-comment))
-         (table (if string-or-comment
-                    electric-pair-text-syntax-table
-                  (syntax-table)))
-         (table-syntax-and-pair (with-syntax-table table
-                                  (list (char-syntax command-event)
-                                        (or (matching-paren command-event)
-                                            command-event))))
+         (table-syntax-and-pair (cl-flet ((f () (list (char-syntax command-event)
+                                                      (or (matching-paren command-event)
+                                                          command-event))))
+                                  (if string-or-comment
+                                      (electric-pair--with-text-syntax () (f))
+                                    (f))))
          (fallback (if string-or-comment
                        (append electric-pair-text-pairs
                                electric-pair-pairs)
@@ -237,22 +255,6 @@ electric-pair--insert
         (electric-layout-allow-duplicate-newlines t))
     (self-insert-command 1)))
 
-(cl-defmacro electric-pair--with-uncached-syntax ((table &optional start) &rest body)
-  "Like `with-syntax-table', but flush the `syntax-ppss' cache afterwards.
-Use this instead of (with-syntax-table TABLE BODY) when BODY
-contains code which may update the `syntax-ppss' cache.  This
-includes calling `parse-partial-sexp' and any sexp-based movement
-functions when `parse-sexp-lookup-properties' is non-nil.  The
-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))))
-       (unwind-protect
-           (with-syntax-table ,table
-             ,@body)
-         (syntax-ppss-flush-cache ,start-var)))))
-
 (defun electric-pair--syntax-ppss (&optional pos where)
   "Like `syntax-ppss', but sometimes fallback to `parse-partial-sexp'.
 
@@ -271,8 +273,7 @@ electric-pair--syntax-ppss
                               (skip-syntax-forward " >!")
                               (point)))))
     (if s-or-c-start
-        (electric-pair--with-uncached-syntax (electric-pair-text-syntax-table
-                                              s-or-c-start)
+        (electric-pair--with-text-syntax (s-or-c-start)
           (parse-partial-sexp s-or-c-start pos))
       ;; HACK! cc-mode apparently has some `syntax-ppss' bugs
       (if (memq major-mode '(c-mode c++ mode))
@@ -301,9 +302,6 @@ electric-pair--balance-info
 If point is not enclosed by any lists, return ((t) . (t))."
   (let* (innermost
          outermost
-         (table (if string-or-comment
-                    electric-pair-text-syntax-table
-                  (syntax-table)))
          (at-top-level-or-equivalent-fn
           ;; called when `scan-sexps' ran perfectly, when it found
           ;; a parenthesis pointing in the direction of travel.
@@ -325,11 +323,12 @@ 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)))))
+                                       (cl-flet ((f () (matching-paren
+                                                        (char-before
+                                                         (scan-sexps (point) 1)))))
+                                         (if string-or-comment
+                                             (electric-pair--with-text-syntax () (f))
+                                           (f))))
                                  (scan-error nil)))
                               (t
                                ;; In this case, no need to use
@@ -343,7 +342,9 @@ electric-pair--balance-info
                                       (opener (char-after start)))
                                  (and start
                                       (eq (char-before pos)
-                                          (or (with-syntax-table table
+                                          (or (if string-or-comment
+                                                  (electric-pair--with-text-syntax ()
+                                                    (matching-paren opener))
                                                 (matching-paren opener))
                                               opener))))))))
                      (actual-pair (if (> direction 0)
@@ -356,11 +357,14 @@ electric-pair--balance-info
     (save-excursion
       (while (not outermost)
         (condition-case err
-            (electric-pair--with-uncached-syntax (table)
-              (scan-sexps (point) (if (> direction 0)
-                                      (point-max)
-                                    (- (point-max))))
-              (funcall at-top-level-or-equivalent-fn))
+            (cl-flet ((f ()
+                         (scan-sexps (point) (if (> direction 0)
+                                                 (point-max)
+                                               (- (point-max))))
+                         (funcall at-top-level-or-equivalent-fn)))
+              (if string-or-comment
+                  (electric-pair--with-text-syntax () (f))
+                (f)))
           (scan-error
            (cond ((or
                    ;; some error happened and it is not of the "ended
-- 
2.37.0

bug#49629: 27.2; electric-pair-mode doesn't work for angle brackets in HTML file

[Lars Ingebrigtsen]

Allen Li <darkfeline@felesatra.moe> writes:

> How does the attached patch look?

Makes sense to me, I think, so I've now pushed it to Emacs 29 (with some
whitespace changes).

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