bug#7234: 24.0.50; strange message after text-scale-adjust

bug#7234: 24.0.50; strange message after text-scale-adjust

[Kenichi Handa]

When I start emacs as this:

% emacs -Q

and type:

  ESC : (text-scale-adjust 1) RET C-b

"(2)" is shown in the echo area.

With a file "~/temp" that contains only "abc\n", when I
start emacs as this:

% emacs -Q ~/temp

and type:

  ESC : (text-scale-adjust 1) RET C-f

"(6)" is shown.  When I type C-e instead of C-f above, "(5)"
is shown.  It seems that the number shown corresponds to the
character code of the last command.

This happesn both with emacs-23 and trunk branches.

---
Kenichi Handa
handa@m17n.org

bug#7234: 24.0.50; strange message after text-scale-adjust

[Eli Zaretskii]

> From: Kenichi Handa <handa@m17n.org>
> Date: Mon, 18 Oct 2010 13:09:29 +0900
> Cc: 
> 
>   ESC : (text-scale-adjust 1) RET C-b
> 
> "(2)" is shown in the echo area.
> 
>   ESC : (text-scale-adjust 1) RET C-f
> 
> "(6)" is shown.  When I type C-e instead of C-f above, "(5)"
> is shown.  It seems that the number shown corresponds to the
> character code of the last command.

That's because text-scale-adjust does at its end

    (push ev unread-command-events)))

and `push' returns its first argument.

bug#7234: 24.0.50; strange message after text-scale-adjust

[Kenichi Handa]

In article <E1P7jwu-0005UJ-KX@fencepost.gnu.org>, Eli Zaretskii <eliz@gnu.org> writes:

> > From: Kenichi Handa <handa@m17n.org>
> > Date: Mon, 18 Oct 2010 13:09:29 +0900
> > Cc: 
> > 
> >   ESC : (text-scale-adjust 1) RET C-b
> > 
> > "(2)" is shown in the echo area.
> > 
> >   ESC : (text-scale-adjust 1) RET C-f
> > 
> > "(6)" is shown.  When I type C-e instead of C-f above, "(5)"
> > is shown.  It seems that the number shown corresponds to the
> > character code of the last command.

> That's because text-scale-adjust does at its end

>     (push ev unread-command-events)))

> and `push' returns its first argument.

Ah, I see.  Thank you for the explanation.  I didn't realize
that when I typed C-f (or C-b), text-scale-adjust was not
yet finished.  It would be better that read-event call in
text-scale-adjust has some prompt:
 e.g. (read-event "+,-,0 for further adjustment: ")

Anyway, I'll close this bug.

---
Kenichi Handa
handa@m17n.org

bug#7234: 24.0.50; strange message after text-scale-adjust

[Miles Bader]

For non-interactive use, you should probably also be using
`text-scale-set' or `text-scale-increase' instead.

-Miles

-- 
Ocean, n. A body of water covering seven-tenths of a world designed for Man -
who has no gills.

Re: bug#7234: 24.0.50; strange message after text-scale-adjust

[Stefan Monnier]

> It would be better that read-event call in
> text-scale-adjust has some prompt:
> e.g. (read-event "+,-,0 for further adjustment: ")

Agreed.

> For non-interactive use, you should probably also be using
> `text-scale-set' or `text-scale-increase' instead.

BTW, I've been playing around with an alternative implementation for
such things.  The reason is that the use of
read-event/read-char/read-key has surprising side-effects: in those
cases I'm concerned with, the user expects that the command is already
finished and the behavior is somewhat consistent with that expectation,
but not completely: indeed typing "any" key sequence (except for a few
special ones) runs the usual corresponding command, but OTOH
post-command-hook and friends aren't run when expected.

So, instead I use a new function set-temporary-overlay-map which sets up
a keymap that's only active for a short period of time (by default just
for the next key-sequence):

Using submit branch file:///home/monnier/src/emacs/bzr/trunk/
=== modified file 'lisp/face-remap.el'
*** lisp/face-remap.el	2010-03-14 21:15:02 +0000
--- lisp/face-remap.el	2010-08-21 07:43:23 +0000
***************
*** 294,319 ****
  `text-scale-decrease' may be more appropriate."
    (interactive "p")
    (let ((first t)
- 	(step t)
  	(ev last-command-event)
  	(echo-keystrokes nil))
!     (while step
!       (let ((base (event-basic-type ev)))
! 	(cond ((or (eq base ?+) (eq base ?=))
! 	       (setq step inc))
! 	      ((eq base ?-)
! 	       (setq step (- inc)))
! 	      ((eq base ?0)
! 	       (setq step 0))
! 	      (first
! 	       (setq step inc))
! 	      (t
! 	       (setq step nil))))
!       (when step
  	(text-scale-increase step)
! 	(setq inc 1 first nil)
! 	(setq ev (read-event))))
!     (push ev unread-command-events)))
  
  \f
  ;; ----------------------------------------------------------------
--- 294,317 ----
  `text-scale-decrease' may be more appropriate."
    (interactive "p")
    (let ((first t)
  	(ev last-command-event)
  	(echo-keystrokes nil))
!     (let* ((base (event-basic-type ev))
!            (step
!             (case base
!               ((?+ ?=) inc)
!               (?- (- inc))
!               (?0 0)
!               (t inc))))
  	(text-scale-increase step)
!       (set-temporary-overlay-map
!        (let ((map (make-sparse-keymap)))
!          (define-key map [?=] 'text-scale-increase)
!          (define-key map [?0] (lambda () (interactive) (text-scale-increase 0)))
!          (define-key map [?+] 'text-scale-increase)
!          (define-key map [?-] 'text-scale-decrease)
!          map)
!        t))))
  
  \f
  ;; ----------------------------------------------------------------

Currently, my implementation of set-temporary-overlay-map (see appended)
is not 100% satisfactory, so it may require some changes to the C code,
but it already works well in practice.


        Stefan


(defun set-temporary-overlay-map (map &optional keep-pred)
  (let* ((clearfunsym (make-symbol "clear-temporary-overlay-map"))
         (overlaysym (make-symbol "t"))
         (alist (list (cons overlaysym map)))
         (clearfun
          `(lambda ()
             (unless ,(cond ((null keep-pred) nil)
                            ((eq t keep-pred)
                             `(eq this-command
                                  (lookup-key ',map
                                              (this-command-keys-vector))))
                            (t `(funcall ',keep-pred)))
               (remove-hook 'pre-command-hook ',clearfunsym)
               (cancel-timer ,overlaysym)
               (setq ,overlaysym nil)
               (save-current-buffer
                 (if (buffer-live-p ',(current-buffer))
                     (set-buffer ',(current-buffer)))
                 (setq emulation-mode-map-alists
                       (delq ',alist emulation-mode-map-alists)))))))
    (fset clearfunsym clearfun)
    (add-hook 'pre-command-hook clearfunsym)
    ;; Sadly, pre-command-hook is occasionally set to nil (if one of its
    ;; functions signals an error).  We should improve safe_run_hooks so as to
    ;; only remove the offending function rather than set the whole thing to
    ;; nil, but in the mean time, let's use an auxiliary timer to monitor
    ;; pre-command-hook to make sure we don't end up with a lingering
    ;; overlay-map which could otherwise render Emacs unusable.
    (set overlaysym
         (run-with-idle-timer
          0 t
          `(lambda ()
             (if (memq ',clearfunsym
                       (default-value 'pre-command-hook))
                 nil
               (message "clear-temporary-overlay-map lost in pre-command-hook!")
               (,clearfunsym)))))
    ;; FIXME: That's the keymaps with highest precedence, except for
    ;; the `keymap' text-property ;-(
    (push alist emulation-mode-map-alists)))

Re: bug#7234: 24.0.50; strange message after text-scale-adjust

[Kenichi Handa]

In article <jwv62wzy4nf.fsf-monnier+emacs@gnu.org>, Stefan Monnier <monnier@iro.umontreal.ca> writes:

> > It would be better that read-event call in
> > text-scale-adjust has some prompt:
> > e.g. (read-event "+,-,0 for further adjustment: ")

> Agreed.

Shall I install that change, or do you have a better prompt
string, or should we wait for your alternative
implementation?

> > For non-interactive use, you should probably also be using
> > `text-scale-set' or `text-scale-increase' instead.

> BTW, I've been playing around with an alternative implementation for
> such things.  The reason is that the use of
> read-event/read-char/read-key has surprising side-effects: in those
> cases I'm concerned with, the user expects that the command is already
> finished and the behavior is somewhat consistent with that expectation,
> but not completely: indeed typing "any" key sequence (except for a few
> special ones) runs the usual corresponding command, but OTOH
> post-command-hook and friends aren't run when expected.

I think the situation is similar to isearch.  How does
isearch solve it?

> So, instead I use a new function set-temporary-overlay-map which sets up
> a keymap that's only active for a short period of time (by default just
> for the next key-sequence):

>   ;; ----------------------------------------------------------------
> --- 294,317 ----
>   `text-scale-decrease' may be more appropriate."
>     (interactive "p")
>     (let ((first t)
>   	(ev last-command-event)
>   	(echo-keystrokes nil))
> !     (let* ((base (event-basic-type ev))
> !            (step
> !             (case base
> !               ((?+ ?=) inc)
> !               (?- (- inc))
> !               (?0 0)
> !               (t inc))))
>   	(text-scale-increase step)
> !       (set-temporary-overlay-map
> !        (let ((map (make-sparse-keymap)))
> !          (define-key map [?=] 'text-scale-increase)
> !          (define-key map [?0] (lambda () (interactive) (text-scale-increase 0)))
> !          (define-key map [?+] 'text-scale-increase)
> !          (define-key map [?-] 'text-scale-decrease)
> !          map)
> !        t))))
  
With that, how to show the prompt?

---
Kenichi Handa
handa@m17n.org

Re: bug#7234: 24.0.50; strange message after text-scale-adjust

[Stefan Monnier]

>> > It would be better that read-event call in
>> > text-scale-adjust has some prompt:
>> > e.g. (read-event "+,-,0 for further adjustment: ")
>> Agreed.
> Shall I install that change, or do you have a better prompt
> string, or should we wait for your alternative
> implementation?

Go ahead with your change.  It's not directly related to my alternative
implementation anyway and that implementation is still work in progress.

>> > For non-interactive use, you should probably also be using
>> > `text-scale-set' or `text-scale-increase' instead.

>> BTW, I've been playing around with an alternative implementation for
>> such things.  The reason is that the use of
>> read-event/read-char/read-key has surprising side-effects: in those
>> cases I'm concerned with, the user expects that the command is already
>> finished and the behavior is somewhat consistent with that expectation,
>> but not completely: indeed typing "any" key sequence (except for a few
>> special ones) runs the usual corresponding command, but OTOH
>> post-command-hook and friends aren't run when expected.

> I think the situation is similar to isearch.  How does
> isearch solve it?

isearch uses an overlay map (overriding-terminal-local-map) rather than
read-key/event/char.

> With that, how to show the prompt?

You'd do it with `message'.


        Stefan

Re: bug#7234: 24.0.50; strange message after text-scale-adjust

[Kenichi Handa]

In article <jwviq0ym9ph.fsf-monnier+emacs@gnu.org>, Stefan Monnier <monnier@IRO.UMontreal.CA> writes:
> > Shall I install that change, or do you have a better prompt
> > string, or should we wait for your alternative
> > implementation?

> Go ahead with your change.

Ok, done.

> > I think the situation is similar to isearch.  How does
> > isearch solve it?

> isearch uses an overlay map (overriding-terminal-local-map) rather than
> read-key/event/char.

Then, what's the pros and cons of using an overlay map and
your set-temporary-overlay-map?

> > With that, how to show the prompt?

> You'd do it with `message'.

Doesn't the message disappear when a command in the
temporary overlay-map is executed?

---
Kenichi Handa
handa@m17n.org

Re: bug#7234: 24.0.50; strange message after text-scale-adjust

[Stefan Monnier]

>> > Shall I install that change, or do you have a better prompt
>> > string, or should we wait for your alternative
>> > implementation?
>> Go ahead with your change.
> Ok, done.

>> > I think the situation is similar to isearch.  How does
>> > isearch solve it?
>> isearch uses an overlay map (overriding-terminal-local-map) rather than
>> read-key/event/char.
> Then, what's the pros and cons of using an overlay map and
> your set-temporary-overlay-map?

I think isearch would like to use set-temporary-overlay-map.
To a large extent they are very similar, but using
overriding-terminal-local-map means that the non-isearch bindings
(which cause isearch to exit) are not directly available, so isearch as
to catch them with a default binding (define-key map [t] ...), then
exit and push the events on unread-command-events for re-execution.

And unread-command-events is evil because it's hellishly difficult to
make it behave 100% correctly in all cases where function-key-map,
input-decode-map, key-translation-map, keyboard-translate-table,
input-methods, etc... are involved.

But set-temporary-overlay-map didn't exist back then (it barely exists
now) ... and it's far from obvious that someone will find the motivation
to change isearch to use something like set-temporary-overlay-map.

>> > With that, how to show the prompt?
>> You'd do it with `message'.
> Doesn't the message disappear when a command in the
> temporary overlay-map is executed?

Yes.  So you only get it at the beginning of the lifetime of the
temporary-overlay-map, or you have to do extra work to also display it
later on (e.g. with a post-command-hook: maybe
set-temporary-overlay-map should provide such a feature itself, so it's
encapsulated).

As I said, set-temporary-overlay-map barely exists now and while it
solves some problems, it comes with a new set of problems.  I know its
good sides (the ones that pushed me to try and play with it) but I'm
only slowly learning the downsides.


        Stefan

Re: bug#7234: 24.0.50; strange message after text-scale-adjust

[Kenichi Handa]

In article <jwvpqv4g8r6.fsf-monnier+emacs@gnu.org>, Stefan Monnier <monnier@iro.umontreal.ca> writes:

> > Then, what's the pros and cons of using an overlay map and
> > your set-temporary-overlay-map?

> I think isearch would like to use set-temporary-overlay-map.
> To a large extent they are very similar, but using
> overriding-terminal-local-map means that the non-isearch bindings
> (which cause isearch to exit) are not directly available, so isearch as
> to catch them with a default binding (define-key map [t] ...), then
> exit and push the events on unread-command-events for re-execution.

> And unread-command-events is evil because it's hellishly difficult to
> make it behave 100% correctly in all cases where function-key-map,
> input-decode-map, key-translation-map, keyboard-translate-table,
> input-methods, etc... are involved.

I see.  Thank you for the explanation.

>>> > With that, how to show the prompt?
>>> You'd do it with `message'.
> > Doesn't the message disappear when a command in the
> > temporary overlay-map is executed?

> Yes.  So you only get it at the beginning of the lifetime of the
> temporary-overlay-map, or you have to do extra work to also display it
> later on (e.g. with a post-command-hook:

Ummm, a little bit ugly.

> maybe set-temporary-overlay-map should provide such a
> feature itself, so it's encapsulated).

Yes, that is better.

> As I said, set-temporary-overlay-map barely exists now and while it
> solves some problems, it comes with a new set of problems.  I know its
> good sides (the ones that pushed me to try and play with it) but I'm
> only slowly learning the downsides.

I understand it.

---
Kenichi Handa
handa@m17n.org