Re: master 9cef8fc: Ensure that new emacsclient frames has focus

Re: master 9cef8fc: Ensure that new emacsclient frames has focus

[Joseph Mingrone]

On Mon, 2020-12-07 at 08:29, larsi@gnus.org (Lars Ingebrigtsen) wrote:

> branch: master
> commit 9cef8fc8cdb5e6e18c9cf617eed3808d67ca340e
> Author: Lars Ingebrigtsen <larsi@gnus.org>
> Commit: Lars Ingebrigtsen <larsi@gnus.org>

>     Ensure that new emacsclient frames has focus

>     * lisp/server.el (server-execute): Focus the frame here...
>     (server-switch-buffer): Instead of here (bug#15469).  This ensures
>     that the frame has focus if Emacs is querying the user about
>     something when opening a file (for instance "Revert from file?").
> ---
>  lisp/server.el | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

> diff --git a/lisp/server.el b/lisp/server.el
> index b1b27e3..651f801 100644
> --- a/lisp/server.el
> +++ b/lisp/server.el
> @@ -1334,6 +1334,8 @@ The following commands are accepted by the client:
>    ;; inhibit-quit flag, which is good since `commands' (as well as
>    ;; find-file-noselect via the major-mode) can run arbitrary code,
>    ;; including code that needs to wait.
> +  (when (and frame server-raise-frame)
> +    (select-frame-set-input-focus frame))
>    (with-local-quit
>      (condition-case err
>          (let ((buffers (server-visit-files files proc nowait)))
> @@ -1686,9 +1688,7 @@ be a cons cell (LINENUMBER . COLUMNNUMBER)."
>                    (switch-to-buffer next-buffer))
>  	      ;; After all the above, we might still have ended up with
>  	      ;; a minibuffer/dedicated-window (if there's no other).
> -	      (error (pop-to-buffer next-buffer)))))))
> -    (when server-raise-frame
> -      (select-frame-set-input-focus (window-frame)))))
> +	      (error (pop-to-buffer next-buffer)))))))))

>  ;;;###autoload
>  (defun server-save-buffers-kill-terminal (arg)

Hi,

I'm seeing some new behaviour with this commit that I suspect was
unintended.

Before this commit, calling emacsclient -n <file> from a terminal
emulator caused the emacs frame to be raised.

After this commit, the terminal emulator normally stays in focus.  I
haven't figured out for certain when the emacs frame still gains focus,
but it seems to happen when <file> does not exist on disk.

Joe

Re: master 9cef8fc: Ensure that new emacsclient frames has focus

[Lars Ingebrigtsen]

Joseph Mingrone <jrm@ftfl.ca> writes:

>>     Ensure that new emacsclient frames has focus

[...]

> I'm seeing some new behaviour with this commit that I suspect was
> unintended.
>
> Before this commit, calling emacsclient -n <file> from a terminal
> emulator caused the emacs frame to be raised.
>
> After this commit, the terminal emulator normally stays in focus.  I
> haven't figured out for certain when the emacs frame still gains focus,
> but it seems to happen when <file> does not exist on disk.

I was afraid of something like that -- the code in this area has gotten
a lot of tweaks over the years because the actions are a bit convoluted.

I'm seeing the same behaviour with -n with both existing and
non-existing files, though.  (That is, that the Emacs frame doesn't get
focus.)  It'd be nice if you could figure out when the frame still gets
focus, because that sounds like another wrinkle...

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

Re: master 9cef8fc: Ensure that new emacsclient frames has focus

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> I'm seeing the same behaviour with -n with both existing and
> non-existing files, though.  (That is, that the Emacs frame doesn't get
> focus.)  It'd be nice if you could figure out when the frame still gets
> focus, because that sounds like another wrinkle...

I've now pushed a change that fixes the -n problem, I think, which
hopefully doesn't regress anything else.  Unfortunately we don't have a
test suite for this stuff, I think, but I hope this doesn't break
anything else.

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

Re: master 9cef8fc: Ensure that new emacsclient frames has focus

[Joseph Mingrone]

On Fri, 2020-12-18 at 10:26, Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Lars Ingebrigtsen <larsi@gnus.org> writes:

>> I'm seeing the same behaviour with -n with both existing and
>> non-existing files, though.  (That is, that the Emacs frame doesn't get
>> focus.)  It'd be nice if you could figure out when the frame still gets
>> focus, because that sounds like another wrinkle...

> I've now pushed a change that fixes the -n problem, I think, which
> hopefully doesn't regress anything else.  Unfortunately we don't have a
> test suite for this stuff, I think, but I hope this doesn't break
> anything else.

I tested your change and it looks good here.

In your other message (and before the fix) you asked about figuring out
when the frame still got focus.  I know that when I visited a few files
that weren't written on disk, the frame still got focus.

I was testing with this alias: emacsclient -a= -n

If there are still issues, I could revert back and try more tests.

Thank you,

Joe

Re: master 9cef8fc: Ensure that new emacsclient frames has focus

[Lars Ingebrigtsen]

Joseph Mingrone <jrm@ftfl.ca> writes:

> I tested your change and it looks good here.

Thanks for testing.

> In your other message (and before the fix) you asked about figuring out
> when the frame still got focus.  I know that when I visited a few files
> that weren't written on disk, the frame still got focus.
>
> I was testing with this alias: emacsclient -a= -n
>
> If there are still issues, I could revert back and try more tests.

It would perhaps be nice to find out why I wasn't seeing exactly the
same thing that you were...  but I think the tweak I pushed should fix
the problem in both cases, so I don't think reverting back and
rechecking is really necessary.

These kinds of problems really make me wish that we had a test suite
that had a standard headless X VM, or something, because it's so easy to
introduce these types of regressions without.

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

Re: master 9cef8fc: Ensure that new emacsclient frames has focus

[Andreas Schwab]

On Dez 19 2020, Lars Ingebrigtsen wrote:

> These kinds of problems really make me wish that we had a test suite
> that had a standard headless X VM, or something,

That should be easy to do, by using xvfb-run as a wrapper.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: master 9cef8fc: Ensure that new emacsclient frames has focus

[Lars Ingebrigtsen]

Andreas Schwab <schwab@linux-m68k.org> writes:

> On Dez 19 2020, Lars Ingebrigtsen wrote:
>
>> These kinds of problems really make me wish that we had a test suite
>> that had a standard headless X VM, or something,
>
> That should be easy to do, by using xvfb-run as a wrapper.

True, and it would allow us to run tests for stuff like this.  A VM
(with a pre-defined set of installed fonts and the like) would allow us
to run a more ambitious set of tests, though, for font
selection/layout/scrolling/etc.

Or Docker instead of a VM, I guess...

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

Re: master 9cef8fc: Ensure that new emacsclient frames has focus

[Joseph Mingrone]

On Sat, 2020-12-19 at 16:46, Lars Ingebrigtsen <larsi@gnus.org> wrote:

> but I think the tweak I pushed should fix the problem in both cases,
> so I don't think reverting back and rechecking is really necessary.

Hi Lars,

I notice another change in behaviour.  If you try something like this
from a terminal (outside of emacs), the emacs frame will steal focus.

% emacsclient -e '(message "Hello World!")'

Joe

Re: master 9cef8fc: Ensure that new emacsclient frames has focus

[Lars Ingebrigtsen]

Joseph Mingrone <jrm@ftfl.ca> writes:

> On Sat, 2020-12-19 at 16:46, Lars Ingebrigtsen <larsi@gnus.org> wrote:
>
>> but I think the tweak I pushed should fix the problem in both cases,
>> so I don't think reverting back and rechecking is really necessary.
>
> Hi Lars,
>
> I notice another change in behaviour.  If you try something like this
> from a terminal (outside of emacs), the emacs frame will steal focus.
>
> % emacsclient -e '(message "Hello World!")'

OK; I think it's time to perhaps revert the change and try to fix the
originally reported problem in a different way...

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

Re: master 9cef8fc: Ensure that new emacsclient frames has focus

[T.V Raman]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=gb18030, Size: 624 bytes --]

Joseph Mingrone <jrm@ftfl.ca> writes:

yes, got bitten by this too -- could we not steal focus for --eval etc?> On Sat, 2020-12-19 at 16:46, Lars Ingebrigtsen <larsi@gnus.org> wrote:
>
>> but I think the tweak I pushed should fix the problem in both cases,
>> so I don't think reverting back and rechecking is really necessary.
>
> Hi Lars,
>
> I notice another change in behaviour.  If you try something like this
> from a terminal (outside of emacs), the emacs frame will steal focus.
>
> % emacsclient -e '(message "Hello World!")'
>
> Joe
>

-- 

Thanks,

--Raman
7©4 Id: kg:/m/0285kf1  •0Ü8

Re: master 9cef8fc: Ensure that new emacsclient frames has focus

[Lars Ingebrigtsen]

Joseph Mingrone <jrm@ftfl.ca> writes:

> I notice another change in behaviour.  If you try something like this
> from a terminal (outside of emacs), the emacs frame will steal focus.

I've now reverted the frame-focus changes.

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