bug#14676: 24.3.50; winner doesn't immediately notice windows created by server

bug#14676: 24.3.50; winner doesn't immediately notice windows created by server

[Michael Heerdegen]

Hello,

I have `winner-mode' turned on.

From e.g. an image viewer I want to open a dired buffer for the
directory containing the images.  So, from the image viewer, I run

  emacsclient file

and in Emacs, I immediately hit C-^ (which I bound to `dired-jump') in
the popped up window, to get the parent dir.

The problem: the buffer showing the file was not recorded in the winner
history - `winner-undo' can't get it.

Note: this only happens if I hit C-^ immediately after opening the
file.  Else, you trigger `post-command-hook' which DTRT.

Evaluating

  (add-hook 'server-switch-hook 'winner-save-old-configurations)

fixes the problem for me.  Should we do something like that in the
definition of `winner-mode'?


Regards,

Michael.



In GNU Emacs 24.3.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.8.2)
 of 2013-06-18 on dex, modified by Debian
 (emacs-snapshot package, version 2:20130618-1)
Windowing system distributor `The X.Org Foundation', version 11.0.11204000
System Description:	Debian GNU/Linux testing (jessie)

Configured using:
 `configure --build x86_64-linux-gnu --host x86_64-linux-gnu
 --prefix=/usr --sharedstatedir=/var/lib --libexecdir=/usr/lib
 --localstatedir=/var --infodir=/usr/share/info --mandir=/usr/share/man
 --with-pop=yes
 --enable-locallisppath=/etc/emacs-snapshot:/etc/emacs:/usr/local/share/emacs/24.3.50/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/24.3.50/site-lisp:/usr/share/emacs/site-lisp
 --without-compress-info --with-crt-dir=/usr/lib/x86_64-linux-gnu/
 --with-x=yes --with-x-toolkit=gtk3 --with-imagemagick=yes
 CFLAGS='-DDEBIAN -DSITELOAD_PURESIZE_EXTRA=5000 -g -O2'
 CPPFLAGS='-D_FORTIFY_SOURCE=2' LDFLAGS='-g -Wl,--as-needed
 -znocombreloc''

bug#14676: 24.3.50; winner doesn't immediately notice windows created by server

[Stefan Monnier]

>   (add-hook 'server-switch-hook 'winner-save-old-configurations)
> fixes the problem for me.  Should we do something like that in the
> definition of `winner-mode'?

I don't much like this solution.


        Stefan

bug#14676: 24.3.50; winner doesn't immediately notice windows created by server

[Stefan Monnier]

>> (add-hook 'server-switch-hook 'winner-save-old-configurations)
>> fixes the problem for me.  Should we do something like that in the
>> definition of `winner-mode'?
> I don't much like this solution.

I think the better solution is to move the (run-hooks
'post-command-hook) in server.el.  Currently it's run at the end of
server-visit-files which I think is simply too early.  It should be run
at the end of server-execute instead.  And that should fix your problem
as well since winner-mode hooks itself on post-command-hook.


        Stefan

bug#14676: 24.3.50; winner doesn't immediately notice windows created by server

[Michael Heerdegen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> >> (add-hook 'server-switch-hook 'winner-save-old-configurations)
> >> fixes the problem for me.  Should we do something like that in the
> >> definition of `winner-mode'?
> > I don't much like this solution.
>
> I think the better solution is to move the (run-hooks
> 'post-command-hook) in server.el.  Currently it's run at the end of
> server-visit-files which I think is simply too early.  It should be run
> at the end of server-execute instead.  And that should fix your problem
> as well since winner-mode hooks itself on post-command-hook.

That sounds reasonable.  I tested your approach quickly, and it worked.

Michael.

bug#14676: 24.3.50; winner doesn't immediately notice windows created by server

[Stefan Monnier]

> That sounds reasonable.  I tested your approach quickly, and it worked.

Can you show me the patch you tested?


        Stefan

bug#14676: 24.3.50; winner doesn't immediately notice windows created by server

[Michael Heerdegen]

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

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Can you show me the patch you tested?

I tested this:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: server.patch --]
[-- Type: text/x-diff, Size: 1689 bytes --]

*** /home/micha/today/server.el	2013-06-23 00:21:58.737341216 +0200
--- /home/micha/today/server-new.el	2013-06-23 00:27:31.470493560 +0200
***************
*** 1292,1298 ****
                (message "%s" (substitute-command-keys
                               "When done with a buffer, type \\[server-edit]")))))
            (when (and frame (null tty-name))
!             (server-unselect-display frame)))
        ((quit error)
         (when (eq (car err) 'quit)
           (message "Quit emacsclient request"))
--- 1292,1299 ----
                (message "%s" (substitute-command-keys
                               "When done with a buffer, type \\[server-edit]")))))
            (when (and frame (null tty-name))
!             (server-unselect-display frame))
! 	  (run-hooks 'post-command-hook))
        ((quit error)
         (when (eq (car err) 'quit)
           (message "Quit emacsclient request"))
***************
*** 1360,1368 ****
              (unless server-buffer-clients
                (setq server-existing-buffer t)))
            (server-goto-line-column (cdr file))
!           (run-hooks 'server-visit-hook)
! 	  ;; hooks may be specific to current buffer:
! 	  (run-hooks 'post-command-hook))
  	(unless nowait
  	  ;; When the buffer is killed, inform the clients.
  	  (add-hook 'kill-buffer-hook 'server-kill-buffer nil t)
--- 1361,1367 ----
              (unless server-buffer-clients
                (setq server-existing-buffer t)))
            (server-goto-line-column (cdr file))
!           (run-hooks 'server-visit-hook))
  	(unless nowait
  	  ;; When the buffer is killed, inform the clients.
  	  (add-hook 'kill-buffer-hook 'server-kill-buffer nil t)

[-- Attachment #3: Type: text/plain, Size: 280 bytes --]


Note that I don't know anything about server.el, I just did what you
proposed.

Anyway, it fixes the problem - with my setup as well as for -Q.

BTW, I didn't know what was meant with this comment:

| ;; hooks may be specific to current buffer:

so I just removed it.


Michael.

bug#14676: 24.3.50; winner doesn't immediately notice windows created by server

[Lars Ingebrigtsen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

>> Can you show me the patch you tested?
>
> I tested this:

This was the final message in this seven year old thread.  Miraculously,
the patch still applies:

>                 (message "%s" (substitute-command-keys
>                                "When done with a buffer, type \\[server-edit]")))))
>             (when (and frame (null tty-name))
> !             (server-unselect-display frame)))
>         ((quit error)
>          (when (eq (car err) 'quit)
>            (message "Quit emacsclient request"))
> --- 1292,1299 ----
>                 (message "%s" (substitute-command-keys
>                                "When done with a buffer, type \\[server-edit]")))))
>             (when (and frame (null tty-name))
> !             (server-unselect-display frame))
> ! 	  (run-hooks 'post-command-hook))
>         ((quit error)
>          (when (eq (car err) 'quit)
>            (message "Quit emacsclient request"))

[...]

> Note that I don't know anything about server.el, I just did what you
> proposed.
>
> Anyway, it fixes the problem - with my setup as well as for -Q.

I'm not very familiar with server.el myself...   this just changes it to
run post-command-hook slightly later?  I guess that could have
unforeseen consequences, but if it fixes this bug, then perhaps we
should give it a go?

Or did the problem fix itself through other means over the years?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#14676: 24.3.50; winner doesn't immediately notice windows created by server

[Michael Heerdegen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Or did the problem fix itself through other means over the years?

No, it didn't.

The "patch" is just the change Stefan suggested, done in a more or less
random way AFAIR, since I don't know server.el, too.  I would still give
it a try.  But AFAIU the change I had posted and you quoted doesn't
fulfill what the current code says, namely that `post-command-hook'
should be run in each buffer individually.  Maybe that's the reason why
Stefan had ignored it ;-)

Michael.

bug#14676: 24.3.50; winner doesn't immediately notice windows created by server

[Stefan Monnier]

>> Or did the problem fix itself through other means over the years?
> No, it didn't.
> The "patch" is just the change Stefan suggested, done in a more or less
> random way AFAIR, since I don't know server.el, too.  I would still give
> it a try.  But AFAIU the change I had posted and you quoted doesn't
> fulfill what the current code says, namely that `post-command-hook'
> should be run in each buffer individually.  Maybe that's the reason why
> Stefan had ignored it ;-)

BTW, runnning `post-command-hook` in there is arguably wrong (we didn't
technically run a command, or at least we didn't go through the usual
"protocol" of running a command, so some `post-command-hook` functions
may misbehave, e.g. because of interaction with things like
`this-command` or `last-command`).

Maybe the right way to fix this is to try and make `server.el` simulate
more carefully a command-loop running a command.  Or even *really* make
it run a command.  E.g. instead of having `server.el` immediately
run within the process filter the code requested by the client, it could
just push some ad-hoc event onto `unread-command-events` which is then
bound to a command.


        Stefan