unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#25547: 25.1.91; emacsclient -c creates frames on the wrong display
@ 2017-01-26 18:09 Alex Hutcheson
  2020-11-02 22:37 ` Stefan Monnier
  0 siblings, 1 reply; 13+ messages in thread
From: Alex Hutcheson @ 2017-01-26 18:09 UTC (permalink / raw)
  To: 25547

When an emacsclient running in a terminal is connected to an Emacs
instance running in a graphical environment, running `emacsclient -c'
from a comint buffer (such as M-x shell) creates a frame in the
graphical environment, where it may be inaccessible to the user who
ran the command.

To reproduce:
1. Run `emacs --daemon' on a system with a graphical display.
2. Login to the system using a terminal with no graphical display. You
    could do this by switching to another virtual terminal (Ctrl-Alt-F1
    works on Ubuntu), or SSH in from another machine.
3. Run `emacsclient -c' or `emacsclient -t' in the terminal to connect
    to the running Emacs instance.
4. Inside Emacs, run M-x shell.
5. In the M-x shell buffer, run `emacsclient -c'.

Ideal behavior: emacsclient creates a frame on the current terminal.
Actual behavior: emacsclient creates a graphical frame in the
   environment in which it was launched, which is inaccessible from the
   terminal.

Possible questions:

Q : Why would you want to run emacsclient from within M-x shell? Why
     not just do whatever you want to do in the frame you already have
     open?
A : Normally the user isn't launching emacsclient directly. Instead,
     a script might be launching emacsclient as part of a larger
     workflow.

Q : Why not just launch emacsclient without the '-c' option?
A : Without the -c flag, emacsclient works well to edit a file or
     series of files, but it doesn't support more complicated tasks.
     For example, a script might want to launch an ediff session, and
     wait until the ediff session exits. The easiest way to do this is
     to use `emacsclient -c' and wait for the connection to be closed.


In GNU Emacs 25.1.91.1 (x86_64-pc-linux-gnu, X toolkit, Xaw3d scroll bars),  
modified by Debian
Windowing system distributor 'The X.Org Foundation', version 11.0.11501000
System Description:	Ubuntu 14.04 LTS

Configured using:
  'configure --build x86_64-linux-gnu --build x86_64-linux-gnu
  --prefix=/usr --sharedstatedir=/var/lib --libexecdir=/usr/lib
  --localstatedir=/var/lib --infodir=/usr/share/info
  --mandir=/usr/share/man --with-pop=yes
   
--enable-locallisppath=/etc/google-emacs:/etc/emacs:/usr/local/share/emacs/25.1.91+gg1+2/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/25.1.91+gg1+2/site-lisp:/usr/share/emacs/site-lisp
  --with-crt-dir=/usr/lib/x86_64-linux-gnu --disable-build-details
  --disable-silent-rules --with-modules GOOGLE_VERSION=25.1.91+gg1+2
  --with-x=yes --with-x-toolkit=lucid --with-toolkit-scroll-bars
  --without-gconf --without-gsettings build_alias=x86_64-linux-gnu
  'CFLAGS=-g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat
  -Werror=format-security -Wall' 'LDFLAGS=-Wl,-Bsymbolic-functions
  -Wl,-z,relro -Wl,-fuse-ld=gold,--export-dynamic-symbol=__google_auxv'
  'CPPFLAGS=-D_FORTIFY_SOURCE=2 -DGOOGLE_EMACS_DEFINE_AUXV''

Configured features:
XAW3D XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS NOTIFY
LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS LUCID X11 MODULES

Important settings:
   value of $LANG: en_US.UTF-8
   value of $XMODIFIERS: @im=ibus
   locale-coding-system: utf-8-unix

Major mode: Message

Minor modes in effect:
   mml-mode: t
   tooltip-mode: t
   global-eldoc-mode: t
   electric-indent-mode: t
   mouse-wheel-mode: t
   tool-bar-mode: t
   menu-bar-mode: t
   file-name-shadow-mode: t
   global-font-lock-mode: t
   font-lock-mode: t
   blink-cursor-mode: t
   auto-composition-mode: t
   auto-encryption-mode: t
   auto-compression-mode: t
   line-number-mode: t
   auto-fill-function: message-do-auto-fill
   transient-mark-mode: t
   abbrev-mode: t

Recent messages:
Sending...
Mark set
Fix continuation lines? (y or n) y [59 times]
Mark set
Sending via mail...
if: Mail headers not found
Mark set
Sending...
Mark set [2 times]
Sending via mail...
if: Mail headers not found

Load-path shadows:
None found.

Features:
(pp shadow sort mail-extr emacsbug message format-spec rfc822 mml
mml-sec password-cache epg epg-config gnus-util mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader
google-sendgmr sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr
mail-utils novice cl-extra help-fns help-mode easymenu misearch
multi-isearch dired-aux cl-loaddefs pcase cl-lib dired time-date
mule-util tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel x-win term/common-win x-dnd tool-bar dnd fontset
image regexp-opt fringe tabulated-list newcomment elisp-mode lisp-mode
prog-mode register page menu-bar rfn-eshadow timer select scroll-bar
mouse jit-lock font-lock syntax facemenu font-core frame cl-generic cham
georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese charscript case-table epa-hook
jka-cmpr-hook help simple abbrev minibuffer cl-preloaded nadvice
loaddefs button faces cus-face macroexp files text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote dbusbind inotify dynamic-setting
font-render-setting x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 111409 15495)
  (symbols 48 20120 0)
  (miscs 40 211 363)
  (strings 32 15788 3029)
  (string-bytes 1 498060)
  (vectors 16 12877)
  (vector-slots 8 445607 5773)
  (floats 8 175 297)
  (intervals 56 4381 159)
  (buffers 976 29)
  (heap 1024 34656 1309))





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#25547: 25.1.91; emacsclient -c creates frames on the wrong display
  2017-01-26 18:09 bug#25547: 25.1.91; emacsclient -c creates frames on the wrong display Alex Hutcheson
@ 2020-11-02 22:37 ` Stefan Monnier
       [not found]   ` <CAK1djOorpaeoMU-gtd+hQnWWK8tOaER6OPYgukYrn=V1cDh+aQ@mail.gmail.com>
  0 siblings, 1 reply; 13+ messages in thread
From: Stefan Monnier @ 2020-11-02 22:37 UTC (permalink / raw)
  To: Alex Hutcheson; +Cc: 25547, Eliza Velasquez

> To reproduce:
> 1. Run `emacs --daemon' on a system with a graphical display.
> 2. Login to the system using a terminal with no graphical display. You
>    could do this by switching to another virtual terminal (Ctrl-Alt-F1
>    works on Ubuntu), or SSH in from another machine.
> 3. Run `emacsclient -c' or `emacsclient -t' in the terminal to connect
>    to the running Emacs instance.
> 4. Inside Emacs, run M-x shell.
> 5. In the M-x shell buffer, run `emacsclient -c'.
>
> Ideal behavior: emacsclient creates a frame on the current terminal.
> Actual behavior: emacsclient creates a graphical frame in the
>   environment in which it was launched, which is inaccessible from the
>   terminal.

Indeed the current behavior is a bummer.

The second emacsclient runs within a shell process which itself is
associated to the *shell* buffer, which can be displayed in several
windows, including some of them in the GUI.

So in the above case you actually would like to open the emacsclient
wherever `selected-frame` is (because it's generally impossible to walk
out way back from the second emacsclient process to the parent tty in
which the first emacsclient has opened a frame).

Have you tried using `emacsclient` without the `-c` option (you'll have
to provide a file-name in that case)?


        Stefan






^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#25547: 25.1.91; emacsclient -c creates frames on the wrong display
       [not found]     ` <jwvsg9rtghs.fsf-monnier+emacs@gnu.org>
@ 2020-11-03 18:36       ` Eliza Velasquez via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2020-11-05  2:00         ` Eliza Velasquez via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 13+ messages in thread
From: Eliza Velasquez via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2020-11-03 18:36 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 25547, Alex Hutcheson

Okay, I'll try to work on this today and see if I can get anywhere with it.

On Mon, Nov 2, 2020 at 4:56 PM Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>
> > This seems like it might be tricky to solve properly. My guess from
> > looking at the emacsclient source is that a new option would be
> > required in order to maintain backwards compatibility, maybe something
> > like `--smart-create-frame` or `-C`, which would behave as in the
> > expected behavior of the bug report.
>
> Maybe we don't need a special option: we should be able to detect this
> situation when `-c` is passed from an emacsclient running in a "dumb
> tty".
>
> IOW it might be a simple matter of tweaking server.el for the case where
> `tty-type` is `dumb` and the `selected-frame` is a non-GUI frame, in
> which case we'll just want to create a new tty frame in the same
> terminal as that of the `selected-frame`.
>
>
>         Stefan
>





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#25547: 25.1.91; emacsclient -c creates frames on the wrong display
  2020-11-03 18:36       ` Eliza Velasquez via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2020-11-05  2:00         ` Eliza Velasquez via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2020-11-05  4:44           ` Stefan Monnier
  0 siblings, 1 reply; 13+ messages in thread
From: Eliza Velasquez via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2020-11-05  2:00 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 25547, Alex Hutcheson

It looks like I was able to make progress today. Just by modifying
server.el, I've gotten `emacsclient -ce "..."` to create a new frame
based on the currently selected frame's terminal when called in a dumb
terminal. Unfortunately, the command for calling ediff is still very
unpleasant:

emacsclient -ucF "((delete-frame-on-ediff-quit . t))" \
  -e "(ediff-merge-with-ancestor \"${local}\" \"${other}\" \"${base}\"
nil \"${output}\")" \
  -e "(add-hook 'ediff-quit-hook (lambda () (when (frame-parameter nil
'delete-frame-on-ediff-quit) (delete-frame))))"

So perhaps introducing a wrapper script for ediff merges would still be useful.

It's getting late today, so tomorrow morning I'll look into how to
actually submit this as a patch.

On Tue, Nov 3, 2020 at 10:36 AM Eliza Velasquez <exv@google.com> wrote:
>
> Okay, I'll try to work on this today and see if I can get anywhere with it.
>
> On Mon, Nov 2, 2020 at 4:56 PM Stefan Monnier <monnier@iro.umontreal.ca> wrote:
> >
> > > This seems like it might be tricky to solve properly. My guess from
> > > looking at the emacsclient source is that a new option would be
> > > required in order to maintain backwards compatibility, maybe something
> > > like `--smart-create-frame` or `-C`, which would behave as in the
> > > expected behavior of the bug report.
> >
> > Maybe we don't need a special option: we should be able to detect this
> > situation when `-c` is passed from an emacsclient running in a "dumb
> > tty".
> >
> > IOW it might be a simple matter of tweaking server.el for the case where
> > `tty-type` is `dumb` and the `selected-frame` is a non-GUI frame, in
> > which case we'll just want to create a new tty frame in the same
> > terminal as that of the `selected-frame`.
> >
> >
> >         Stefan
> >





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#25547: 25.1.91; emacsclient -c creates frames on the wrong display
  2020-11-05  2:00         ` Eliza Velasquez via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2020-11-05  4:44           ` Stefan Monnier
  2020-11-05 23:41             ` Eliza Velasquez via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 13+ messages in thread
From: Stefan Monnier @ 2020-11-05  4:44 UTC (permalink / raw)
  To: Eliza Velasquez; +Cc: 25547, Alex Hutcheson

> It looks like I was able to make progress today. Just by modifying
> server.el, I've gotten `emacsclient -ce "..."` to create a new frame
> based on the currently selected frame's terminal when called in a dumb
> terminal.

Great!

> Unfortunately, the command for calling ediff is still very
> unpleasant:
>
> emacsclient -ucF "((delete-frame-on-ediff-quit . t))" \
>   -e "(ediff-merge-with-ancestor \"${local}\" \"${other}\" \"${base}\"
> nil \"${output}\")" \
>   -e "(add-hook 'ediff-quit-hook (lambda () (when (frame-parameter nil
> 'delete-frame-on-ediff-quit) (delete-frame))))"

I think the problem is that emacsclient lacks the equivalent of `-f`,
i.e. calling a particular ELisp function where the remaining arguments
can be consumed by that function.  That would solve the problem of
quoting (your above script will stumble when encountering
a file name with quotes in it or with a final backslash).

> So perhaps introducing a wrapper script for ediff merges would still be useful.

We should be able to add a convenience `batch-ediff-merge-with-ancestor`.
Bonus points if you manage to make it work with `emacs --batch -f` as
well as with `emacsclient --<newflag>`.


        Stefan






^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#25547: 25.1.91; emacsclient -c creates frames on the wrong display
  2020-11-05  4:44           ` Stefan Monnier
@ 2020-11-05 23:41             ` Eliza Velasquez via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2020-11-06  0:55               ` Eliza Velasquez via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2020-11-06  5:39               ` Eli Zaretskii
  0 siblings, 2 replies; 13+ messages in thread
From: Eliza Velasquez via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2020-11-05 23:41 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 25547, Alex Hutcheson

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

Oops, forgot to reply all on my last message. Attached is the patch.

On Wed, Nov 4, 2020 at 8:44 PM Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>
> > It looks like I was able to make progress today. Just by modifying
> > server.el, I've gotten `emacsclient -ce "..."` to create a new frame
> > based on the currently selected frame's terminal when called in a dumb
> > terminal.
>
> Great!
>
> > Unfortunately, the command for calling ediff is still very
> > unpleasant:
> >
> > emacsclient -ucF "((delete-frame-on-ediff-quit . t))" \
> >   -e "(ediff-merge-with-ancestor \"${local}\" \"${other}\" \"${base}\"
> > nil \"${output}\")" \
> >   -e "(add-hook 'ediff-quit-hook (lambda () (when (frame-parameter nil
> > 'delete-frame-on-ediff-quit) (delete-frame))))"
>
> I think the problem is that emacsclient lacks the equivalent of `-f`,
> i.e. calling a particular ELisp function where the remaining arguments
> can be consumed by that function.  That would solve the problem of
> quoting (your above script will stumble when encountering
> a file name with quotes in it or with a final backslash).
>
> > So perhaps introducing a wrapper script for ediff merges would still be useful.
>
> We should be able to add a convenience `batch-ediff-merge-with-ancestor`.
> Bonus points if you manage to make it work with `emacs --batch -f` as
> well as with `emacsclient --<newflag>`.
>
>
>         Stefan
>

[-- Attachment #2: 25547.patch --]
[-- Type: application/octet-stream, Size: 2286 bytes --]

diff --git a/lisp/server.el b/lisp/server.el
index a660deab8e..b6d0136d51 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -354,9 +354,11 @@ server-delete-client
 
       (setq server-clients (delq proc server-clients))
 
-      ;; Delete the client's tty, except on Windows (both GUI and console),
-      ;; where there's only one terminal and does not make sense to delete it.
-      (unless (eq system-type 'windows-nt)
+      ;; Delete the client's tty, except on Windows (both GUI and
+      ;; console), where there's only one terminal and does not make
+      ;; sense to delete it, or if we are explicitly told not.
+      (unless (or (eq system-type 'windows-nt)
+                  (process-get proc 'no-delete-terminal))
 	(let ((terminal (process-get proc 'terminal)))
 	  ;; Only delete the terminal if it is non-nil.
 	  (when (and terminal (eq (terminal-live-p terminal) t))
@@ -918,6 +920,19 @@ server-create-window-system-frame
            (server-send-string proc "-window-system-unsupported \n")
            nil))))
 
+(defun server-create-frame (nowait proc &optional parameters)
+  (add-to-list 'frame-inherited-parameters 'client)
+  (let ((frame (make-frame `((client . ,(if nowait 'nowait proc))
+                             ;; This is a leftover, see above.
+                             (environment . ,(process-get proc 'env))
+                             ,@parameters))))
+    (server-log (format "%s created" frame) proc)
+    (select-frame frame)
+    (process-put proc 'frame frame)
+    (process-put proc 'terminal (frame-terminal frame))
+    (process-put proc 'no-delete-terminal t)
+    frame))
+
 (defun server-goto-toplevel (proc)
   (condition-case nil
       ;; If we're running isearch, we must abort it to allow Emacs to
@@ -1264,6 +1279,10 @@ server-process-filter
 					   terminal-frame)))))
 		    (setq tty-name nil tty-type nil)
 		    (if display (server-select-display display)))
+                   ((equal tty-type "dumb")
+                    ;; Dumb terminals should create frames without
+                    ;; preference for tty or window system (bug#25547)
+                    (server-create-frame nowait proc frame-parameters))
 		   ((or (and (eq system-type 'windows-nt)
 			     (daemonp)
 			     (setq display "w32"))

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#25547: 25.1.91; emacsclient -c creates frames on the wrong display
  2020-11-05 23:41             ` Eliza Velasquez via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2020-11-06  0:55               ` Eliza Velasquez via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2020-11-06  5:39               ` Eli Zaretskii
  1 sibling, 0 replies; 13+ messages in thread
From: Eliza Velasquez via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2020-11-06  0:55 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 25547, Alex Hutcheson

Actually, I'm realizing now that my patch requires some more work
after testing Emacs in daemon mode without an X server. I'll continue
this tomorrow.

On Thu, Nov 5, 2020 at 3:41 PM Eliza Velasquez <exv@google.com> wrote:
>
> Oops, forgot to reply all on my last message. Attached is the patch.
>
> On Wed, Nov 4, 2020 at 8:44 PM Stefan Monnier <monnier@iro.umontreal.ca> wrote:
> >
> > > It looks like I was able to make progress today. Just by modifying
> > > server.el, I've gotten `emacsclient -ce "..."` to create a new frame
> > > based on the currently selected frame's terminal when called in a dumb
> > > terminal.
> >
> > Great!
> >
> > > Unfortunately, the command for calling ediff is still very
> > > unpleasant:
> > >
> > > emacsclient -ucF "((delete-frame-on-ediff-quit . t))" \
> > >   -e "(ediff-merge-with-ancestor \"${local}\" \"${other}\" \"${base}\"
> > > nil \"${output}\")" \
> > >   -e "(add-hook 'ediff-quit-hook (lambda () (when (frame-parameter nil
> > > 'delete-frame-on-ediff-quit) (delete-frame))))"
> >
> > I think the problem is that emacsclient lacks the equivalent of `-f`,
> > i.e. calling a particular ELisp function where the remaining arguments
> > can be consumed by that function.  That would solve the problem of
> > quoting (your above script will stumble when encountering
> > a file name with quotes in it or with a final backslash).
> >
> > > So perhaps introducing a wrapper script for ediff merges would still be useful.
> >
> > We should be able to add a convenience `batch-ediff-merge-with-ancestor`.
> > Bonus points if you manage to make it work with `emacs --batch -f` as
> > well as with `emacsclient --<newflag>`.
> >
> >
> >         Stefan
> >





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#25547: 25.1.91; emacsclient -c creates frames on the wrong display
  2020-11-05 23:41             ` Eliza Velasquez via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2020-11-06  0:55               ` Eliza Velasquez via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2020-11-06  5:39               ` Eli Zaretskii
  2020-11-06 19:14                 ` Eliza Velasquez via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2020-11-06  5:39 UTC (permalink / raw)
  To: Eliza Velasquez; +Cc: 25547, alexhutcheson, monnier

> Cc: 25547@debbugs.gnu.org, Alex Hutcheson <alexhutcheson@google.com>
> Date: Thu, 5 Nov 2020 15:41:08 -0800
> From: Eliza Velasquez via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> +(defun server-create-frame (nowait proc &optional parameters)

This function's name is too general, and therefore will confuse
someone who tries to learn how server.el works.  Please name it
according to the convention used by 2 other functions in server.el
that create GUI and TTY frames.

Btw, why can't you use server-create-tty-frame here?  IOW, I don't
think I understand this comment:

> +                   ((equal tty-type "dumb")
> +                    ;; Dumb terminals should create frames without
> +                    ;; preference for tty or window system (bug#25547)
> +                    (server-create-frame nowait proc frame-parameters))

Thanks.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#25547: 25.1.91; emacsclient -c creates frames on the wrong display
  2020-11-06  5:39               ` Eli Zaretskii
@ 2020-11-06 19:14                 ` Eliza Velasquez via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2020-11-06 22:07                   ` Stefan Monnier
  2020-11-07  8:04                   ` Eli Zaretskii
  0 siblings, 2 replies; 13+ messages in thread
From: Eliza Velasquez via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2020-11-06 19:14 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 25547, Alex Hutcheson, Stefan Monnier

To answer your second question first, if we try to use
server-create-tty-frame, Emacs will try to run display the frame in
the terminal of the calling client, which it can't do in the case of a
dumb terminal. The next best thing would be to allocate a new frame on
an existing frame's terminal, whether that is on the tty or window
system. Currently a dumb terminal running `emacsclient -c` can only
create new frames on the window system, and this patch should allow it
to create new frames on an existing tty terminal as well (once it's
working correctly). As the initial report suggests, most of the time a
dumb terminal is running `emacsclient -c`, its because it was called
as part of a script in an Emacs subprocess.

Also, I'll rename the function to something like
(server-create-frame-on-selected-terminal) since I agree its purpose
is not clear from the name.

On Thu, Nov 5, 2020 at 9:39 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > Cc: 25547@debbugs.gnu.org, Alex Hutcheson <alexhutcheson@google.com>
> > Date: Thu, 5 Nov 2020 15:41:08 -0800
> > From: Eliza Velasquez via "Bug reports for GNU Emacs,
> >  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> >
> > +(defun server-create-frame (nowait proc &optional parameters)
>
> This function's name is too general, and therefore will confuse
> someone who tries to learn how server.el works.  Please name it
> according to the convention used by 2 other functions in server.el
> that create GUI and TTY frames.
>
> Btw, why can't you use server-create-tty-frame here?  IOW, I don't
> think I understand this comment:
>
> > +                   ((equal tty-type "dumb")
> > +                    ;; Dumb terminals should create frames without
> > +                    ;; preference for tty or window system (bug#25547)
> > +                    (server-create-frame nowait proc frame-parameters))
>
> Thanks.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#25547: 25.1.91; emacsclient -c creates frames on the wrong display
  2020-11-06 19:14                 ` Eliza Velasquez via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2020-11-06 22:07                   ` Stefan Monnier
  2020-11-07  8:04                   ` Eli Zaretskii
  1 sibling, 0 replies; 13+ messages in thread
From: Stefan Monnier @ 2020-11-06 22:07 UTC (permalink / raw)
  To: Eliza Velasquez; +Cc: 25547, Alex Hutcheson

> To answer your second question first, if we try to use
> server-create-tty-frame, Emacs will try to run display the frame in
> the terminal of the calling client, which it can't do in the case of a
> dumb terminal.

The comment could say something like "`emacsclient` is running
inside something like a `M-x shell` buffer: we can't run Emacs in such
ttys, so we use whichever terminal is currently selected".


        Stefan






^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#25547: 25.1.91; emacsclient -c creates frames on the wrong display
  2020-11-06 19:14                 ` Eliza Velasquez via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2020-11-06 22:07                   ` Stefan Monnier
@ 2020-11-07  8:04                   ` Eli Zaretskii
  2020-11-10  0:14                     ` Eliza Velasquez via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2020-11-07  8:04 UTC (permalink / raw)
  To: Eliza Velasquez; +Cc: 25547, alexhutcheson, monnier

> From: Eliza Velasquez <exv@google.com>
> Date: Fri, 6 Nov 2020 11:14:42 -0800
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, 25547@debbugs.gnu.org, 
> 	Alex Hutcheson <alexhutcheson@google.com>
> 
> To answer your second question first, if we try to use
> server-create-tty-frame, Emacs will try to run display the frame in
> the terminal of the calling client, which it can't do in the case of a
> dumb terminal. The next best thing would be to allocate a new frame on
> an existing frame's terminal, whether that is on the tty or window
> system. Currently a dumb terminal running `emacsclient -c` can only
> create new frames on the window system, and this patch should allow it
> to create new frames on an existing tty terminal as well (once it's
> working correctly). As the initial report suggests, most of the time a
> dumb terminal is running `emacsclient -c`, its because it was called
> as part of a script in an Emacs subprocess.

Thanks for the explanation.

> Also, I'll rename the function to something like
> (server-create-frame-on-selected-terminal) since I agree its purpose
> is not clear from the name.

How about server-create-dumb-terminal-frame instead, since that's
what it really is?





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#25547: 25.1.91; emacsclient -c creates frames on the wrong display
  2020-11-07  8:04                   ` Eli Zaretskii
@ 2020-11-10  0:14                     ` Eliza Velasquez via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2020-11-11  3:14                       ` Stefan Monnier
  0 siblings, 1 reply; 13+ messages in thread
From: Eliza Velasquez via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2020-11-10  0:14 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 25547, Alex Hutcheson, Stefan Monnier

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

> The comment could say something like "`emacsclient` is running
> inside something like a `M-x shell` buffer: we can't run Emacs in such
> ttys, so we use whichever terminal is currently selected".

Done.

> How about server-create-dumb-terminal-frame instead, since that's
> what it really is?

Done.

Also, it turns out my code did actually work properly, I think I
accidentally had Emacs running as a daemon last time I tested and
didn't realize. I just tested it with Emacs running on the GUI,
running on the tty, and running as a daemon/emacsclient tty frame. For
each case, I opened an M-x shell, and ran `emacsclient -c`, and each
time it opened in the expected place.

I attached the patch with the new name/comments.


On Sat, Nov 7, 2020 at 12:04 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Eliza Velasquez <exv@google.com>
> > Date: Fri, 6 Nov 2020 11:14:42 -0800
> > Cc: Stefan Monnier <monnier@iro.umontreal.ca>, 25547@debbugs.gnu.org,
> >       Alex Hutcheson <alexhutcheson@google.com>
> >
> > To answer your second question first, if we try to use
> > server-create-tty-frame, Emacs will try to run display the frame in
> > the terminal of the calling client, which it can't do in the case of a
> > dumb terminal. The next best thing would be to allocate a new frame on
> > an existing frame's terminal, whether that is on the tty or window
> > system. Currently a dumb terminal running `emacsclient -c` can only
> > create new frames on the window system, and this patch should allow it
> > to create new frames on an existing tty terminal as well (once it's
> > working correctly). As the initial report suggests, most of the time a
> > dumb terminal is running `emacsclient -c`, its because it was called
> > as part of a script in an Emacs subprocess.
>
> Thanks for the explanation.
>
> > Also, I'll rename the function to something like
> > (server-create-frame-on-selected-terminal) since I agree its purpose
> > is not clear from the name.
>
> How about server-create-dumb-terminal-frame instead, since that's
> what it really is?

[-- Attachment #2: 25547.patch --]
[-- Type: application/octet-stream, Size: 2543 bytes --]

diff --git a/lisp/server.el b/lisp/server.el
index a660deab8e..734fb8a34a 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -354,9 +354,11 @@ server-delete-client
 
       (setq server-clients (delq proc server-clients))
 
-      ;; Delete the client's tty, except on Windows (both GUI and console),
-      ;; where there's only one terminal and does not make sense to delete it.
-      (unless (eq system-type 'windows-nt)
+      ;; Delete the client's tty, except on Windows (both GUI and
+      ;; console), where there's only one terminal and does not make
+      ;; sense to delete it, or if we are explicitly told not.
+      (unless (or (eq system-type 'windows-nt)
+                  (process-get proc 'no-delete-terminal))
 	(let ((terminal (process-get proc 'terminal)))
 	  ;; Only delete the terminal if it is non-nil.
 	  (when (and terminal (eq (terminal-live-p terminal) t))
@@ -918,6 +920,19 @@ server-create-window-system-frame
            (server-send-string proc "-window-system-unsupported \n")
            nil))))
 
+(defun server-create-dumb-terminal-frame (nowait proc &optional parameters)
+  (add-to-list 'frame-inherited-parameters 'client)
+  (let ((frame (make-frame `((client . ,(if nowait 'nowait proc))
+                             ;; This is a leftover, see above.
+                             (environment . ,(process-get proc 'env))
+                             ,@parameters))))
+    (server-log (format "%s created" frame) proc)
+    (select-frame frame)
+    (process-put proc 'frame frame)
+    (process-put proc 'terminal (frame-terminal frame))
+    (process-put proc 'no-delete-terminal t)
+    frame))
+
 (defun server-goto-toplevel (proc)
   (condition-case nil
       ;; If we're running isearch, we must abort it to allow Emacs to
@@ -1264,6 +1279,14 @@ server-process-filter
 					   terminal-frame)))))
 		    (setq tty-name nil tty-type nil)
 		    (if display (server-select-display display)))
+                   ((equal tty-type "dumb")
+                    ;; Emacsclient is likely running inside something
+                    ;; like an Emacs shell buffer. We can't run an
+                    ;; Emacs frame in a tty like this, so instead, use
+                    ;; whichever terminal is currently
+                    ;; selected. (bug#25547)
+                    (server-create-dumb-terminal-frame nowait proc
+                                                       frame-parameters))
 		   ((or (and (eq system-type 'windows-nt)
 			     (daemonp)
 			     (setq display "w32"))

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#25547: 25.1.91; emacsclient -c creates frames on the wrong display
  2020-11-10  0:14                     ` Eliza Velasquez via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2020-11-11  3:14                       ` Stefan Monnier
  0 siblings, 0 replies; 13+ messages in thread
From: Stefan Monnier @ 2020-11-11  3:14 UTC (permalink / raw)
  To: Eliza Velasquez; +Cc: 25547, Alex Hutcheson

> I attached the patch with the new name/comments.

Thanks, pushed,


        Stefan






^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-11-11  3:14 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-26 18:09 bug#25547: 25.1.91; emacsclient -c creates frames on the wrong display Alex Hutcheson
2020-11-02 22:37 ` Stefan Monnier
     [not found]   ` <CAK1djOorpaeoMU-gtd+hQnWWK8tOaER6OPYgukYrn=V1cDh+aQ@mail.gmail.com>
     [not found]     ` <jwvsg9rtghs.fsf-monnier+emacs@gnu.org>
2020-11-03 18:36       ` Eliza Velasquez via Bug reports for GNU Emacs, the Swiss army knife of text editors
2020-11-05  2:00         ` Eliza Velasquez via Bug reports for GNU Emacs, the Swiss army knife of text editors
2020-11-05  4:44           ` Stefan Monnier
2020-11-05 23:41             ` Eliza Velasquez via Bug reports for GNU Emacs, the Swiss army knife of text editors
2020-11-06  0:55               ` Eliza Velasquez via Bug reports for GNU Emacs, the Swiss army knife of text editors
2020-11-06  5:39               ` Eli Zaretskii
2020-11-06 19:14                 ` Eliza Velasquez via Bug reports for GNU Emacs, the Swiss army knife of text editors
2020-11-06 22:07                   ` Stefan Monnier
2020-11-07  8:04                   ` Eli Zaretskii
2020-11-10  0:14                     ` Eliza Velasquez via Bug reports for GNU Emacs, the Swiss army knife of text editors
2020-11-11  3:14                       ` Stefan Monnier

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).