bug#65805: 30.0.50; quoted-insert doesn't work in zap-to-char

bug#65805: 30.0.50; quoted-insert doesn't work in zap-to-char

[Filipp Gunbin]

Hi, I'm not sure this should be treated as bug (and can be fixed), but
still I find it inconvenient.

- emacs -Q
- Open new buffer in text mode
- Type "foo C-q C-i bar"
- Now, with point at "foo", isearch for tab character works: "C-s C-q C-i"
- But zap-to-char doesn't: "M-z C-q C-i RET" displays "Wrong answer"

Thanks.

In GNU Emacs 30.0.50 (build 27, x86_64-apple-darwin20.6.0, NS
 appkit-2022.70 Version 11.7.7 (Build 20G1345)) of 2023-09-06 built on
 fgunbin.local
Repository revision: 7055e47cde4d18b50ef11a1422a64d69f8a550a2
Repository branch: master
System Description:  macOS 11.7.7

Configured using:
 'configure --enable-check-lisp-object-type --with-file-notification=no'

Configured features:
ACL GLIB GNUTLS LCMS2 LIBXML2 MODULES NS PDUMPER PNG RSVG SQLITE3
THREADS TOOLKIT_SCROLL_BARS WEBP XIM ZLIB

Important settings:
  value of $LC_ALL: ru_RU.UTF-8
  value of $LC_CTYPE: UTF-8
  locale-coding-system: utf-8-unix

bug#65805: 30.0.50; quoted-insert doesn't work in zap-to-char

[Eli Zaretskii]

> From: Filipp Gunbin <fgunbin@fastmail.fm>
> Date: Thu, 07 Sep 2023 17:53:33 +0300
> 
> Hi, I'm not sure this should be treated as bug (and can be fixed), but
> still I find it inconvenient.
> 
> - emacs -Q
> - Open new buffer in text mode
> - Type "foo C-q C-i bar"
> - Now, with point at "foo", isearch for tab character works: "C-s C-q C-i"
> - But zap-to-char doesn't: "M-z C-q C-i RET" displays "Wrong answer"

That's because we switched to using read-from-minibuffer (via
read-char-from-minibuffer) instead of using read-char, for the dubious
benefit of being able to use history in this command, see bug#10477.
And read-from-minibuffer interprets some characters specially.

Stefan, is there a way to have this cake and eat it, too?

bug#65805: 30.0.50; quoted-insert doesn't work in zap-to-char

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

Eli Zaretskii [2023-09-07 18:07:46] wrote:

>> From: Filipp Gunbin <fgunbin@fastmail.fm>
>> Date: Thu, 07 Sep 2023 17:53:33 +0300
>> 
>> Hi, I'm not sure this should be treated as bug (and can be fixed), but
>> still I find it inconvenient.
>> 
>> - emacs -Q
>> - Open new buffer in text mode
>> - Type "foo C-q C-i bar"
>> - Now, with point at "foo", isearch for tab character works: "C-s C-q C-i"
>> - But zap-to-char doesn't: "M-z C-q C-i RET" displays "Wrong answer"
>
> That's because we switched to using read-from-minibuffer (via
> read-char-from-minibuffer) instead of using read-char, for the dubious
> benefit of being able to use history in this command, see bug#10477.
> And read-from-minibuffer interprets some characters specially.
>
> Stefan, is there a way to have this cake and eat it, too?

How 'bout the patch below?

AFAICT it would also make it unnecessary to use
`read-char-from-minibuffer-insert-char`.


        Stefan


diff --git a/lisp/subr.el b/lisp/subr.el
index 216981eaf31..5b268e9a069 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -3733,7 +3733,14 @@ read-char-from-minibuffer
                 read-char-from-minibuffer-map))
          ;; Protect this-command when called from pre-command-hook (bug#45029)
          (this-command this-command)
-         (result (progn
+         (result (minibuffer-with-setup-hook
+                     (lambda ()
+                       (add-hook 'post-command-hook
+                                 (lambda ()
+                                   (if (= (1+ (minibuffer-prompt-end))
+                                          (point-max))
+                                       (exit-minibuffer)))
+                                 'local))
                    ;; Disable text conversion if it is enabled.
                    ;; (bug#65370)
                    (when (fboundp 'set-text-conversion-style)

bug#65805: 30.0.50; quoted-insert doesn't work in zap-to-char

[Juri Linkov]

> AFAICT it would also make it unnecessary to use
> `read-char-from-minibuffer-insert-char`.
>
> @@ -3733,7 +3733,14 @@ read-char-from-minibuffer
>                  read-char-from-minibuffer-map))
>           ;; Protect this-command when called from pre-command-hook (bug#45029)
>           (this-command this-command)
> -         (result (progn
> +         (result (minibuffer-with-setup-hook
> +                     (lambda ()
> +                       (add-hook 'post-command-hook
> +                                 (lambda ()
> +                                   (if (= (1+ (minibuffer-prompt-end))
> +                                          (point-max))
> +                                       (exit-minibuffer)))
> +                                 'local))

I have a patch where exit-minibuffer in read-char-from-minibuffer-map
is bound to a new command that ensures that the minibuffer contains
exactly 1 character before exiting.  But need more testing to see
what is better.

bug#65805: 30.0.50; quoted-insert doesn't work in zap-to-char

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

>> @@ -3733,7 +3733,14 @@ read-char-from-minibuffer
>>                  read-char-from-minibuffer-map))
>>           ;; Protect this-command when called from pre-command-hook (bug#45029)
>>           (this-command this-command)
>> -         (result (progn
>> +         (result (minibuffer-with-setup-hook
>> +                     (lambda ()
>> +                       (add-hook 'post-command-hook
>> +                                 (lambda ()
>> +                                   (if (= (1+ (minibuffer-prompt-end))
>> +                                          (point-max))
>> +                                       (exit-minibuffer)))
>> +                                 'local))
>
> I have a patch where exit-minibuffer in read-char-from-minibuffer-map
> is bound to a new command that ensures that the minibuffer contains
> exactly 1 character before exiting.  But need more testing to see
> what is better.

`read-char-from-minibuffer` usually doesn't require the user to use an
explicit key to exit (because it tries to mimic `read-char`).


        Stefan

bug#65805: 30.0.50; quoted-insert doesn't work in zap-to-char

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Filipp Gunbin <fgunbin@fastmail.fm>,  65805@debbugs.gnu.org
> Date: Thu, 07 Sep 2023 12:47:03 -0400
> 
> Eli Zaretskii [2023-09-07 18:07:46] wrote:
> 
> > That's because we switched to using read-from-minibuffer (via
> > read-char-from-minibuffer) instead of using read-char, for the dubious
> > benefit of being able to use history in this command, see bug#10477.
> > And read-from-minibuffer interprets some characters specially.
> >
> > Stefan, is there a way to have this cake and eat it, too?
> 
> How 'bout the patch below?

Is this appropriate for emacs-29?

> AFAICT it would also make it unnecessary to use
> `read-char-from-minibuffer-insert-char`.

Not sure I understand what are you proposing here about that function.

Thanks.

bug#65805: 30.0.50; quoted-insert doesn't work in zap-to-char

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

>> > That's because we switched to using read-from-minibuffer (via
>> > read-char-from-minibuffer) instead of using read-char, for the dubious
>> > benefit of being able to use history in this command, see bug#10477.
>> > And read-from-minibuffer interprets some characters specially.
>> > Stefan, is there a way to have this cake and eat it, too?
>> How 'bout the patch below?

Had a missing `nil`.

> Is this appropriate for emacs-29?

It looks fairly safe, but is it a regression in Emacs-29 or was it
already there in Emacs-28?  If it's not a regression in Emacs-29, I'd
keep it in `master`.

>> AFAICT it would also make it unnecessary to use
>> `read-char-from-minibuffer-insert-char`.
> Not sure I understand what are you proposing here about that function.

`read-char-from-minibuffer-insert-char` is a function which just inserts
a char and then exits the minibuffer.  With my patch,
`self-insert-command` should work as well for that.


        Stefan

bug#65805: 30.0.50; quoted-insert doesn't work in zap-to-char

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: fgunbin@fastmail.fm,  65805@debbugs.gnu.org
> Date: Sun, 10 Sep 2023 19:14:34 -0400
> 
> >> > That's because we switched to using read-from-minibuffer (via
> >> > read-char-from-minibuffer) instead of using read-char, for the dubious
> >> > benefit of being able to use history in this command, see bug#10477.
> >> > And read-from-minibuffer interprets some characters specially.
> >> > Stefan, is there a way to have this cake and eat it, too?
> >> How 'bout the patch below?
> 
> Had a missing `nil`.
> 
> > Is this appropriate for emacs-29?
> 
> It looks fairly safe, but is it a regression in Emacs-29 or was it
> already there in Emacs-28?

It was already in Emacs 28.

> If it's not a regression in Emacs-29, I'd keep it in `master`.

OK, then please install on master and close this bug when you do.

> >> AFAICT it would also make it unnecessary to use
> >> `read-char-from-minibuffer-insert-char`.
> > Not sure I understand what are you proposing here about that function.
> 
> `read-char-from-minibuffer-insert-char` is a function which just inserts
> a char and then exits the minibuffer.  With my patch,
> `self-insert-command` should work as well for that.

I'm okay with making such a change, but we'd probably need a
compatibility shim if we do that?  So maybe it isn't worth the hassle,
just make read-char-from-minibuffer-insert-char obsolete?

bug#65805: 30.0.50; quoted-insert doesn't work in zap-to-char

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

> OK, then please install on master and close this bug when you do.

Done, closing.

>> `read-char-from-minibuffer-insert-char` is a function which just inserts
>> a char and then exits the minibuffer.  With my patch,
>> `self-insert-command` should work as well for that.
>
> I'm okay with making such a change, but we'd probably need a
> compatibility shim if we do that?  So maybe it isn't worth the hassle,
> just make read-char-from-minibuffer-insert-char obsolete?

Actually, it's still used at one place.  We could get rid of that use as
well, but I haven't bothered (yet :-).


        Stefan

bug#65805: 30.0.50; quoted-insert doesn't work in zap-to-char

[Juri Linkov]

>> I have a patch where exit-minibuffer in read-char-from-minibuffer-map
>> is bound to a new command that ensures that the minibuffer contains
>> exactly 1 character before exiting.  But need more testing to see
>> what is better.
>
> `read-char-from-minibuffer` usually doesn't require the user to use an
> explicit key to exit (because it tries to mimic `read-char`).

I don't remember how I got into a situation that required typing RET
to exit the minibuffer.  Probably pasted a character into the minibuffer
with C-y.  I see that your patch handles this now.

@@ -3499,7 +3499,7 @@ read-char-from-minibuffer-map
   (let ((map (make-sparse-keymap)))
     (set-keymap-parent map minibuffer-local-map)
 
-    (define-key map [remap self-insert-command] #'read-char-from-minibuffer-insert-char)
+    ;; (define-key map [remap self-insert-command] #'read-char-from-minibuffer-insert-char)
     (define-key map [remap exit-minibuffer] #'read-char-from-minibuffer-insert-other)

Maybe remapping exit-minibuffer is not needed anymore too?

@@ -3530,7 +3530,7 @@ read-char-from-minibuffer-insert-other
 such key, this command discard all minibuffer input and displays
 an error message."
   (interactive)
-  (when (minibufferp)
+  (when (minibufferp) ;;FIXME: Why?

Because of bug#45029.  Maybe this bug number should be mentioned
in the comments.

@@ -3578,6 +3578,10 @@ read-char-from-minibuffer
                                         (interactive)
                                         (let ((help-form msg)) ; lexically bound msg
                                           (help-form-show)))))
+                        ;; FIXME: We use `read-char-from-minibuffer-insert-char'
+                        ;; here only as a kind of alias of `self-insert-command'
+                        ;; to prevent those keys from being remapped to
+                        ;; `read-char-from-minibuffer-insert-other'.
                         (dolist (char chars)
                           (define-key map (vector char)
                                       #'read-char-from-minibuffer-insert-char))
                         (define-key map [remap self-insert-command]
                                     #'read-char-from-minibuffer-insert-other)

Interesting that [remap self-insert-command] is applied even to the
commands explicitly bound to 'self-insert-command' in the same keymap.
I see no way to override this.  So 'read-char-from-minibuffer-insert-char'
can't be replaced with 'self-insert-command'.

@@ -3589,7 +3593,15 @@ read-char-from-minibuffer
                 read-char-from-minibuffer-map))
          ;; Protect this-command when called from pre-command-hook (bug#45029)
          (this-command this-command)
-         (result (progn
+         (result (minibuffer-with-setup-hook
+		     (lambda ()
+		       (add-hook 'post-command-hook
+				 (lambda ()
+				   ;; FIXME: Should we use `<='?
+				   (if (= (1+ (minibuffer-prompt-end))
+					  (point-max))
+                                       (exit-minibuffer)))
+				 nil 'local))

I think we should use `<=' to handle the case when a string with
more than 1 character is pasted with 'C-y'.  This still requires
some special-handling like I intended to do in a new command that
exits the minibuffer where it should remove extra characters to
leave only one: either the first or the last one.  Perhaps leaving
the first character makes more sense.

bug#65805: 30.0.50; quoted-insert doesn't work in zap-to-char

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

>> `read-char-from-minibuffer` usually doesn't require the user to use an
>> explicit key to exit (because it tries to mimic `read-char`).
> I don't remember how I got into a situation that required typing RET
> to exit the minibuffer.  Probably pasted a character into the minibuffer
> with C-y.  I see that your patch handles this now.

Yes, I tested `C-y` as well :-)
Tho the ideal behavior when doing things like "C-y inserts a string of
length > 1" is unclear.

> @@ -3499,7 +3499,7 @@ read-char-from-minibuffer-map
>    (let ((map (make-sparse-keymap)))
>      (set-keymap-parent map minibuffer-local-map)
>  
> -    (define-key map [remap self-insert-command] #'read-char-from-minibuffer-insert-char)
> +    ;; (define-key map [remap self-insert-command] #'read-char-from-minibuffer-insert-char)
>      (define-key map [remap exit-minibuffer] #'read-char-from-minibuffer-insert-other)
>
> Maybe remapping exit-minibuffer is not needed anymore too?

I think it's still needed (RET would otherwise cause exit with an empty
minibuffer, and hence no "char").

> @@ -3530,7 +3530,7 @@ read-char-from-minibuffer-insert-other
>  such key, this command discard all minibuffer input and displays
>  an error message."
>    (interactive)
> -  (when (minibufferp)
> +  (when (minibufferp) ;;FIXME: Why?
>
> Because of bug#45029.

Thank you.

> Maybe this bug number should be mentioned in the comments.

[ Or maybe I should have bothered to `C-x v h` :-(  ]

> Interesting that [remap self-insert-command] is applied even to the
> commands explicitly bound to 'self-insert-command' in the same keymap.

Indeed, the command remapping doesn't pay attention to "in which keymap
did I find this binding".  We could try and refine it, but it could be
a lot of work, I think.

> I see no way to override this.  So 'read-char-from-minibuffer-insert-char'
> can't be replaced with 'self-insert-command'.

Exactly.  We could use something like:

    (defalias 'read-char-from-minibuffer-insert-char 'self-insert-command)

> @@ -3589,7 +3593,15 @@ read-char-from-minibuffer
>                  read-char-from-minibuffer-map))
>           ;; Protect this-command when called from pre-command-hook (bug#45029)
>           (this-command this-command)
> -         (result (progn
> +         (result (minibuffer-with-setup-hook
> +		     (lambda ()
> +		       (add-hook 'post-command-hook
> +				 (lambda ()
> +				   ;; FIXME: Should we use `<='?
> +				   (if (= (1+ (minibuffer-prompt-end))
> +					  (point-max))
> +                                       (exit-minibuffer)))
> +				 nil 'local))
>
> I think we should use `<=' to handle the case when a string with
> more than 1 character is pasted with 'C-y'.

Indeed, that's one option.  But I was thinking that maybe we should try
and signal an error.  Do the users really expect that yanking a string
into that prompt will end up picking just the first char of that string?


        Stefan

bug#65805: 30.0.50; quoted-insert doesn't work in zap-to-char

[Juri Linkov]

>> @@ -3499,7 +3499,7 @@ read-char-from-minibuffer-map
>>    (let ((map (make-sparse-keymap)))
>>      (set-keymap-parent map minibuffer-local-map)
>>
>> -    (define-key map [remap self-insert-command] #'read-char-from-minibuffer-insert-char)
>> +    ;; (define-key map [remap self-insert-command] #'read-char-from-minibuffer-insert-char)
>>      (define-key map [remap exit-minibuffer] #'read-char-from-minibuffer-insert-other)
>>
>> Maybe remapping exit-minibuffer is not needed anymore too?
>
> I think it's still needed (RET would otherwise cause exit with an empty
> minibuffer, and hence no "char").

Hmm, maybe then RET should be bound to a command that signals an error
since the behavior is undefined for the case when the minibuffer gets
more characters than one, or when the minibuffer is empty.

>> I see no way to override this.  So 'read-char-from-minibuffer-insert-char'
>> can't be replaced with 'self-insert-command'.
>
> Exactly.  We could use something like:
>
>     (defalias 'read-char-from-minibuffer-insert-char 'self-insert-command)

I have only one doubt how this will affect the users who already bind
this command in own configs.  Probably there should be no problems.

>> @@ -3589,7 +3593,15 @@ read-char-from-minibuffer
>> +         (result (minibuffer-with-setup-hook
>> +		     (lambda ()
>> +		       (add-hook 'post-command-hook
>> +				 (lambda ()
>> +				   ;; FIXME: Should we use `<='?
>> +				   (if (= (1+ (minibuffer-prompt-end))
>> +					  (point-max))
>> +                                       (exit-minibuffer)))
>> +				 nil 'local))
>>
>> I think we should use `<=' to handle the case when a string with
>> more than 1 character is pasted with 'C-y'.
>
> Indeed, that's one option.  But I was thinking that maybe we should try
> and signal an error.  Do the users really expect that yanking a string
> into that prompt will end up picking just the first char of that string?

Picking just the first char looks like a random choice.  So indeed maybe
better to signal an error.