bug#11983: 24.1; Electric-command-loop broken?

bug#11983: 24.1; Electric-command-loop broken?

[Roland Winkler]

I am trying to understand Electric-command-loop in electric.el
(this is used by BBDB 3):

Two things:

- The code contains a hard-coded

    (setq universal-argument-num-events 0)

  Apparently this is never reset, so exiting Electric-command-loop
  leaves behind this binding.

- The doc string says

  ;; Given third argument non-nil, it
  ;; INHIBITS quitting unless the user types C-g at toplevel.  This is
  ;; so user can do things like C-u C-g and not get thrown out.

  Yet it appears to me, that even for C-u C-g the user gets thrown out.
  Here is a slightly simplified version of the code from Electric-command-loop
  It does not distinguish between C-g and C-u C-g.
  Unfortunately, this hackery goes beyond my understanding of Emacs
  internals.

(catch 'return-tag
  (let (cmd (inhibit-quit t))
    (while t
      (setq cmd (read-key-sequence "Prompt: "))
      (setq last-command-event (aref cmd (1- (length cmd)))
            this-command (key-binding cmd t)
            cmd this-command)
      ;; This makes universal-argument-other-key work.
      (setq universal-argument-num-events 0)
      (if (or (prog1 quit-flag (setq quit-flag nil))
              (eq last-input-event ?\C-g))
          (progn (setq unread-command-events nil
                       prefix-arg nil)
                 ;; If it wasn't canceling a prefix character, then quit.
                 (if (or (= (length (this-command-keys)) 1)
                         (not inhibit-quit)) ; safety
                     (throw 'return-tag (this-command-keys))
                   (setq cmd nil))))
      (setq current-prefix-arg prefix-arg)
      (condition-case nil
          (progn (command-execute cmd)
                 (setq last-command this-command))
        (error nil)))))



In GNU Emacs 24.1.1 (x86_64-unknown-linux-gnu, GTK+ Version 2.20.1)
 of 2012-06-10 on regnitz
Windowing system distributor `The X.Org Foundation', version 11.0.10706000

bug#11983: 24.1; Electric-command-loop broken?

[Stefan Monnier]

> I am trying to understand Electric-command-loop in electric.el

I do not understand what it's trying to do, let alone how it's trying to
do it.
The best I could understand of what it's trying to do is summarized in
the commentary I added:

;; - electric modes and buffers: modes that typically pop-up in a modal kind of
;;   way a transient buffer that automatically disappears as soon as the user
;;   is done with it.

> (this is used by BBDB 3):

Could you maybe then describe the expected behavior, from the user's
point of view?  Adding docstrings, and/or improving comments would be
very welcome.

> - The code contains a hard-coded
>     (setq universal-argument-num-events 0)
>   Apparently this is never reset, so exiting Electric-command-loop
>   leaves behind this binding.

It's a global var used by `universal-argument'.
`universal-argument' is implemented in a rather intricate way, part of
which is actually hidden deep in the C code.

universal-argument-num-events is used for when you finish a C-u sequence
to find which keys are part of the universal argument and which keys are
part of the actual command you wan to run.

`universal-argument' should really use something like
set-temporary-overlay-map instead so it wouldn't need
universal-argument-other-key and neither would it have to fiddle with
unread-command-events.

> - The doc string says
>   ;; Given third argument non-nil, it
>   ;; INHIBITS quitting unless the user types C-g at toplevel.  This is
>   ;; so user can do things like C-u C-g and not get thrown out.
>   Yet it appears to me, that even for C-u C-g the user gets thrown out.

I have no idea what this "C-u C-g" refers to, indeed.


        Stefan

bug#11983: 24.1; Electric-command-loop broken?

[Roland Winkler]

On Thu Jul 19 2012 Stefan Monnier wrote:
> The best I could understand of what it's trying to do is summarized in
> the commentary I added:
>
> ;; - electric modes and buffers: modes that typically pop-up in a
> ;;   modal kind of way a transient buffer that automatically
> ;;   disappears as soon as the user is done with it.
>
> > (this is used by BBDB 3):
>
> Could you maybe then describe the expected behavior, from the user's
> point of view?  Adding docstrings, and/or improving comments would be
> very welcome.

My understanding of this is the following (better suggestions
welcome):

Imagine you have an unwind-protect form. It allows you to execute
whatever noninteractive code, and you can be sure that in the end
it executes cleanup-form. Now imagine you want to do the same
thing interactively: you want to run some commands interactively,
and in the end you want to be sure you execute cleanup-form, no
matter what happens. The code for this is

(unwind-protect
    (catch 'return-tag
      (Electric-command-loop 'return-tag))
  (cleanup-form))

Electric-command-loop throws 'return-tag if you type
C-g (keyboard-quit). Or you can make commands electric by letting
them throw 'return-tag. But any other command is executed inside
the temporary command loop implemented by Electric-command-loop
without ever leaving this loop. In particular, this temporary
command loop catches all errors that do not throw 'return-tag.

So in short: Electric-command-loop allows one to execute commands
in an `unwind-protect'ed temporary command loop.

Maybe there is a simpler way to implement such a functionality.

A typical application is suggested by the function
Electric-pop-up-window which pops up a window fairly aggresively
(see bug#11985): You quickly execute some command in the electric
window. When you are done, this code guarantees that cleanup-form
gets executed.

> > - The doc string says
> >   ;; Given third argument non-nil, it
> >   ;; INHIBITS quitting unless the user types C-g at toplevel.  This is
> >   ;; so user can do things like C-u C-g and not get thrown out.
> >   Yet it appears to me, that even for C-u C-g the user gets thrown out.
>
> I have no idea what this "C-u C-g" refers to, indeed.

If you type a plain C-g, Electric-command-loop throws
'return-tag.  Now the idea is that if you type C-u, then you
change your mind and want to cancel it by typing C-g, then the
code should not leave the temporary command loop.

bug#11983: 24.1; Electric-command-loop broken?

[Stefan Monnier]

> (unwind-protect
>     (catch 'return-tag
>       (Electric-command-loop 'return-tag))
>   (cleanup-form))

But in which way is this different from `recursive-edit'?
Hmmm... I guess it is different in that it is easier to exit an
Electric-command-loop than a recursive-edit, so it is like
a "transient/lightweight" recursive-edit.
I think it would be good to try to describe it by comparing it to
recursive-edit.

It seems that it requires a fair bit of extra surrounding code to use it
right.  E.g. electric-buffer-list is buggy because it lacks this extra
code: after popping up the electric-buffer-list, you can select some
other window and work there, but the behavior is then all
messed up.

>> > - The doc string says
>> >   ;; Given third argument non-nil, it
>> >   ;; INHIBITS quitting unless the user types C-g at toplevel.  This is
>> >   ;; so user can do things like C-u C-g and not get thrown out.
>> >   Yet it appears to me, that even for C-u C-g the user gets thrown out.
>> 
>> I have no idea what this "C-u C-g" refers to, indeed.

> If you type a plain C-g, Electric-command-loop throws
> 'return-tag.  Now the idea is that if you type C-u, then you
> change your mind and want to cancel it by typing C-g, then the
> code should not leave the temporary command loop.

I see, yes.  This inhibit-quitting seems dangerous since it binds
inhibit-quit.  But yes, I see in the code it tries to handle C-g after
some prefix key.  Not sure why it now fails or whether it ever worked.


        Stefan

bug#11983: 24.1; Electric-command-loop broken?

[Roland Winkler]

On Fri Jul 20 2012 Stefan Monnier wrote:
> > (unwind-protect
> >     (catch 'return-tag
> >       (Electric-command-loop 'return-tag))
> >   (cleanup-form))
> 
> But in which way is this different from `recursive-edit'?

I do not know much about recursive-edit. How would you use it as a
replacement for the above to be sure that after leaving
recursive-edit cleanup-form is always executed?

> It seems that it requires a fair bit of extra surrounding code to
> use it right. E.g. electric-buffer-list is buggy because it lacks
> this extra code: after popping up the electric-buffer-list, you
> can select some other window and work there, but the behavior is
> then all messed up.

The amount of protection provided by an Electric-command-loop
depends on the surrounding code (save-excursion,
save-window-excursion, etc.) I believe that the intended usage
pattern of Electric-command-loop does not include too wild things
such as selecting other windows. Or phrased differently: of course,
you can always do whatever you like. But only cleanup-form is
definitely evaluated at the end.

Roland

bug#11983: 24.1; Electric-command-loop broken?

[Stefan Monnier]

>> > (unwind-protect
>> >     (catch 'return-tag
>> >       (Electric-command-loop 'return-tag))
>> >   (cleanup-form))
>> But in which way is this different from `recursive-edit'?
> I do not know much about recursive-edit. How would you use it as a
> replacement for the above to be sure that after leaving
> recursive-edit cleanup-form is always executed?

    (unwind-protect
        (recursive-edit)
      (cleanup-form))

>> It seems that it requires a fair bit of extra surrounding code to
>> use it right. E.g. electric-buffer-list is buggy because it lacks
>> this extra code: after popping up the electric-buffer-list, you
>> can select some other window and work there, but the behavior is
>> then all messed up.
> The amount of protection provided by an Electric-command-loop
> depends on the surrounding code (save-excursion,
> save-window-excursion, etc.)

The issue is not that the buffer is not reset when you return from
electric-buffer-list, but that during electric-buffer-list if you select
some other window you do not exit electric-buffer-list and the keys end
up behaving weird (e.g. typing "c" in a normal buffer will insert "c"
and then move to EOB or something like that).  Once you exit from
electric-buffer-list, things are back to normal.

> I believe that the intended usage pattern of Electric-command-loop
> does not include too wild things such as selecting other windows.  Or
> phrased differently: of course, you can always do whatever you
> like.  But only cleanup-form is definitely evaluated at the end.

What I'm saying is that it's tricky to use Electric-command-loop without
introducing bugs because Electric-command-loop presumes that all
operations will stay within the current buffer, but it does not (help
to) try to enforce it.  So it's a poor API.


        Stefan

bug#11983: 24.1; Electric-command-loop broken?

[Roland Winkler]

On Fri Jul 20 2012 Stefan Monnier wrote:
> > I do not know much about recursive-edit. How would you use it as a
> > replacement for the above to be sure that after leaving
> > recursive-edit cleanup-form is always executed?
> 
>     (unwind-protect
>         (recursive-edit)
>       (cleanup-form))

Ah, thanks, that's indeed not more complicated either.

> The issue is not that the buffer is not reset when you return from
> electric-buffer-list, but that during electric-buffer-list if you select
> some other window you do not exit electric-buffer-list and the keys end
> up behaving weird (e.g. typing "c" in a normal buffer will insert "c"
> and then move to EOB or something like that).  Once you exit from
> electric-buffer-list, things are back to normal.

In BBDB this issue has been address by making relevant commands
electric: any command that is intended to quit the electric command
loop (in the context of electric-buffer-list this could mean: you
select another buffer to work with) will throw 'return-tag. Then you
can continue normally.

Of course, this means: the code needs to provide electric versions
of all relevant commands. If the user still decides to do something
else, the result is undefined.

> What I'm saying is that it's tricky to use Electric-command-loop without
> introducing bugs because Electric-command-loop presumes that all
> operations will stay within the current buffer, but it does not (help
> to) try to enforce it.  So it's a poor API.

I do not disagree here. I got into all this business because the
Electric-command-loop has been present in old versions of BBDB. But
I would not miss it, if it disappeared. (I do not even know whether
any other BBDB user would miss it. I'll ask on the BBDB mailing
list.)

Roland