* bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding @ 2024-06-03 12:40 Arash Esbati 2024-06-03 13:50 ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 21+ messages in thread From: Arash Esbati @ 2024-06-03 12:40 UTC (permalink / raw) To: 71337 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding 2024-06-03 12:40 bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding Arash Esbati @ 2024-06-03 13:50 ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-04 5:53 ` Arash Esbati 0 siblings, 1 reply; 21+ messages in thread From: Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-03 13:50 UTC (permalink / raw) To: Arash Esbati; +Cc: 71337 [-- 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'. ^ permalink raw reply related [flat|nested] 21+ messages in thread
* bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding 2024-06-03 13:50 ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-04 5:53 ` Arash Esbati 2024-06-04 7:30 ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 21+ messages in thread From: Arash Esbati @ 2024-06-04 5:53 UTC (permalink / raw) To: Stephen Berman; +Cc: 71337 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding 2024-06-04 5:53 ` Arash Esbati @ 2024-06-04 7:30 ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-04 7:37 ` João Távora 0 siblings, 1 reply; 21+ messages in thread From: Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-04 7:30 UTC (permalink / raw) To: Arash Esbati; +Cc: 71337, João Távora 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding 2024-06-04 7:30 ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-04 7:37 ` João Távora 2024-06-04 8:08 ` João Távora 2024-06-04 8:09 ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 2 replies; 21+ messages in thread From: João Távora @ 2024-06-04 7:37 UTC (permalink / raw) To: Stephen Berman; +Cc: Arash Esbati, 71337 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding 2024-06-04 7:37 ` João Távora @ 2024-06-04 8:08 ` João Távora 2024-06-04 8:18 ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-04 9:24 ` Eli Zaretskii 2024-06-04 8:09 ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors 1 sibling, 2 replies; 21+ messages in thread From: João Távora @ 2024-06-04 8:08 UTC (permalink / raw) To: Stephen Berman, Stefan Monnier; +Cc: Arash Esbati, 71337 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding 2024-06-04 8:08 ` João Távora @ 2024-06-04 8:18 ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-04 14:52 ` Eli Zaretskii 2024-06-04 9:24 ` Eli Zaretskii 1 sibling, 1 reply; 21+ messages in thread From: Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-04 8:18 UTC (permalink / raw) To: João Távora; +Cc: Arash Esbati, Stefan Monnier, 71337 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding 2024-06-04 8:18 ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-04 14:52 ` Eli Zaretskii 0 siblings, 0 replies; 21+ messages in thread From: Eli Zaretskii @ 2024-06-04 14:52 UTC (permalink / raw) To: Stephen Berman; +Cc: arash, 71337, joaotavora, monnier > 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding 2024-06-04 8:08 ` João Távora 2024-06-04 8:18 ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-04 9:24 ` Eli Zaretskii 2024-06-04 10:09 ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors ` (2 more replies) 1 sibling, 3 replies; 21+ messages in thread From: Eli Zaretskii @ 2024-06-04 9:24 UTC (permalink / raw) To: 71337, joaotavora, stephen.berman, monnier; +Cc: arash 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? ^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding 2024-06-04 9:24 ` Eli Zaretskii @ 2024-06-04 10:09 ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-04 10:49 ` João Távora ` (2 more replies) 2024-06-04 12:33 ` Arash Esbati 2024-06-04 14:21 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors 2 siblings, 3 replies; 21+ messages in thread From: Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-04 10:09 UTC (permalink / raw) To: Eli Zaretskii; +Cc: arash, 71337, joaotavora, monnier 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding 2024-06-04 10:09 ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-04 10:49 ` João Távora 2024-06-04 11:04 ` Eli Zaretskii 2024-06-04 14:56 ` Eli Zaretskii 2 siblings, 0 replies; 21+ messages in thread From: João Távora @ 2024-06-04 10:49 UTC (permalink / raw) To: Stephen Berman; +Cc: arash, eliz, 71337, monnier 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding 2024-06-04 10:09 ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-04 10:49 ` João Távora @ 2024-06-04 11:04 ` Eli Zaretskii 2024-06-04 14:56 ` Eli Zaretskii 2 siblings, 0 replies; 21+ messages in thread From: Eli Zaretskii @ 2024-06-04 11:04 UTC (permalink / raw) To: Stephen Berman; +Cc: arash, 71337, joaotavora, monnier 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding 2024-06-04 10:09 ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-04 10:49 ` João Távora 2024-06-04 11:04 ` Eli Zaretskii @ 2024-06-04 14:56 ` Eli Zaretskii 2024-06-04 15:43 ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors 2 siblings, 1 reply; 21+ messages in thread From: Eli Zaretskii @ 2024-06-04 14:56 UTC (permalink / raw) To: Stephen Berman; +Cc: arash, 71337, joaotavora, monnier > 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? ^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding 2024-06-04 14:56 ` Eli Zaretskii @ 2024-06-04 15:43 ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 0 replies; 21+ messages in thread From: Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-04 15:43 UTC (permalink / raw) To: Eli Zaretskii; +Cc: arash, 71337, joaotavora, monnier 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding 2024-06-04 9:24 ` Eli Zaretskii 2024-06-04 10:09 ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-04 12:33 ` Arash Esbati 2024-06-04 13:36 ` João Távora 2024-06-04 14:21 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors 2 siblings, 1 reply; 21+ messages in thread From: Arash Esbati @ 2024-06-04 12:33 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 71337, stephen.berman, joaotavora, monnier 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding 2024-06-04 12:33 ` Arash Esbati @ 2024-06-04 13:36 ` João Távora 2024-06-04 14:09 ` Arash Esbati 0 siblings, 1 reply; 21+ messages in thread From: João Távora @ 2024-06-04 13:36 UTC (permalink / raw) To: Arash Esbati; +Cc: eliz, stephen.berman, 71337, monnier 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding 2024-06-04 13:36 ` João Távora @ 2024-06-04 14:09 ` Arash Esbati 0 siblings, 0 replies; 21+ messages in thread From: Arash Esbati @ 2024-06-04 14:09 UTC (permalink / raw) To: João Távora; +Cc: eliz, stephen.berman, 71337, monnier 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding 2024-06-04 9:24 ` Eli Zaretskii 2024-06-04 10:09 ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-04 12:33 ` Arash Esbati @ 2024-06-04 14:21 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-15 8:10 ` Eli Zaretskii 2 siblings, 1 reply; 21+ messages in thread From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-04 14:21 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Arash Esbati, Stephen Berman, João Távora, 71337 > I think internal_self_insert should overwrite last-command-event or > something, if we want to support this kind of scenario. Stefan, WDYT? +1 ^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding 2024-06-04 14:21 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-15 8:10 ` Eli Zaretskii 2024-06-16 10:42 ` Arash Esbati 0 siblings, 1 reply; 21+ messages in thread From: Eli Zaretskii @ 2024-06-15 8:10 UTC (permalink / raw) To: Stefan Monnier; +Cc: arash, stephen.berman, joaotavora, 71337-done > 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding 2024-06-15 8:10 ` Eli Zaretskii @ 2024-06-16 10:42 ` Arash Esbati 0 siblings, 0 replies; 21+ messages in thread From: Arash Esbati @ 2024-06-16 10:42 UTC (permalink / raw) To: 71337; +Cc: eliz Eli Zaretskii <eliz@gnu.org> writes: > Now done, and closing the bug. Thanks, it works as expected now. Best, Arash ^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding 2024-06-04 7:37 ` João Távora 2024-06-04 8:08 ` João Távora @ 2024-06-04 8:09 ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors 1 sibling, 0 replies; 21+ messages in thread From: Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-04 8:09 UTC (permalink / raw) To: João Távora; +Cc: Arash Esbati, 71337 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2024-06-16 10:42 UTC | newest] Thread overview: 21+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-06-03 12:40 bug#71337: 30.0.50; `electric-pair-mode' and custom keybinding Arash Esbati 2024-06-03 13:50 ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-04 5:53 ` Arash Esbati 2024-06-04 7:30 ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-04 7:37 ` João Távora 2024-06-04 8:08 ` João Távora 2024-06-04 8:18 ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-04 14:52 ` Eli Zaretskii 2024-06-04 9:24 ` Eli Zaretskii 2024-06-04 10:09 ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-04 10:49 ` João Távora 2024-06-04 11:04 ` Eli Zaretskii 2024-06-04 14:56 ` Eli Zaretskii 2024-06-04 15:43 ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-04 12:33 ` Arash Esbati 2024-06-04 13:36 ` João Távora 2024-06-04 14:09 ` Arash Esbati 2024-06-04 14:21 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-06-15 8:10 ` Eli Zaretskii 2024-06-16 10:42 ` Arash Esbati 2024-06-04 8:09 ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).