bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding

bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding

[Arash Esbati]

Hi all,

I have a custom keybinding "s-ß" with my German keyboard, but the issue
I'm facing is also reproducible like this:

• emacs -Q
• In scratch, eval:

  (progn
    (setq debug-on-error t)
    (electric-pair-mode  1)
    (keymap-global-set "s-#" (lambda (arg)
                               "Insert ARG backslash(es)."
                               (interactive "*p")
                               (self-insert-command arg ?\\))))

• Now hit "s-#" and the debugger says (linebreaks added manually):

Debugger entered--Lisp error: (wrong-type-argument characterp 8388643)
  #f(compiled-function () #<bytecode -0x15954a2c5d74b890>)()
  electric-pair--with-syntax-1(nil #f(compiled-function () #<bytecode -0x15954a2c5d74b890>))
  electric-pair-syntax-info(8388643)
  electric-pair-post-self-insert-function()
  self-insert-command(1 92)
  #f(lambda (arg) [t] "Insert ARG backslash(es)." (interactive "*p")
   (self-insert-command arg 92))(1)
  funcall-interactively(#f(lambda (arg) [t] "Insert ARG backslash(es)."
   (interactive "*p") (self-insert-command arg 92)) 1)
  command-execute(#f(lambda (arg) [t] "Insert ARG backslash(es)."
   (interactive "*p") (self-insert-command arg 92)))

Running the exercise with (electric-pair-mode -1) doesn't throw an
error.

Best, Arash

In GNU Emacs 30.0.50 (build 1, aarch64-apple-darwin23.5.0, NS
 appkit-2487.60 Version 14.5 (Build 23F79)) of 2024-05-28 built on
 MacMutant.local
Repository revision: 066e9b598858cc4c0b666b12242f07a7fdf3e073
Repository branch: master
Windowing system distributor 'Apple', version 10.3.2487
System Description:  macOS 14.5

Configured using:
 'configure --with-ns --without-pop --without-mailutils --with-threads
 --with-modules --with-native-compilation --without-compress-install
 'CFLAGS=-O2 -g0 -pipe'
 'CPPFLAGS=-I/opt/homebrew/Cellar/gcc/14.1.0/include
 -I/opt/homebrew/Cellar/libgccjit/14.1.0/include
 -I/opt/homebrew/Cellar/gmp/6.3.0/include'
 'LDFLAGS=-L/opt/homebrew/Cellar/gcc/14.1.0/lib/gcc/current
 -L/opt/homebrew/Cellar/gmp/6.3.0/lib''

Configured features:
ACL GLIB GMP GNUTLS LCMS2 LIBXML2 MODULES NATIVE_COMP NOTIFY KQUEUE NS
PDUMPER PNG RSVG SQLITE3 THREADS TOOLKIT_SCROLL_BARS TREE_SITTER WEBP
XIM ZLIB

bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding

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

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

On Mon, 03 Jun 2024 14:40:47 +0200 Arash Esbati <arash@gnu.org> wrote:

> Hi all,
>
> I have a custom keybinding "s-ß" with my German keyboard, but the issue
> I'm facing is also reproducible like this:
>
> • emacs -Q
> • In scratch, eval:
>
>   (progn
>     (setq debug-on-error t)
>     (electric-pair-mode  1)
>     (keymap-global-set "s-#" (lambda (arg)
>                                "Insert ARG backslash(es)."
>                                (interactive "*p")
>                                (self-insert-command arg ?\\))))
>
> • Now hit "s-#" and the debugger says (linebreaks added manually):
>
> Debugger entered--Lisp error: (wrong-type-argument characterp 8388643)
>   #f(compiled-function () #<bytecode -0x15954a2c5d74b890>)()
>   electric-pair--with-syntax-1(nil #f(compiled-function () #<bytecode -0x15954a2c5d74b890>))
>   electric-pair-syntax-info(8388643)
>   electric-pair-post-self-insert-function()
>   self-insert-command(1 92)
>   #f(lambda (arg) [t] "Insert ARG backslash(es)." (interactive "*p")
>    (self-insert-command arg 92))(1)
>   funcall-interactively(#f(lambda (arg) [t] "Insert ARG backslash(es)."
>    (interactive "*p") (self-insert-command arg 92)) 1)
>   command-execute(#f(lambda (arg) [t] "Insert ARG backslash(es)."
>    (interactive "*p") (self-insert-command arg 92)))
>
> Running the exercise with (electric-pair-mode -1) doesn't throw an
> error.

If the pairing in electric-pair-mode should only be triggered by
self-inserting characters (as the current code seems to require), then
the attached patch appears to avoid the above problem.

Steve Berman


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: electric-pair-post-self-insert-function patch --]
[-- Type: text/x-patch, Size: 7441 bytes --]

diff --git a/lisp/elec-pair.el b/lisp/elec-pair.el
index 40618e9ef38..1b5b6d401a9 100644
--- a/lisp/elec-pair.el
+++ b/lisp/elec-pair.el
@@ -528,71 +528,72 @@ electric-pair-post-self-insert-function
   and `electric-pair-skip-whitespace' (which see)."
   (let* ((pos (and electric-pair-mode (electric--after-char-pos)))
          (skip-whitespace-info))
-    (pcase (electric-pair-syntax-info last-command-event)
-      (`(,syntax ,pair ,unconditional ,_)
-       (cond
-        ((null pos) 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))
-           (delete-region pos (1- pos))
-           (electric-pair--insert pair)
-           (goto-char (mark))
-           (electric-pair--insert last-command-event)))
-        ;; Backslash-escaped: no pairing, no skipping.
-        ((save-excursion
-           (goto-char (1- pos))
-           (not (zerop (% (skip-syntax-backward "\\") 2))))
-         nil)
-        ;; 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 (1- pos) (if (eq skip-whitespace-info 'chomp)
-                                     (point)
-                                   pos))
-         (forward-char))
-        ;; Insert matching pair.
-        ((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))))))))
+    (when (characterp last-command-event)
+      (pcase (electric-pair-syntax-info last-command-event)
+        (`(,syntax ,pair ,unconditional ,_)
+         (cond
+          ((null pos) 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))
+             (delete-region pos (1- pos))
+             (electric-pair--insert pair)
+             (goto-char (mark))
+             (electric-pair--insert last-command-event)))
+          ;; Backslash-escaped: no pairing, no skipping.
+          ((save-excursion
+             (goto-char (1- pos))
+             (not (zerop (% (skip-syntax-backward "\\") 2))))
+           nil)
+          ;; 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 (1- pos) (if (eq skip-whitespace-info 'chomp)
+                                       (point)
+                                     pos))
+           (forward-char))
+          ;; Insert matching pair.
+          ((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)))))))))

 (defun electric-pair-open-newline-between-pairs-psif ()
   "Honor `electric-pair-open-newline-between-pairs'.

bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding

[Arash Esbati]

Stephen Berman <stephen.berman@gmx.net> writes:

> If the pairing in electric-pair-mode should only be triggered by
> self-inserting characters (as the current code seems to require), then
> the attached patch appears to avoid the above problem.

Thanks for your response.  Yes, your patch fixes the issue.  Do you want
to install it?

Best, Arash

bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding

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

On Tue, 04 Jun 2024 07:53:16 +0200 Arash Esbati <arash@gnu.org> wrote:

> Stephen Berman <stephen.berman@gmx.net> writes:
>
>> If the pairing in electric-pair-mode should only be triggered by
>> self-inserting characters (as the current code seems to require), then
>> the attached patch appears to avoid the above problem.
>
> Thanks for your response.  Yes, your patch fixes the issue.  Do you want
> to install it?

I think it would be prudent to wait for someone familiar with the
electric-pair-mode implementation (e.g. its author João Távora, Cc'd),
or failing that, an Emacs maintainer, to ok the patch.

Steve Berman

bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding

[João Távora]

On Tue, Jun 4, 2024 at 8:30 AM Stephen Berman <stephen.berman@gmx.net> wrote:
>
> On Tue, 04 Jun 2024 07:53:16 +0200 Arash Esbati <arash@gnu.org> wrote:
>
> > Stephen Berman <stephen.berman@gmx.net> writes:
> >
> >> If the pairing in electric-pair-mode should only be triggered by
> >> self-inserting characters (as the current code seems to require), then
> >> the attached patch appears to avoid the above problem.
> >
> > Thanks for your response.  Yes, your patch fixes the issue.  Do you want
> > to install it?
>
> I think it would be prudent to wait for someone familiar with the
> electric-pair-mode implementation (e.g. its author João Távora, Cc'd),
> or failing that, an Emacs maintainer, to ok the patch.

I will have a look if I can.  It would also be prudent to make sure
the unit tests all pass, like they presumably do before the patch.

João

bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding

[João Távora]

On Tue, Jun 4, 2024 at 8:37 AM João Távora <joaotavora@gmail.com> wrote:
>
> On Tue, Jun 4, 2024 at 8:30 AM Stephen Berman <stephen.berman@gmx.net> wrote:
> >
> > On Tue, 04 Jun 2024 07:53:16 +0200 Arash Esbati <arash@gnu.org> wrote:
> >
> > > Stephen Berman <stephen.berman@gmx.net> writes:
> > >
> > >> If the pairing in electric-pair-mode should only be triggered by
> > >> self-inserting characters (as the current code seems to require), then
> > >> the attached patch appears to avoid the above problem.
> > >
> > > Thanks for your response.  Yes, your patch fixes the issue.  Do you want
> > > to install it?
> >
> > I think it would be prudent to wait for someone familiar with the
> > electric-pair-mode implementation (e.g. its author João Távora, Cc'd),
> > or failing that, an Emacs maintainer, to ok the patch.
>
> I will have a look if I can.  It would also be prudent to make sure
> the unit tests all pass, like they presumably do before the patch.

I've had a look.  it looks like the problems is e-p-mode's assumption
that last-command-event is the thing to be inserted.  The fact that
it isn't here (somehow an innocent 92 is now a monstrous 8388643),
triggers the problem (8388643 isn't a representation of a character,
apparently).

But according to the docstring of post-self-insert-hook,
the assumption seems sane, and I probably coded for it.

  post-self-insert-hook is a variable defined in `src/cmds.c'.

  ...

  The hook can access the inserted character via `last-command-event'.
  ...

I don't think the patch is fully correct.  I think Stefan is the right
person to call here.  I've had an even briefer look at cmds.c and I
don't understand how that hook's promise is honoured.

João

bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding

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

On Tue, 4 Jun 2024 09:08:51 +0100 João Távora <joaotavora@gmail.com> wrote:

> On Tue, Jun 4, 2024 at 8:37 AM João Távora <joaotavora@gmail.com> wrote:
>>
>> On Tue, Jun 4, 2024 at 8:30 AM Stephen Berman <stephen.berman@gmx.net> wrote:
>> >
>> > On Tue, 04 Jun 2024 07:53:16 +0200 Arash Esbati <arash@gnu.org> wrote:
>> >
>> > > Stephen Berman <stephen.berman@gmx.net> writes:
>> > >
>> > >> If the pairing in electric-pair-mode should only be triggered by
>> > >> self-inserting characters (as the current code seems to require), then
>> > >> the attached patch appears to avoid the above problem.
>> > >
>> > > Thanks for your response.  Yes, your patch fixes the issue.  Do you want
>> > > to install it?
>> >
>> > I think it would be prudent to wait for someone familiar with the
>> > electric-pair-mode implementation (e.g. its author João Távora, Cc'd),
>> > or failing that, an Emacs maintainer, to ok the patch.
>>
>> I will have a look if I can.  It would also be prudent to make sure
>> the unit tests all pass, like they presumably do before the patch.
>
> I've had a look.  it looks like the problems is e-p-mode's assumption
> that last-command-event is the thing to be inserted.  The fact that
> it isn't here (somehow an innocent 92 is now a monstrous 8388643),
> triggers the problem (8388643 isn't a representation of a character,
> apparently).

Right, because (max-char) => 4194303 (#o17777777, #x3fffff)

> But according to the docstring of post-self-insert-hook,
> the assumption seems sane, and I probably coded for it.

That's what I thought, too.

>   post-self-insert-hook is a variable defined in `src/cmds.c'.
>
>   ...
>
>   The hook can access the inserted character via `last-command-event'.
>   ...
>
> I don't think the patch is fully correct.  I think Stefan is the right
> person to call here.  I've had an even briefer look at cmds.c and I
> don't understand how that hook's promise is honoured.

Thanks for taking a look.

Steve Berman

bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding

[Eli Zaretskii]

> Cc: Arash Esbati <arash@gnu.org>, Stefan Monnier <monnier@iro.umontreal.ca>,
>  71337@debbugs.gnu.org
> Date: Tue, 04 Jun 2024 10:18:07 +0200
> From:  Stephen Berman via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> > I've had a look.  it looks like the problems is e-p-mode's assumption
> > that last-command-event is the thing to be inserted.  The fact that
> > it isn't here (somehow an innocent 92 is now a monstrous 8388643),
> > triggers the problem (8388643 isn't a representation of a character,
> > apparently).
> 
> Right, because (max-char) => 4194303 (#o17777777, #x3fffff)

8388643 is #x800023, i.e. the character # with the super-modifier bit
set.  That's because this is the keyboard event that invoked the
command presented by Arash.

bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding

[Eli Zaretskii]

On June 4, 2024 11:08:51 AM GMT+03:00, "João Távora" <joaotavora@gmail.com> wrote:
> On Tue, Jun 4, 2024 at 8:37 AM João Távora <joaotavora@gmail.com> wrote:
> 
> I've had a look.  it looks like the problems is e-p-mode's assumption
> that last-command-event is the thing to be inserted.  The fact that
> it isn't here (somehow an innocent 92 is now a monstrous 8388643),
> triggers the problem (8388643 isn't a representation of a character,
> apparently).
> 
> But according to the docstring of post-self-insert-hook,
> the assumption seems sane, and I probably coded for it.
> 
>   post-self-insert-hook is a variable defined in `src/cmds.c'.
> 
>   ...
> 
>   The hook can access the inserted character via `last-command-event'.
>   ...
> 
> I don't think the patch is fully correct.  I think Stefan is the right
> person to call here.  I've had an even briefer look at cmds.c and I
> don't understand how that hook's promise is honoured.


It looks like Arash made the mistake of being the first one, ever, of invoking self-insert-command from Lisp with 2nd arg non-nil, and turning on electric-pair-mode on top of that.  When self-insert-command is called with 2 args, it uses the 2nd arg as the character to insert, but it does NOT overwrite last-command-event with that character.  So post-self-insert-hook sees the wrong event and rightfully barfs.

Which means the patch proposed by Stephen is not TRT, because it means electric-pair-mode will ignore the inserted backslashes.

I think internal_self_insert should overwrite last-command-event or something, if we want to support this kind of scenario.  Stefan, WDYT?

bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding

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

On Tue, 04 Jun 2024 12:24:59 +0300 Eli Zaretskii <eliz@gnu.org> wrote:

> On June 4, 2024 11:08:51 AM GMT+03:00, "João Távora" <joaotavora@gmail.com> wrote:
>> On Tue, Jun 4, 2024 at 8:37 AM João Távora <joaotavora@gmail.com> wrote:
>> 
>> I've had a look.  it looks like the problems is e-p-mode's assumption
>> that last-command-event is the thing to be inserted.  The fact that
>> it isn't here (somehow an innocent 92 is now a monstrous 8388643),
>> triggers the problem (8388643 isn't a representation of a character,
>> apparently).
>> 
>> But according to the docstring of post-self-insert-hook,
>> the assumption seems sane, and I probably coded for it.
>> 
>>   post-self-insert-hook is a variable defined in `src/cmds.c'.
>> 
>>   ...
>> 
>>   The hook can access the inserted character via `last-command-event'.
>>   ...
>> 
>> I don't think the patch is fully correct.  I think Stefan is the right
>> person to call here.  I've had an even briefer look at cmds.c and I
>> don't understand how that hook's promise is honoured.
>
>
> It looks like Arash made the mistake of being the first one, ever, of invoking
> self-insert-command from Lisp with 2nd arg non-nil, and turning on
> electric-pair-mode on top of that.  When self-insert-command is called with 2
> args, it uses the 2nd arg as the character to insert, but it does NOT
> overwrite last-command-event with that character.  So post-self-insert-hook
> sees the wrong event and rightfully barfs.
>
> Which means the patch proposed by Stephen is not TRT, because it means
> electric-pair-mode will ignore the inserted backslashes.

My patch should only make electric-pair-mode ignore key sequences which
don't satisfy characterp, e.g. "s-#" or "C-#".  I just tested my patch
after giving ?\ open parenthesis syntax, binding it to "C-#" and
enabling electric-pair-mode, and what I see is that typing `C-#' inserts
a "\" while typing `\' insert "\\".  Is this not the desired behavior?
(Of course, that doesn't means a lower-level change isn't preferable.)

Steve Berman

bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding

[João Távora]

On Tue, Jun 4, 2024 at 11:09 AM Stephen Berman <stephen.berman@gmx.net> wrote:

> enabling electric-pair-mode, and what I see is that typing `C-#' inserts
> a "\" while typing `\' insert "\\".  Is this not the desired behavior?

Seems so, but it looks very odd to check if last_command_event is a
character when checked in post-self-insert-hook, because that hook
explicitly says it must be one.  The fact that it works at all also
suggests the hook is being called twice. Anyway, if the workaround
is needed for whatever reason, I'd put it in electric-pair-syntax-info
with a prominent comment pointing to this bug.

bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding

[Eli Zaretskii]

On June 4, 2024 1:09:09 PM GMT+03:00, Stephen Berman <stephen.berman@gmx.net> wrote:
> On Tue, 04 Jun 2024 12:24:59 +0300 Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > On June 4, 2024 11:08:51 AM GMT+03:00, "João Távora" <joaotavora@gmail.com> wrote:
> >> On Tue, Jun 4, 2024 at 8:37 AM João Távora <joaotavora@gmail.com> wrote:
> >> 
> >> I've had a look.  it looks like the problems is e-p-mode's assumption
> >> that last-command-event is the thing to be inserted.  The fact that
> >> it isn't here (somehow an innocent 92 is now a monstrous 8388643),
> >> triggers the problem (8388643 isn't a representation of a character,
> >> apparently).
> >> 
> >> But according to the docstring of post-self-insert-hook,
> >> the assumption seems sane, and I probably coded for it.
> >> 
> >>   post-self-insert-hook is a variable defined in `src/cmds.c'.
> >> 
> >>   ...
> >> 
> >>   The hook can access the inserted character via `last-command-event'.
> >>   ...
> >> 
> >> I don't think the patch is fully correct.  I think Stefan is the right
> >> person to call here.  I've had an even briefer look at cmds.c and I
> >> don't understand how that hook's promise is honoured.
> >
> >
> > It looks like Arash made the mistake of being the first one, ever, of invoking
> > self-insert-command from Lisp with 2nd arg non-nil, and turning on
> > electric-pair-mode on top of that.  When self-insert-command is called with 2
> > args, it uses the 2nd arg as the character to insert, but it does NOT
> > overwrite last-command-event with that character.  So post-self-insert-hook
> > sees the wrong event and rightfully barfs.
> >
> > Which means the patch proposed by Stephen is not TRT, because it means
> > electric-pair-mode will ignore the inserted backslashes.
> 
> My patch should only make electric-pair-mode ignore key sequences which
> don't satisfy characterp, e.g. "s-#" or "C-#".  I just tested my patch
> after giving ?\ open parenthesis syntax, binding it to "C-#" and
> enabling electric-pair-mode, and what I see is that typing `C-#' inserts
> a "\" while typing `\' insert "\\".  Is this not the desired behavior?
> (Of course, that doesn't means a lower-level change isn't preferable.)
> 
> Steve Berman
> 
> 

My point is that if a character is inserted by Arash's command, your patch might ignore it, depending on the command's binding.

bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding

[Eli Zaretskii]

> From: Stephen Berman <stephen.berman@gmx.net>
> Cc: bug-gnu-emacs@gnu.org,  João Távora
>  <joaotavora@gmail.com>,  Stefan
>  Monnier <monnier@iro.umontreal.ca>,  Arash Esbati <arash@gnu.org>,
>   71337@debbugs.gnu.org
> Date: Tue, 04 Jun 2024 12:09:09 +0200
> 
> > Which means the patch proposed by Stephen is not TRT, because it means
> > electric-pair-mode will ignore the inserted backslashes.
> 
> My patch should only make electric-pair-mode ignore key sequences which
> don't satisfy characterp, e.g. "s-#" or "C-#".  I just tested my patch
> after giving ?\ open parenthesis syntax, binding it to "C-#" and
> enabling electric-pair-mode, and what I see is that typing `C-#' inserts
> a "\" while typing `\' insert "\\".  Is this not the desired behavior?

No, I don't think it's the desired behavior.  A command that invokes
self-insert-command internally most probably expects all the side
effects of self-insert-command to happen as if the character was
actually typed.  Otherwise, why use self-insert-command instead of,
say, insert?

bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding

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

On Tue, 04 Jun 2024 17:56:25 +0300 Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Stephen Berman <stephen.berman@gmx.net>
>> Cc: bug-gnu-emacs@gnu.org,  João Távora
>>  <joaotavora@gmail.com>,  Stefan
>>  Monnier <monnier@iro.umontreal.ca>,  Arash Esbati <arash@gnu.org>,
>>   71337@debbugs.gnu.org
>> Date: Tue, 04 Jun 2024 12:09:09 +0200
>> 
>> > Which means the patch proposed by Stephen is not TRT, because it means
>> > electric-pair-mode will ignore the inserted backslashes.
>> 
>> My patch should only make electric-pair-mode ignore key sequences which
>> don't satisfy characterp, e.g. "s-#" or "C-#".  I just tested my patch
>> after giving ?\ open parenthesis syntax, binding it to "C-#" and
>> enabling electric-pair-mode, and what I see is that typing `C-#' inserts
>> a "\" while typing `\' insert "\\".  Is this not the desired behavior?
>
> No, I don't think it's the desired behavior.  A command that invokes
> self-insert-command internally most probably expects all the side
> effects of self-insert-command to happen as if the character was
> actually typed.  Otherwise, why use self-insert-command instead of,
> say, insert?

Ok.  It seems I misunderstood the issue, so my patch is irrelevant.

Steve Berman

bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding

[Arash Esbati]

Eli Zaretskii <eliz@gnu.org> writes:

> It looks like Arash made the mistake of being the first one, ever, of
> invoking self-insert-command from Lisp with 2nd arg non-nil, and
> turning on electric-pair-mode on top of that.

Sorry for that :-)

> When self-insert-command is called with 2 args, it uses the 2nd arg as
> the character to insert, but it does NOT overwrite last-command-event
> with that character.  So post-self-insert-hook sees the wrong event
> and rightfully barfs.

Maybe I missed something, but is there a canonical way to achieve what I
want?  Background is that on macOS with German keyboard layout, I have
to hit 'Shift-Option-7' for a backslash which really bugs me.

> I think internal_self_insert should overwrite last-command-event or
> something, if we want to support this kind of scenario.  Stefan, WDYT?

It seems I'm not the only one with this idea:

  https://medium.com/@chasinglogic/defying-your-keyboard-with-elisp-366ab9cec85c

So supporting this kind of scenario could make sense.

Best, Arash

bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding

[João Távora]

On Tue, Jun 4, 2024 at 1:34 PM Arash Esbati <arash@gnu.org> wrote:

> but is there a canonical way to achieve what I want?

See below.

> It seems I'm not the only one with this idea:
>
>   https://medium.com/@chasinglogic/defying-your-keyboard-with-elisp-366ab9cec85c

A much simpler technique than that macro contraption is to

  (let ((last-command-event <character>)) (self-insert-command <nrepetitions>))

That's how I (and everyone?) used to do it before the relatively new
arg made it in (in broken form, apparently). Even elec-pair.el does it.

João

bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding

[Arash Esbati]

João Távora <joaotavora@gmail.com> writes:

> A much simpler technique than that macro contraption is to
>
>   (let ((last-command-event <character>)) (self-insert-command <nrepetitions>))
>
> That's how I (and everyone?) used to do it before the relatively new
> arg made it in (in broken form, apparently). Even elec-pair.el does it.

Thanks João, your suggestion solves my issue -- the macro based one
looked also a little too complicated to me.

So let's see what Emacs maintainers say about the possibly buggy
implementation.

Best, Arash

bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding

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

> I think internal_self_insert should overwrite last-command-event or
> something, if we want to support this kind of scenario.  Stefan, WDYT?

+1

bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: João Távora <joaotavora@gmail.com>,  Stephen
>  Berman
>  <stephen.berman@gmx.net>,  Arash Esbati <arash@gnu.org>,
>   71337@debbugs.gnu.org
> Date: Tue, 04 Jun 2024 10:21:11 -0400
> 
> > I think internal_self_insert should overwrite last-command-event or
> > something, if we want to support this kind of scenario.  Stefan, WDYT?
> 
> +1

Now done, and closing the bug.

bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding

[Arash Esbati]

Eli Zaretskii <eliz@gnu.org> writes:

> Now done, and closing the bug.

Thanks, it works as expected now.

Best, Arash

bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding

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

On Tue, 4 Jun 2024 08:37:48 +0100 João Távora <joaotavora@gmail.com> wrote:

> On Tue, Jun 4, 2024 at 8:30 AM Stephen Berman <stephen.berman@gmx.net> wrote:
>>
>> On Tue, 04 Jun 2024 07:53:16 +0200 Arash Esbati <arash@gnu.org> wrote:
>>
>> > Stephen Berman <stephen.berman@gmx.net> writes:
>> >
>> >> If the pairing in electric-pair-mode should only be triggered by
>> >> self-inserting characters (as the current code seems to require), then
>> >> the attached patch appears to avoid the above problem.
>> >
>> > Thanks for your response.  Yes, your patch fixes the issue.  Do you want
>> > to install it?
>>
>> I think it would be prudent to wait for someone familiar with the
>> electric-pair-mode implementation (e.g. its author João Távora, Cc'd),
>> or failing that, an Emacs maintainer, to ok the patch.
>
> I will have a look if I can.

Thanks.

>                               It would also be prudent to make sure
> the unit tests all pass, like they presumably do before the patch.

All tests passed with and without the patch in batch runs, but when
running ert manually on test/lisp/electric-tests.el, two tests failed,
but again both with and without the patch:

F electric-pair-autowrapping-7-at-point-1-in-tex-mode
    Electricity test in a ‘tex-mode’ buffer.
    (ert-test-failed
     ((should
       (equal (buffer-substring-no-properties (point-min) (point-max))
	      expected-string))
      :form (equal "foo''" "``foo''") :value nil :explanation
      (arrays-of-different-length 5 7 "foo''" "``foo''" first-mismatch-at 0)))

F electric-pair-autowrapping-7-at-point-2-in-tex-mode-in-strings
    Electricity test in a ‘tex-mode’ buffer.
    (ert-test-failed
     ((should
       (equal (buffer-substring-no-properties (point-min) (point-max))
	      expected-string))
      :form (equal "\"foo''\"" "\"``foo''\"") :value nil :explanation
      (arrays-of-different-length 7 9 "\"foo''\"" "\"``foo''\"" first-mismatch-at
				  1)))

Steve Berman