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

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 49629 in the body.
You can then email your comments to 49629 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Allen Li <darkfeline <at> felesatra.moe>
To: bug-gnu-emacs <at> gnu.org
Subject: 27.2; electric-pair-mode doesn't work for angle brackets in HTML file
Date: Sun, 18 Jul 2021 16:52:37 -0700

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

Message #8 received at 49629 <at> debbugs.gnu.org (full text, mbox):

From: Allen Li <darkfeline <at> felesatra.moe>
To: 49629 <at> debbugs.gnu.org
Subject: bug#49629: Acknowledgement (27.2; electric-pair-mode doesn't work for
 angle brackets in HTML file)
Date: Thu, 22 Jul 2021 07:03:29 +0000

> 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)))))

Message #11 received at 49629 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Allen Li <darkfeline <at> felesatra.moe>
Cc: 49629 <at> debbugs.gnu.org
Subject: Re: bug#49629: 27.2; electric-pair-mode doesn't work for angle
 brackets in HTML file
Date: Fri, 23 Jul 2021 01:34:52 +0200

Allen Li <darkfeline <at> 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

Message #14 received at 49629 <at> debbugs.gnu.org (full text, mbox):

From: Allen Li <darkfeline <at> felesatra.moe>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 49629 <at> debbugs.gnu.org
Subject: Re: bug#49629: 27.2; electric-pair-mode doesn't work for angle
 brackets in HTML file
Date: Sun, 25 Jul 2021 10:08:09 +0000

On Thu, Jul 22, 2021 at 11:34 PM Lars Ingebrigtsen <larsi <at> gnus.org> wrote:
>
> Allen Li <darkfeline <at> 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.

Message #17 received at 49629 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Allen Li <darkfeline <at> felesatra.moe>
Cc: 49629 <at> debbugs.gnu.org
Subject: Re: bug#49629: 27.2; electric-pair-mode doesn't work for angle
 brackets in HTML file
Date: Wed, 28 Jul 2021 17:42:32 +0200

Allen Li <darkfeline <at> 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

Message #20 received at 49629 <at> debbugs.gnu.org (full text, mbox):

From: Allen Li <darkfeline <at> felesatra.moe>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 49629 <at> debbugs.gnu.org
Subject: Re: bug#49629: 27.2; electric-pair-mode doesn't work for angle
 brackets in HTML file
Date: Sun, 1 Aug 2021 05:06:35 +0000

On Wed, Jul 28, 2021 at 3:42 PM Lars Ingebrigtsen <larsi <at> gnus.org> wrote:
>
> Allen Li <darkfeline <at> 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.

Message #23 received at 49629 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Allen Li <darkfeline <at> felesatra.moe>
Cc: 49629 <at> debbugs.gnu.org
Subject: Re: bug#49629: 27.2; electric-pair-mode doesn't work for angle
 brackets in HTML file
Date: Sun, 01 Aug 2021 12:41:45 +0200

Allen Li <darkfeline <at> 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

Message #26 received at 49629 <at> debbugs.gnu.org (full text, mbox):

From: Allen Li <darkfeline <at> felesatra.moe>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 49629 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#49629: 27.2; electric-pair-mode doesn't work for angle
 brackets in HTML file
Date: Thu, 9 Dec 2021 10:31:08 +0000

[Message part 1 (text/plain, inline)]

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 <at> 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 <at> gnus.org> wrote:

> Allen Li <darkfeline <at> 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
>

[Message part 2 (text/html, inline)]

Message #29 received at 49629 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Allen Li <darkfeline <at> felesatra.moe>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 49629 <at> debbugs.gnu.org
Subject: Re: bug#49629: 27.2; electric-pair-mode doesn't work for angle
 brackets in HTML file
Date: Thu, 09 Dec 2021 08:30:24 -0500

> 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

Message #32 received at 49629 <at> debbugs.gnu.org (full text, mbox):

From: Allen Li <darkfeline <at> felesatra.moe>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 49629 <at> debbugs.gnu.org
Subject: Re: bug#49629: 27.2; electric-pair-mode doesn't work for angle
 brackets in HTML file
Date: Fri, 24 Dec 2021 02:07:54 -0800

Stefan Monnier <monnier <at> 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.

Message #35 received at 49629 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Allen Li <darkfeline <at> felesatra.moe>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 49629 <at> debbugs.gnu.org
Subject: Re: bug#49629: 27.2; electric-pair-mode doesn't work for angle
 brackets in HTML file
Date: Fri, 24 Dec 2021 09:23:06 -0500

>     -(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

Message #38 received at 49629 <at> debbugs.gnu.org (full text, mbox):

From: Allen Li <darkfeline <at> felesatra.moe>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 49629 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#49629: 27.2; electric-pair-mode doesn't work for angle
 brackets in HTML file
Date: Sun, 26 Jun 2022 01:41:49 -0700

[Message part 1 (text/plain, inline)]

On Thu, Dec 9, 2021 at 2:31 AM Allen Li <darkfeline <at> 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 <at> 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 <at> gnus.org> wrote:
>
>> Allen Li <darkfeline <at> 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
>>
>

[Message part 2 (text/html, inline)]

Message #41 received at 49629 <at> debbugs.gnu.org (full text, mbox):

From: Allen Li <darkfeline <at> felesatra.moe>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Noam Postavsky <npostavs <at> gmail.com>, 49629 <at> debbugs.gnu.org,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#49629: 27.2; electric-pair-mode doesn't work for angle
 brackets in HTML file
Date: Sun, 26 Jun 2022 02:38:28 -0700

[Message part 1 (text/plain, inline)]

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 <at> gmail.com> since they added
`electric-pair--with-uncached-syntax`

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

> On Thu, Dec 9, 2021 at 2:31 AM Allen Li <darkfeline <at> 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 <at> 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 <at> gnus.org> wrote:
>>
>>> Allen Li <darkfeline <at> 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
>>>
>>

[Message part 2 (text/html, inline)]

[0001-Fix-regression.patch (text/x-patch, attachment)]

[0001-elec-pair-Fix-bug-incorrectly-hiding-syntax-properti.patch (text/x-patch, attachment)]

Message #44 received at 49629 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Allen Li <darkfeline <at> felesatra.moe>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 49629 <at> debbugs.gnu.org,
 Noam Postavsky <npostavs <at> gmail.com>
Subject: Re: bug#49629: 27.2; electric-pair-mode doesn't work for angle
 brackets in HTML file
Date: Sun, 26 Jun 2022 08:17:43 -0400

> 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

Message #47 received at 49629 <at> debbugs.gnu.org (full text, mbox):

From: Allen Li <darkfeline <at> felesatra.moe>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 49629 <at> debbugs.gnu.org,
 Noam Postavsky <npostavs <at> gmail.com>
Subject: Re: bug#49629: 27.2; electric-pair-mode doesn't work for angle
 brackets in HTML file
Date: Sun, 26 Jun 2022 17:33:37 -0700

[Message part 1 (text/plain, inline)]

On Sun, Jun 26, 2022 at 5:17 AM Stefan Monnier <monnier <at> 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
>
>

[Message part 2 (text/html, inline)]

Message #50 received at 49629 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Allen Li <darkfeline <at> felesatra.moe>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 49629 <at> debbugs.gnu.org,
 Noam Postavsky <npostavs <at> gmail.com>
Subject: Re: bug#49629: 27.2; electric-pair-mode doesn't work for angle
 brackets in HTML file
Date: Mon, 27 Jun 2022 08:39:31 -0400

> 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

Message #53 received at 49629 <at> debbugs.gnu.org (full text, mbox):

From: Allen Li <darkfeline <at> felesatra.moe>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 49629 <at> debbugs.gnu.org,
 Noam Postavsky <npostavs <at> gmail.com>
Subject: Re: bug#49629: 27.2; electric-pair-mode doesn't work for angle
 brackets in HTML file
Date: Sat, 2 Jul 2022 03:48:31 -0700

[Message part 1 (text/plain, inline)]

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 <at> 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
>
>

[Message part 2 (text/html, inline)]

[0001-elec-pair-Fix-bug-incorrectly-hiding-syntax-properti.patch (text/x-patch, attachment)]

Message #56 received at 49629 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Allen Li <darkfeline <at> felesatra.moe>
Cc: Noam Postavsky <npostavs <at> gmail.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, 49629 <at> debbugs.gnu.org
Subject: Re: bug#49629: 27.2; electric-pair-mode doesn't work for angle
 brackets in HTML file
Date: Sun, 03 Jul 2022 12:32:01 +0200

Allen Li <darkfeline <at> 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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.