unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* emacsclient's option decoding code
@ 2008-11-01 14:04 Eli Zaretskii
  2008-11-02  2:13 ` Stefan Monnier
  2008-11-02 21:45 ` Chong Yidong
  0 siblings, 2 replies; 67+ messages in thread
From: Eli Zaretskii @ 2008-11-01 14:04 UTC (permalink / raw)
  To: emacs-devel

Please take a look at this fragment of emacsclient.c, which deals with
various combinations of command-line options:

  if (!current_frame && !tty && !display)
    display = egetenv ("DISPLAY");

  if (display && strlen (display) == 0)
    display = NULL;

  if (!tty && display)
    window_system = 1;
  else if (!current_frame)
    tty = 1;

  /* --no-wait implies --current-frame on ttys when there are file
       arguments or expressions given.  */
  if (nowait && tty && argc - optind > 0)
    current_frame = 1;

  if (current_frame)
    {
      tty = 0;
      window_system = 0;
    }

  if (tty)
    window_system = 0;

I find this code highly confusing to the degree of being
unmaintainable.  Look, for example, how it sets `tty' to zero or
non-zero, and how this is interleaved with testing whether it is zero
or non-zero, completely obfuscating the semantics of the code and the
meaning of each variable.

I think this should be rewritten, but since I cannot make heads or
tails out of it, I cannot do that myself without risking to break the
code.  Please someone make this code speak to a programmer.




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

* Re: emacsclient's option decoding code
  2008-11-01 14:04 emacsclient's option decoding code Eli Zaretskii
@ 2008-11-02  2:13 ` Stefan Monnier
  2008-11-02 21:45 ` Chong Yidong
  1 sibling, 0 replies; 67+ messages in thread
From: Stefan Monnier @ 2008-11-02  2:13 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

> Please take a look at this fragment of emacsclient.c, which deals with
> various combinations of command-line options:

>   if (!current_frame && !tty && !display)
>     display = egetenv ("DISPLAY");

>   if (display && strlen (display) == 0)
>     display = NULL;

>   if (!tty && display)
>     window_system = 1;
>   else if (!current_frame)
>     tty = 1;

>   /* --no-wait implies --current-frame on ttys when there are file
>        arguments or expressions given.  */
>   if (nowait && tty && argc - optind > 0)
>     current_frame = 1;

>   if (current_frame)
>     {
>       tty = 0;
>       window_system = 0;
>     }

>   if (tty)
>     window_system = 0;

> I find this code highly confusing to the degree of being
> unmaintainable.  Look, for example, how it sets `tty' to zero or
> non-zero, and how this is interleaved with testing whether it is zero
> or non-zero, completely obfuscating the semantics of the code and the
> meaning of each variable.

> I think this should be rewritten, but since I cannot make heads or
> tails out of it, I cannot do that myself without risking to break the
> code.  Please someone make this code speak to a programmer.

Yes, I've written my fair share of it, and I'm not proud of it.
If someone can make it more clear, that'd be welcome.  A marginal
improvement is to add an `else' before the last `if'.

Basically, the first 2 `if's setup the `display' variable; the
subsequent 3 enable various options (window_system, tty, and
current_frame) implied by the context; and the last 2 choose one among
the 3 mutually-exclusive options `current_frame', `tty', and
`window_system'.


        Stefan




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

* Re: emacsclient's option decoding code
  2008-11-01 14:04 emacsclient's option decoding code Eli Zaretskii
  2008-11-02  2:13 ` Stefan Monnier
@ 2008-11-02 21:45 ` Chong Yidong
  2008-11-02 22:00   ` Juanma Barranquero
  1 sibling, 1 reply; 67+ messages in thread
From: Chong Yidong @ 2008-11-02 21:45 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

> I find this code highly confusing to the degree of being
> unmaintainable.  Look, for example, how it sets `tty' to zero or
> non-zero, and how this is interleaved with testing whether it is zero
> or non-zero, completely obfuscating the semantics of the code and the
> meaning of each variable.
>
> I think this should be rewritten, but since I cannot make heads or
> tails out of it, I cannot do that myself without risking to break the
> code.  Please someone make this code speak to a programmer.

I did what I could to simplify the code (partly by removing the
redundant variable window_system, partly by adding comments).

Hopefully nothing broke.  My testing indicates no problems.




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

* Re: emacsclient's option decoding code
  2008-11-02 21:45 ` Chong Yidong
@ 2008-11-02 22:00   ` Juanma Barranquero
  2008-11-02 23:18     ` Chong Yidong
  0 siblings, 1 reply; 67+ messages in thread
From: Juanma Barranquero @ 2008-11-02 22:00 UTC (permalink / raw)
  To: Chong Yidong; +Cc: Eli Zaretskii, emacs-devel

On Sun, Nov 2, 2008 at 22:45, Chong Yidong <cyd@stupidchicken.com> wrote:

> Hopefully nothing broke.  My testing indicates no problems.

_find_tty is defined inside the "#if !defined
(NO_SOCKETS_IN_FILE_SYSTEM)" conditional, so yes, something breaks on
Windows:

oo-spd/i386/emacsclient.o: In function `main':
C:\emacs\lib-src/emacsclient.c:1490: undefined reference to `_find_tty'
collect2: ld returned 1 exit status

             Juanma




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

* Re: emacsclient's option decoding code
  2008-11-02 22:00   ` Juanma Barranquero
@ 2008-11-02 23:18     ` Chong Yidong
  2008-11-03  0:50       ` Juanma Barranquero
  0 siblings, 1 reply; 67+ messages in thread
From: Chong Yidong @ 2008-11-02 23:18 UTC (permalink / raw)
  To: Juanma Barranquero; +Cc: Eli Zaretskii, emacs-devel

"Juanma Barranquero" <lekktu@gmail.com> writes:

> _find_tty is defined inside the "#if !defined
> (NO_SOCKETS_IN_FILE_SYSTEM)" conditional, so yes, something breaks on
> Windows:

Thanks for the heads-up.  I moved it out of the conditional.  Does it
work okay now?




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

* Re: emacsclient's option decoding code
  2008-11-02 23:18     ` Chong Yidong
@ 2008-11-03  0:50       ` Juanma Barranquero
  2008-11-03  4:14         ` Eli Zaretskii
  0 siblings, 1 reply; 67+ messages in thread
From: Juanma Barranquero @ 2008-11-03  0:50 UTC (permalink / raw)
  To: Chong Yidong; +Cc: Eli Zaretskii, emacs-devel

On Mon, Nov 3, 2008 at 00:18, Chong Yidong <cyd@stupidchicken.com> wrote:

> Thanks for the heads-up.  I moved it out of the conditional.  Does it
> work okay now?

It does compile, yes. emacsclient -c or -t still trigger the same
assertion failure.

  Juanma




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

* Re: emacsclient's option decoding code
  2008-11-03  0:50       ` Juanma Barranquero
@ 2008-11-03  4:14         ` Eli Zaretskii
  2008-11-03 12:12           ` Juanma Barranquero
  2008-11-03 14:45           ` Juanma Barranquero
  0 siblings, 2 replies; 67+ messages in thread
From: Eli Zaretskii @ 2008-11-03  4:14 UTC (permalink / raw)
  To: Juanma Barranquero; +Cc: cyd, emacs-devel

> Date: Mon, 3 Nov 2008 01:50:59 +0100
> From: "Juanma Barranquero" <lekktu@gmail.com>
> Cc: "Eli Zaretskii" <eliz@gnu.org>, emacs-devel@gnu.org
> 
> On Mon, Nov 3, 2008 at 00:18, Chong Yidong <cyd@stupidchicken.com> wrote:
> 
> > Thanks for the heads-up.  I moved it out of the conditional.  Does it
> > work okay now?
> 
> It does compile, yes. emacsclient -c or -t still trigger the same
> assertion failure.

That's a different problem, it needs some work in server.el.  After
that, at least -c should work on Windows, but its support in server.el
was never written originally to work on anything but Unix.

I will look into it when I have time.  You can file a bug report, if
you want to make sure we don't forget this.

Thanks.





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

* Re: emacsclient's option decoding code
  2008-11-03  4:14         ` Eli Zaretskii
@ 2008-11-03 12:12           ` Juanma Barranquero
  2008-11-03 20:06             ` Eli Zaretskii
  2008-11-03 14:45           ` Juanma Barranquero
  1 sibling, 1 reply; 67+ messages in thread
From: Juanma Barranquero @ 2008-11-03 12:12 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: cyd, emacs-devel

On Mon, Nov 3, 2008 at 05:14, Eli Zaretskii <eliz@gnu.org> wrote:

> That's a different problem, it needs some work in server.el.

Yes, I know. I was pointing out that Chong had succesfully reverted to
the previous condition.

> After
> that, at least -c should work on Windows, but its support in server.el
> was never written originally to work on anything but Unix.
>
> I will look into it when I have time.  You can file a bug report, if
> you want to make sure we don't forget this.

The following proof-of-concept patch seems to work (at the cost of
adding windows-specific conditions).

  Juanma


Index: lib-src/emacsclient.c
===================================================================
RCS file: /sources/emacs/emacs/lib-src/emacsclient.c,v
retrieving revision 1.140
diff -u -2 -b -r1.140 emacsclient.c
--- lib-src/emacsclient.c	2 Nov 2008 23:16:33 -0000	1.140
+++ lib-src/emacsclient.c	3 Nov 2008 11:37:19 -0000
@@ -575,7 +575,9 @@
     display = NULL;

+#ifndef WINDOWSNT
   /* If no display is available, new frames are tty frames.  */
   if (!current_frame && !display)
     tty = 1;
+#endif

   /* --no-wait implies --current-frame on ttys when there are file
Index: lisp/server.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/server.el,v
retrieving revision 1.171
diff -u -2 -b -r1.171 server.el
--- lisp/server.el	30 Oct 2008 15:50:01 -0000	1.171
+++ lisp/server.el	3 Nov 2008 12:06:49 -0000
@@ -631,10 +631,12 @@
                      ;; This is a leftover, see above.
                      (environment . ,(process-get proc 'env))))
-           (frame (make-frame-on-display
+           (frame (if (eq system-type 'windows-nt)
+		      (make-frame params)
+		    (make-frame-on-display
                    (or display
                        (frame-parameter nil 'display)
                        (getenv "DISPLAY")
                        (error "Please specify display"))
-                   params)))
+		     params))))
       (server-log (format "%s created" frame) proc)
       (select-frame frame)




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

* Re: emacsclient's option decoding code
  2008-11-03  4:14         ` Eli Zaretskii
  2008-11-03 12:12           ` Juanma Barranquero
@ 2008-11-03 14:45           ` Juanma Barranquero
  2008-11-03 19:59             ` Eli Zaretskii
  2008-11-03 20:03             ` Eli Zaretskii
  1 sibling, 2 replies; 67+ messages in thread
From: Juanma Barranquero @ 2008-11-03 14:45 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: cyd, emacs-devel

On Mon, Nov 3, 2008 at 05:14, Eli Zaretskii <eliz@gnu.org> wrote:

> I will look into it when I have time.  You can file a bug report, if
> you want to make sure we don't forget this.

As for the -t crash, on one hand emacsclient on Windows shouldn't ever
request a tty.

OTOH, the underlying bug is that, on Windows, calling
make-terminal-frame with a non-nil tty-type from inside a windowing
Emacs crashes it.

Is there any point in allowing it? Shouldn't make-terminal-frame on
Windows just err out unless Emacs is running in -nw mode?

  Juanma




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

* Re: emacsclient's option decoding code
  2008-11-03 14:45           ` Juanma Barranquero
@ 2008-11-03 19:59             ` Eli Zaretskii
  2008-11-03 20:03             ` Eli Zaretskii
  1 sibling, 0 replies; 67+ messages in thread
From: Eli Zaretskii @ 2008-11-03 19:59 UTC (permalink / raw)
  To: Juanma Barranquero; +Cc: cyd, emacs-devel

> X-Spam-Status: No, score=-0.4 required=5.0 tests=AWL,BAYES_00,
> 	DNS_FROM_RFC_POST,DNS_FROM_SECURITYSAGE autolearn=no version=3.1.0
> Date: Mon, 3 Nov 2008 15:45:48 +0100
> From: "Juanma Barranquero" <lekktu@gmail.com>
> Cc: cyd@stupidchicken.com, emacs-devel@gnu.org
> 
> On Mon, Nov 3, 2008 at 05:14, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > I will look into it when I have time.  You can file a bug report, if
> > you want to make sure we don't forget this.
> 
> As for the -t crash, on one hand emacsclient on Windows shouldn't ever
> request a tty.
> 
> OTOH, the underlying bug is that, on Windows, calling
> make-terminal-frame with a non-nil tty-type from inside a windowing
> Emacs crashes it.
> 
> Is there any point in allowing it? Shouldn't make-terminal-frame on
> Windows just err out unless Emacs is running in -nw mode?
> 
>   Juanma
> 




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

* Re: emacsclient's option decoding code
  2008-11-03 14:45           ` Juanma Barranquero
  2008-11-03 19:59             ` Eli Zaretskii
@ 2008-11-03 20:03             ` Eli Zaretskii
  1 sibling, 0 replies; 67+ messages in thread
From: Eli Zaretskii @ 2008-11-03 20:03 UTC (permalink / raw)
  To: Juanma Barranquero; +Cc: cyd, emacs-devel

> Date: Mon, 3 Nov 2008 15:45:48 +0100
> From: "Juanma Barranquero" <lekktu@gmail.com>
> Cc: cyd@stupidchicken.com, emacs-devel@gnu.org
> 
> On Mon, Nov 3, 2008 at 05:14, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > I will look into it when I have time.  You can file a bug report, if
> > you want to make sure we don't forget this.
> 
> As for the -t crash, on one hand emacsclient on Windows shouldn't ever
> request a tty.

I'm not sure I agree: a native w32 emacsclient could be talking to a
Cygwin Emacs, in which case -t should work.  Also, what about the case
where a native w32 Emacs runs with -nw, and the tty name is CONOUT$ ?

> OTOH, the underlying bug is that, on Windows, calling
> make-terminal-frame with a non-nil tty-type from inside a windowing
> Emacs crashes it.

Yes, and that should be fixed.

> Shouldn't make-terminal-frame on Windows just err out unless Emacs
> is running in -nw mode?

Probably so, but make-terminal-frame should also DTRT or at least fail
gracefully under -nw as well, when called with tty that is not the
default ("CONOUT$").




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

* Re: emacsclient's option decoding code
  2008-11-03 12:12           ` Juanma Barranquero
@ 2008-11-03 20:06             ` Eli Zaretskii
  2008-11-03 20:19               ` Chong Yidong
  2008-11-03 21:26               ` Juanma Barranquero
  0 siblings, 2 replies; 67+ messages in thread
From: Eli Zaretskii @ 2008-11-03 20:06 UTC (permalink / raw)
  To: Juanma Barranquero; +Cc: cyd, emacs-devel

> Date: Mon, 3 Nov 2008 13:12:33 +0100
> From: "Juanma Barranquero" <lekktu@gmail.com>
> Cc: cyd@stupidchicken.com, emacs-devel@gnu.org
> 
> The following proof-of-concept patch seems to work (at the cost of
> adding windows-specific conditions).

Thanks, but I'd like to look for a cleaner patch, one that doesn't add
new ifdef's.

> --- lisp/server.el	30 Oct 2008 15:50:01 -0000	1.171
> +++ lisp/server.el	3 Nov 2008 12:06:49 -0000
> @@ -631,10 +631,12 @@
>                       ;; This is a leftover, see above.
>                       (environment . ,(process-get proc 'env))))
> -           (frame (make-frame-on-display
> +           (frame (if (eq system-type 'windows-nt)
> +		      (make-frame params)
> +		    (make-frame-on-display
>                     (or display
>                         (frame-parameter nil 'display)
>                         (getenv "DISPLAY")
>                         (error "Please specify display"))
> -                   params)))
> +		     params))))
>        (server-log (format "%s created" frame) proc)
>        (select-frame frame)
> 

Here' I think it's unclean that the w32 port has make-frame-on-display
fboundp, but it's dysfunctional when called.  If make-frame-on-display
can be made to work on Windows, provided that `display' is nil (or
maybe it should ignore `display' even if non-nil?), that would be a
cleaner and more general change.




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

* Re: emacsclient's option decoding code
  2008-11-03 20:06             ` Eli Zaretskii
@ 2008-11-03 20:19               ` Chong Yidong
  2008-11-04  4:20                 ` Eli Zaretskii
  2008-11-03 21:26               ` Juanma Barranquero
  1 sibling, 1 reply; 67+ messages in thread
From: Chong Yidong @ 2008-11-03 20:19 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Juanma Barranquero, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

> I think it's unclean that the w32 port has make-frame-on-display
> fboundp, but it's dysfunctional when called.  If make-frame-on-display
> can be made to work on Windows, provided that `display' is nil (or
> maybe it should ignore `display' even if non-nil?), that would be a
> cleaner and more general change.

I agree.  Maybe something like this?

*** trunk/lisp/frame.el.~1.287.~	2008-11-03 11:44:19.000000000 -0500
--- trunk/lisp/frame.el	2008-11-03 15:18:41.000000000 -0500
***************
*** 610,628 ****
    "Make a frame on X display DISPLAY.
  The optional second argument PARAMETERS specifies additional frame parameters."
    (interactive "sMake frame on display: ")
!   (if (featurep 'ns)
!       (progn
! 	(when (and (boundp 'ns-initialized) (not ns-initialized))
! 	  (setq x-display-name display)
! 	  (ns-initialize-window-system))
! 	(make-frame `((window-system . ns) (display . ,display) . ,parameters)))
!     (progn
!       (unless (string-match "\\`[^:]*:[0-9]+\\(\\.[0-9]+\\)?\\'" display)
! 	(error "Invalid display, not HOST:SERVER or HOST:SERVER.SCREEN"))
!       (when (and (boundp 'x-initialized) (not x-initialized))
! 	(setq x-display-name display)
! 	(x-initialize-window-system))
!       (make-frame `((window-system . x) (display . ,display) . ,parameters)))))
  
  (defun make-frame-on-tty (tty type &optional parameters)
    "Make a frame on terminal device TTY.
--- 610,632 ----
    "Make a frame on X display DISPLAY.
  The optional second argument PARAMETERS specifies additional frame parameters."
    (interactive "sMake frame on display: ")
!   (cond ((featurep 'ns)
! 	 (when (and (boundp 'ns-initialized) (not ns-initialized))
! 	   (setq x-display-name display)
! 	   (ns-initialize-window-system))
! 	 (make-frame `((window-system . ns)
! 		       (display . ,display) . ,parameters)))
! 	((eq system-type 'windows-nt)
! 	 ;; Ignore DISPLAY.
! 	 (make-frame parameters))
! 	(t
! 	 (unless (string-match "\\`[^:]*:[0-9]+\\(\\.[0-9]+\\)?\\'" display)
! 	   (error "Invalid display, not HOST:SERVER or HOST:SERVER.SCREEN"))
! 	 (when (and (boundp 'x-initialized) (not x-initialized))
! 	   (setq x-display-name display)
! 	   (x-initialize-window-system))
! 	 (make-frame `((window-system . x)
! 		       (display . ,display) . ,parameters)))))
  
  (defun make-frame-on-tty (tty type &optional parameters)
    "Make a frame on terminal device TTY.




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

* Re: emacsclient's option decoding code
  2008-11-03 20:06             ` Eli Zaretskii
  2008-11-03 20:19               ` Chong Yidong
@ 2008-11-03 21:26               ` Juanma Barranquero
  2008-11-04  4:19                 ` Eli Zaretskii
  1 sibling, 1 reply; 67+ messages in thread
From: Juanma Barranquero @ 2008-11-03 21:26 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: cyd, emacs-devel

On Mon, Nov 3, 2008 at 21:06, Eli Zaretskii <eliz@gnu.org> wrote:

> Thanks, but I'd like to look for a cleaner patch, one that doesn't add
> new ifdef's.

That's why I said "proof-of-concept".

> Here' I think it's unclean that the w32 port has make-frame-on-display
> fboundp, but it's dysfunctional when called.  If make-frame-on-display
> can be made to work on Windows, provided that `display' is nil (or
> maybe it should ignore `display' even if non-nil?), that would be a
> cleaner and more general change.

Of course, but, is there any use of make-frame-on-display on Windows,
with display non-nil?

  Juanma




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

* Re: emacsclient's option decoding code
  2008-11-03 21:26               ` Juanma Barranquero
@ 2008-11-04  4:19                 ` Eli Zaretskii
  2008-11-06 12:08                   ` Juanma Barranquero
  0 siblings, 1 reply; 67+ messages in thread
From: Eli Zaretskii @ 2008-11-04  4:19 UTC (permalink / raw)
  To: Juanma Barranquero; +Cc: cyd, emacs-devel

> Date: Mon, 3 Nov 2008 22:26:54 +0100
> From: "Juanma Barranquero" <lekktu@gmail.com>
> Cc: cyd@stupidchicken.com, emacs-devel@gnu.org
> 
> > Here' I think it's unclean that the w32 port has make-frame-on-display
> > fboundp, but it's dysfunctional when called.  If make-frame-on-display
> > can be made to work on Windows, provided that `display' is nil (or
> > maybe it should ignore `display' even if non-nil?), that would be a
> > cleaner and more general change.
> 
> Of course, but, is there any use of make-frame-on-display on Windows,
> with display non-nil?

"Any use" in what sense?

I was thinking about doing TRT when some code calls
make-frame-on-display, assuming that it will work on any platform that
supports a GUI session.




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

* Re: emacsclient's option decoding code
  2008-11-03 20:19               ` Chong Yidong
@ 2008-11-04  4:20                 ` Eli Zaretskii
  2008-11-04 16:40                   ` Chong Yidong
  0 siblings, 1 reply; 67+ messages in thread
From: Eli Zaretskii @ 2008-11-04  4:20 UTC (permalink / raw)
  To: Chong Yidong; +Cc: lekktu, emacs-devel

> From: Chong Yidong <cyd@stupidchicken.com>
> Cc: Juanma Barranquero <lekktu@gmail.com>,  emacs-devel@gnu.org
> Date: Mon, 03 Nov 2008 15:19:42 -0500
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I think it's unclean that the w32 port has make-frame-on-display
> > fboundp, but it's dysfunctional when called.  If make-frame-on-display
> > can be made to work on Windows, provided that `display' is nil (or
> > maybe it should ignore `display' even if non-nil?), that would be a
> > cleaner and more general change.
> 
> I agree.  Maybe something like this?

Actually, I was thinking about something deeper, somewhere below
make-frame.  But I guess this will also do.  Thanks.




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

* Re: emacsclient's option decoding code
  2008-11-04  4:20                 ` Eli Zaretskii
@ 2008-11-04 16:40                   ` Chong Yidong
  2008-11-06 11:56                     ` Juanma Barranquero
  0 siblings, 1 reply; 67+ messages in thread
From: Chong Yidong @ 2008-11-04 16:40 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: lekktu, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> > I think it's unclean that the w32 port has make-frame-on-display
>> > fboundp, but it's dysfunctional when called.  If make-frame-on-display
>> > can be made to work on Windows, provided that `display' is nil (or
>> > maybe it should ignore `display' even if non-nil?), that would be a
>> > cleaner and more general change.
>> 
>> I agree.  Maybe something like this?
>
> Actually, I was thinking about something deeper, somewhere below
> make-frame.  But I guess this will also do.  Thanks.

OK, checked in.




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

* Re: emacsclient's option decoding code
  2008-11-04 16:40                   ` Chong Yidong
@ 2008-11-06 11:56                     ` Juanma Barranquero
  0 siblings, 0 replies; 67+ messages in thread
From: Juanma Barranquero @ 2008-11-06 11:56 UTC (permalink / raw)
  To: Chong Yidong; +Cc: Eli Zaretskii, emacs-devel

On Tue, Nov 4, 2008 at 17:40, Chong Yidong <cyd@stupidchicken.com> wrote:

>> Actually, I was thinking about something deeper, somewhere below
>> make-frame.  But I guess this will also do.  Thanks.
>
> OK, checked in.

Your patch makes changes in server.el unnecessary, but for emacsclient
-c to work on Windows the attached change is still needed to
emacsclient.c.

             Juanma


Index: lib-src/emacsclient.c
===================================================================
RCS file: /sources/emacs/emacs/lib-src/emacsclient.c,v
retrieving revision 1.140
diff -u -2 -b -r1.140 emacsclient.c
--- lib-src/emacsclient.c       2 Nov 2008 23:16:33 -0000       1.140
+++ lib-src/emacsclient.c       3 Nov 2008 11:37:19 -0000
@@ -575,7 +575,9 @@
    display = NULL;

+#ifndef WINDOWSNT
  /* If no display is available, new frames are tty frames.  */
  if (!current_frame && !display)
    tty = 1;
+#endif




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

* Re: emacsclient's option decoding code
  2008-11-04  4:19                 ` Eli Zaretskii
@ 2008-11-06 12:08                   ` Juanma Barranquero
  2008-11-07  9:53                     ` Eli Zaretskii
  0 siblings, 1 reply; 67+ messages in thread
From: Juanma Barranquero @ 2008-11-06 12:08 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: cyd, emacs-devel

On Tue, Nov 4, 2008 at 05:19, Eli Zaretskii <eliz@gnu.org> wrote:

>> Of course, but, is there any use of make-frame-on-display on Windows,
>> with display non-nil?
>
> "Any use" in what sense?

Is there any possibility (now or in the future) that a Windows
(non-Cygwin) Emacs could create a frame in a X server?

  Juanma




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

* Re: emacsclient's option decoding code
  2008-11-06 12:08                   ` Juanma Barranquero
@ 2008-11-07  9:53                     ` Eli Zaretskii
  2008-11-07 11:42                       ` Juanma Barranquero
  0 siblings, 1 reply; 67+ messages in thread
From: Eli Zaretskii @ 2008-11-07  9:53 UTC (permalink / raw)
  To: Juanma Barranquero; +Cc: cyd, emacs-devel

> Date: Thu, 6 Nov 2008 13:08:04 +0100
> From: "Juanma Barranquero" <lekktu@gmail.com>
> Cc: cyd@stupidchicken.com, emacs-devel@gnu.org
> 
> On Tue, Nov 4, 2008 at 05:19, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> >> Of course, but, is there any use of make-frame-on-display on Windows,
> >> with display non-nil?
> >
> > "Any use" in what sense?
> 
> Is there any possibility (now or in the future) that a Windows
> (non-Cygwin) Emacs could create a frame in a X server?

When support for such a possibility is added to Emacs,
make-frame-on-display should be modified to use it.  Since using that
API would be the first thing people will do after adding support for X
frames, I'm not worried that it will be forgotten.




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

* Re: emacsclient's option decoding code
  2008-11-07  9:53                     ` Eli Zaretskii
@ 2008-11-07 11:42                       ` Juanma Barranquero
  2008-11-07 11:47                         ` Juanma Barranquero
  2008-11-07 14:35                         ` Eli Zaretskii
  0 siblings, 2 replies; 67+ messages in thread
From: Juanma Barranquero @ 2008-11-07 11:42 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: cyd, emacs-devel

On Fri, Nov 7, 2008 at 10:53, Eli Zaretskii <eliz@gnu.org> wrote:

> When support for such a possibility is added to Emacs,
> make-frame-on-display should be modified to use it.  Since using that
> API would be the first thing people will do after adding support for X
> frames, I'm not worried that it will be forgotten.

No, of course it won't be forgotten.

But I don't see how making

  (make-frame ...)

and

  (make-frame-on-display nil ...)

be synonyms exclusively on Windows is a bonus. If a null display means
"make a frame on the current display", it should be so for X too.

  Juanma




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

* Re: emacsclient's option decoding code
  2008-11-07 11:42                       ` Juanma Barranquero
@ 2008-11-07 11:47                         ` Juanma Barranquero
  2008-11-07 14:36                           ` Eli Zaretskii
  2008-11-07 14:35                         ` Eli Zaretskii
  1 sibling, 1 reply; 67+ messages in thread
From: Juanma Barranquero @ 2008-11-07 11:47 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: cyd, emacs-devel

On Fri, Nov 7, 2008 at 12:42, Juanma Barranquero <lekktu@gmail.com> wrote:

> If a null display means
> "make a frame on the current display", it should be so for X too.

Also, the Windows-only behavior is undocumented, except in a comment
in the code. And `make-frame-on-display' is documented in the Multiple
Displays section of the Elisp reference, obviously.

All in all, I think that's a clumsy change.

  Juanma




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

* Re: emacsclient's option decoding code
  2008-11-07 11:42                       ` Juanma Barranquero
  2008-11-07 11:47                         ` Juanma Barranquero
@ 2008-11-07 14:35                         ` Eli Zaretskii
  1 sibling, 0 replies; 67+ messages in thread
From: Eli Zaretskii @ 2008-11-07 14:35 UTC (permalink / raw)
  To: Juanma Barranquero; +Cc: cyd, emacs-devel

> Date: Fri, 7 Nov 2008 12:42:54 +0100
> From: "Juanma Barranquero" <lekktu@gmail.com>
> Cc: cyd@stupidchicken.com, emacs-devel@gnu.org
> 
> But I don't see how making
> 
>   (make-frame ...)
> 
> and
> 
>   (make-frame-on-display nil ...)
> 
> be synonyms exclusively on Windows is a bonus. If a null display means
> "make a frame on the current display", it should be so for X too.

That's fine with me.




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

* Re: emacsclient's option decoding code
  2008-11-07 11:47                         ` Juanma Barranquero
@ 2008-11-07 14:36                           ` Eli Zaretskii
  2008-11-07 14:42                             ` Juanma Barranquero
  0 siblings, 1 reply; 67+ messages in thread
From: Eli Zaretskii @ 2008-11-07 14:36 UTC (permalink / raw)
  To: Juanma Barranquero; +Cc: cyd, emacs-devel

> Date: Fri, 7 Nov 2008 12:47:48 +0100
> From: "Juanma Barranquero" <lekktu@gmail.com>
> Cc: cyd@stupidchicken.com, emacs-devel@gnu.org
> 
> On Fri, Nov 7, 2008 at 12:42, Juanma Barranquero <lekktu@gmail.com> wrote:
> 
> > If a null display means
> > "make a frame on the current display", it should be so for X too.
> 
> Also, the Windows-only behavior is undocumented, except in a comment
> in the code. And `make-frame-on-display' is documented in the Multiple
> Displays section of the Elisp reference, obviously.
> 
> All in all, I think that's a clumsy change.

What would you suggest in its stead?




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

* Re: emacsclient's option decoding code
  2008-11-07 14:36                           ` Eli Zaretskii
@ 2008-11-07 14:42                             ` Juanma Barranquero
  2008-11-10 18:22                               ` Juanma Barranquero
  2008-11-15  2:58                               ` Evil Boris
  0 siblings, 2 replies; 67+ messages in thread
From: Juanma Barranquero @ 2008-11-07 14:42 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: cyd, emacs-devel

On Fri, Nov 7, 2008 at 15:36, Eli Zaretskii <eliz@gnu.org> wrote:

>> All in all, I think that's a clumsy change.
>
> What would you suggest in its stead?

IMO making a frame in the current display and making a frame in a
remote display are different operations and the point of call should
know about it. We don't usually conflate making tty and X frames,
either.

  Juanma




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

* Re: emacsclient's option decoding code
  2008-11-07 14:42                             ` Juanma Barranquero
@ 2008-11-10 18:22                               ` Juanma Barranquero
  2008-11-10 18:36                                 ` Chong Yidong
  2008-11-11  4:09                                 ` Eli Zaretskii
  2008-11-15  2:58                               ` Evil Boris
  1 sibling, 2 replies; 67+ messages in thread
From: Juanma Barranquero @ 2008-11-10 18:22 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: cyd, emacs-devel

Summary of pending issues in this thread:

 - emacsclient -t causes a crash / assertion error; filed as bug#1325.

 - emacsclient -c causes the same error, because emacsclient thinks
that "-c" + null DISPLAY => "-t"; I proposed the attached patch (1),
because even if -t is supported on Windows, the assumption that an
empty DISPLAY implies a tty frame is false.

 - (make-frame-on-display nil ...) is a synonym of (make-frame ...),
but only on Windows; I proposed patch (2). Does anyone see a problem
with it?

 - Regardless of whether the previous point is accepted or not, the
new behavior should be documented.

   Juanma



Patch (1):

Index: lib-src/emacsclient.c
===================================================================
RCS file: /sources/emacs/emacs/lib-src/emacsclient.c,v
retrieving revision 1.140
diff -u -2 -b -r1.140 emacsclient.c
--- lib-src/emacsclient.c       2 Nov 2008 23:16:33 -0000       1.140
+++ lib-src/emacsclient.c       3 Nov 2008 11:37:19 -0000
@@ -575,7 +575,9 @@
   display = NULL;

+#ifndef WINDOWSNT
 /* If no display is available, new frames are tty frames.  */
 if (!current_frame && !display)
   tty = 1;
+#endif


Patch (2):

Index: lisp/frame.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/frame.el,v
retrieving revision 1.289
diff -u -2 -r1.289 frame.el
--- lisp/frame.el	7 Nov 2008 14:52:04 -0000	1.289
+++ lisp/frame.el	10 Nov 2008 18:17:48 -0000
@@ -609,4 +609,5 @@
 (defun make-frame-on-display (display &optional parameters)
   "Make a frame on display DISPLAY.
+If DISPLAY is nil, default to the current display.
 The optional argument PARAMETERS specifies additional frame parameters."
   (interactive "sMake frame on display: ")
@@ -617,6 +618,5 @@
 	 (make-frame `((window-system . ns)
 		       (display . ,display) . ,parameters)))
-	((eq system-type 'windows-nt)
-	 ;; On Windows, ignore DISPLAY.
+	((null display)
 	 (make-frame parameters))
 	(t




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

* Re: emacsclient's option decoding code
  2008-11-10 18:22                               ` Juanma Barranquero
@ 2008-11-10 18:36                                 ` Chong Yidong
  2008-11-10 23:33                                   ` Juanma Barranquero
  2008-11-11  4:09                                 ` Eli Zaretskii
  1 sibling, 1 reply; 67+ messages in thread
From: Chong Yidong @ 2008-11-10 18:36 UTC (permalink / raw)
  To: Juanma Barranquero; +Cc: Eli Zaretskii, emacs-devel

"Juanma Barranquero" <lekktu@gmail.com> writes:

>  - emacsclient -c causes the same error, because emacsclient thinks
> that "-c" + null DISPLAY => "-t"; I proposed the attached patch (1),
> because even if -t is supported on Windows, the assumption that an
> empty DISPLAY implies a tty frame is false.
>
>  - (make-frame-on-display nil ...) is a synonym of (make-frame ...),
> but only on Windows; I proposed patch (2). Does anyone see a problem
> with it?

These two patches look OK to me.




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

* Re: emacsclient's option decoding code
  2008-11-10 18:36                                 ` Chong Yidong
@ 2008-11-10 23:33                                   ` Juanma Barranquero
  0 siblings, 0 replies; 67+ messages in thread
From: Juanma Barranquero @ 2008-11-10 23:33 UTC (permalink / raw)
  To: Chong Yidong; +Cc: Eli Zaretskii, emacs-devel

On Mon, Nov 10, 2008 at 19:36, Chong Yidong <cyd@stupidchicken.com> wrote:

>>  - (make-frame-on-display nil ...) is a synonym of (make-frame ...),
>> but only on Windows; I proposed patch (2). Does anyone see a problem
>> with it?
>
> These two patches look OK to me.

Well, it's worse than that, because with your change to `make-frame-on-display',

   (make-frame-on-display "")

works on Windows; in fact, that's what
`server-create-window-system-frame' uses, because on Windows,

  (frame-parameter nil 'display)  => ""

With my patch, "" would not be accepted, because it is obviously not
null. We'd have to use

	((or (null display) (eq system-type 'windows-nt))
	 (make-frame parameters))

which is progressively uglier.

All in all, I think accepting anything as DISPLAY for Windows is a bad
choice. If we're going that route, it'd be cleaner to accept only nil
as default display, and fix the callers to use nil for "" on Windows.

  Juanma




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

* Re: emacsclient's option decoding code
  2008-11-10 18:22                               ` Juanma Barranquero
  2008-11-10 18:36                                 ` Chong Yidong
@ 2008-11-11  4:09                                 ` Eli Zaretskii
  2008-11-11  4:14                                   ` Chong Yidong
  2008-11-11  9:39                                   ` Juanma Barranquero
  1 sibling, 2 replies; 67+ messages in thread
From: Eli Zaretskii @ 2008-11-11  4:09 UTC (permalink / raw)
  To: Juanma Barranquero; +Cc: cyd, emacs-devel

> Date: Mon, 10 Nov 2008 19:22:08 +0100
> From: "Juanma Barranquero" <lekktu@gmail.com>
> Cc: cyd@stupidchicken.com, emacs-devel@gnu.org
> 
> Summary of pending issues in this thread:
> 
>  - emacsclient -t causes a crash / assertion error; filed as bug#1325.

That's fine; I'd like to try to fix this problem.

>  - emacsclient -c causes the same error, because emacsclient thinks
> that "-c" + null DISPLAY => "-t"; I proposed the attached patch (1),
> because even if -t is supported on Windows, the assumption that an
> empty DISPLAY implies a tty frame is false.

Then why is the patch Windows-specific?  If the above assumption is
false, it shouldn't be made on Posix platforms as well.

Can someone explain why this assumption is in the code?

>  - (make-frame-on-display nil ...) is a synonym of (make-frame ...),
> but only on Windows; I proposed patch (2). Does anyone see a problem
> with it?

What happens if emacsclient is invoked with --display=DISPLAY argument
on Windows, after your patch?

>  - Regardless of whether the previous point is accepted or not, the
> new behavior should be documented.

Please add an entry to NEWS, so we won't forget that.

Thanks.




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

* Re: emacsclient's option decoding code
  2008-11-11  4:09                                 ` Eli Zaretskii
@ 2008-11-11  4:14                                   ` Chong Yidong
  2008-11-11 21:16                                     ` Eli Zaretskii
  2008-11-11  9:39                                   ` Juanma Barranquero
  1 sibling, 1 reply; 67+ messages in thread
From: Chong Yidong @ 2008-11-11  4:14 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Juanma Barranquero, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>>  - emacsclient -c causes the same error, because emacsclient thinks
>> that "-c" + null DISPLAY => "-t"; I proposed the attached patch (1),
>> because even if -t is supported on Windows, the assumption that an
>> empty DISPLAY implies a tty frame is false.
>
> Then why is the patch Windows-specific?  If the above assumption is
> false, it shouldn't be made on Posix platforms as well.

The `-c' option means "create a new frame".  On Posix platforms, we
default to creating an X frame, but if no DISPLAY is available, then we
create a tty frame.  On Windows, presumably tty can't be used.




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

* Re: emacsclient's option decoding code
  2008-11-11  4:09                                 ` Eli Zaretskii
  2008-11-11  4:14                                   ` Chong Yidong
@ 2008-11-11  9:39                                   ` Juanma Barranquero
  2008-11-11 15:37                                     ` Juanma Barranquero
  2008-11-11 21:24                                     ` Eli Zaretskii
  1 sibling, 2 replies; 67+ messages in thread
From: Juanma Barranquero @ 2008-11-11  9:39 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: cyd, emacs-devel

On Tue, Nov 11, 2008 at 05:09, Eli Zaretskii <eliz@gnu.org> wrote:

>>  - emacsclient -c causes the same error, because emacsclient thinks
>> that "-c" + null DISPLAY => "-t"; I proposed the attached patch (1),
>> because even if -t is supported on Windows, the assumption that an
>> empty DISPLAY implies a tty frame is false.
>
> Then why is the patch Windows-specific?  If the above assumption is
> false, it shouldn't be made on Posix platforms as well.
>
> Can someone explain why this assumption is in the code?

Chong has already explained the assumption. And IMO that assumption is
correct for POSIX, but will never (for some values of "never", of
course) be for the Windows port, because even if we someday support X,
Windows and tty frames simultaneously in the same running Emacs
instance, it is a long shot to assume that any specific user will be
using X or have DISPLAY defined in their environment. On POSIX, having
DISPLAY defined is the norm; on Windows it will always be the
exception, unless things change drastically. -c for a Windows user is
much more likely to mean "another frame on my display" than "another
frame on some remote X display", and if it really is the latter, well,
then the probability of the user having DISPLAY set is much higher in
the first place. (Note, BTW, that emacsclient's help for -c talks just
about "a new frame", with no mention of DISPLAY.)

> What happens if emacsclient is invoked with --display=DISPLAY argument
> on Windows, after your patch?

-d/--display is not currently accepted on Windows. Once activated, and
without X display support:

- As it stands now, with Chong's patch, if would make a frame on the
current display.
- With my patch, it would throw the "Invalid display, no HOST:SERVER
or HOST:SERVER.SCREEN" error.

I think the behavior I propose is more consistent. Also, if we someday
implement X support on the Windows port, my patch would naturally
work, while the current one would have to be fixed.

IMHO the desire to fix the -c/-t problem on Windows has muddled the
waters a bit. As I said in a previous message, if we want
`make-frame-on-display' to allow `nil' to default to the current
frame, that seems sensible. But making an exception for Windows and
allowing "", or in fact any other value, string or otherwise, is ugly
and just a consequence of the fact that (frame-parameter nil 'display)
returns "" on Windows. The right thing to do IMHO is fix the places
where the Windows port calls make-frame-on-display, and make sure that
it passes nil when it really means "I don't have a DISPLAY".

> Please add an entry to NEWS, so we won't forget that.

I'll do as soon as we settle on a fix.

Thanks,

  Juanma




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

* Re: emacsclient's option decoding code
  2008-11-11  9:39                                   ` Juanma Barranquero
@ 2008-11-11 15:37                                     ` Juanma Barranquero
  2008-11-11 21:24                                     ` Eli Zaretskii
  1 sibling, 0 replies; 67+ messages in thread
From: Juanma Barranquero @ 2008-11-11 15:37 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: cyd, emacs-devel

On Tue, Nov 11, 2008 at 10:39, Juanma Barranquero <lekktu@gmail.com> wrote:

> But making an exception for Windows and
> allowing "", or in fact any other value, string or otherwise, is ugly
> and just a consequence of the fact that (frame-parameter nil 'display)
> returns "" on Windows.

AFAICS, the reason Windows frames contain (display . "") is that
`x-open-connection' does not accept DISPLAY == nil on Windows (even if
it would make more sense than ""), because w32_term_init uses DISPLAY
to initialize the terminal name.

The attached patch makes the following changes:

  Window specific:
   - allows `x-open-connection' to accept DISPLAY == nil
   - makes sure that the terminal name is correctly initialized even
if DISPLAY == nil
   - passes nil to `x-open-connection' from w32-initialize-window-system
   - allows "emacsclient --display=XXX"
   - does not force tty for empty DISPLAYs on emacsclient

  Generic:
   - allows DISPLAY == nil in `make-frame-on-display', meaning "current display"
   - does not err out in `server-create-window-system-frame' when
there's no DISPLAY

Consequences:

  Windows specific:
   - (frame-parameter nil 'display) => nil (instead of "")
   - emacsclient --display=WHATEVER => "ERROR*: Invalid display, not
HOST:SERVER or HOST:SERVER.SCREEN"
   - emacsclient --display=:0.0 => "ERROR*: Don't know how to create a
frame on window system x"
   - emacsclient -c  => works

  Generic:
   - (make-frame-on-display nil ...) => (make-frame ...)

From my (biased) POV, there's mostly advantages: better Windows / X
compatibility, fewer Windows-specific hacks, etc.

Disadvantages:

   - I'm not sure whether "emacsclient -c" with no DISPLAY will do
something different on non-Windows systems (I don't think so, but it'd
be better to try it)

   - Returning nil instead of "" for the 'display frame-parameter on
Windows is not back-compatible; could be a problem for code expecting
it to always be a string, but this is already false on ttys.

  Juanma




2008-11-11  Juanma Barranquero  <lekktu@gmail.com>

	* term/w32-win.el (w32-initialize-window-system): Pass nil as DISPLAY
	arg to `x-open-connection', not "".

	* frame.el (make-frame-on-display): Don't do anything special for
	Windows; accept a null DISPLAY meaning "use the current frame".

	* server.el (server-create-window-system-frame): Don't err out if
	DISPLAY is null.

2008-11-11  Juanma Barranquero  <lekktu@gmail.com>

	* w32fns.c (Fx_open_connection): Accept a null display.

	* w32term.c (w32_term_init): If display_name is nil, assign an
	empty string to the terminal name.

2008-11-11  Juanma Barranquero  <lekktu@gmail.com>

	* emacsclient.c (longopts): Accept "-display" on Windows.
	(decode_options): Don't force tty on Windows when there is no DISPLAY.


Index: lib-src/emacsclient.c
===================================================================
RCS file: /sources/emacs/emacs/lib-src/emacsclient.c,v
retrieving revision 1.140
diff -u -2 -r1.140 emacsclient.c
--- lib-src/emacsclient.c	2 Nov 2008 23:16:33 -0000	1.140
+++ lib-src/emacsclient.c	11 Nov 2008 09:44:07 -0000
@@ -165,7 +165,5 @@
 #endif
   { "server-file",	required_argument, NULL, 'f' },
-#ifndef WINDOWSNT
   { "display",	required_argument, NULL, 'd' },
-#endif
   { 0, 0, 0, 0 }
 };
@@ -575,7 +573,9 @@
     display = NULL;

+#ifndef WINDOWSNT
   /* If no display is available, new frames are tty frames.  */
   if (!current_frame && !display)
     tty = 1;
+#endif

   /* --no-wait implies --current-frame on ttys when there are file
Index: lisp/frame.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/frame.el,v
retrieving revision 1.289
diff -u -2 -r1.289 frame.el
--- lisp/frame.el	7 Nov 2008 14:52:04 -0000	1.289
+++ lisp/frame.el	11 Nov 2008 09:44:30 -0000
@@ -617,6 +617,5 @@
 	 (make-frame `((window-system . ns)
 		       (display . ,display) . ,parameters)))
-	((eq system-type 'windows-nt)
-	 ;; On Windows, ignore DISPLAY.
+	((null display)
 	 (make-frame parameters))
 	(t
Index: lisp/server.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/server.el,v
retrieving revision 1.174
diff -u -2 -r1.174 server.el
--- lisp/server.el	11 Nov 2008 10:51:37 -0000	1.174
+++ lisp/server.el	11 Nov 2008 11:52:32 -0000
@@ -634,6 +634,5 @@
                    (or display
                        (frame-parameter nil 'display)
-                       (getenv "DISPLAY")
-                       (error "Please specify display"))
+                       (getenv "DISPLAY"))
                    params)))
       (server-log (format "%s created" frame) proc)
Index: lisp/term/w32-win.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/term/w32-win.el,v
retrieving revision 1.105
diff -u -2 -r1.105 w32-win.el
--- lisp/term/w32-win.el	11 Aug 2008 01:23:07 -0000	1.105
+++ lisp/term/w32-win.el	11 Nov 2008 12:45:39 -0000
@@ -246,5 +246,5 @@
             (replace-regexp-in-string "[.*]" "-" (invocation-name))))

-  (x-open-connection "" x-command-line-resources
+  (x-open-connection nil x-command-line-resources
                      ;; Exit with a fatal error if this fails and we
                      ;; are the initial display
Index: src/w32fns.c
===================================================================
RCS file: /sources/emacs/emacs/src/w32fns.c,v
retrieving revision 1.349
diff -u -2 -r1.349 w32fns.c
--- src/w32fns.c	30 Oct 2008 01:27:07 -0000	1.349
+++ src/w32fns.c	11 Nov 2008 14:25:43 -0000
@@ -4931,5 +4931,6 @@
     return Qnil;

-  CHECK_STRING (display);
+  if (! NILP (display))
+    CHECK_STRING (display);
   if (! NILP (xrm_string))
     CHECK_STRING (xrm_string);
Index: src/w32term.c
===================================================================
RCS file: /sources/emacs/emacs/src/w32term.c,v
retrieving revision 1.308
diff -u -2 -r1.308 w32term.c
--- src/w32term.c	27 Oct 2008 22:20:27 -0000	1.308
+++ src/w32term.c	11 Nov 2008 15:14:49 -0000
@@ -6181,4 +6181,7 @@

   /* Set the name of the terminal. */
+  if (NILP (display_name))
+    display_name = empty_unibyte_string;
+
   terminal->name = (char *) xmalloc (SBYTES (display_name) + 1);
   strncpy (terminal->name, SDATA (display_name), SBYTES (display_name));




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

* Re: emacsclient's option decoding code
  2008-11-11  4:14                                   ` Chong Yidong
@ 2008-11-11 21:16                                     ` Eli Zaretskii
  0 siblings, 0 replies; 67+ messages in thread
From: Eli Zaretskii @ 2008-11-11 21:16 UTC (permalink / raw)
  To: Chong Yidong; +Cc: lekktu, emacs-devel

> From: Chong Yidong <cyd@stupidchicken.com>
> Cc: Juanma Barranquero <lekktu@gmail.com>,  emacs-devel@gnu.org
> Date: Mon, 10 Nov 2008 23:14:28 -0500
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >>  - emacsclient -c causes the same error, because emacsclient thinks
> >> that "-c" + null DISPLAY => "-t"; I proposed the attached patch (1),
> >> because even if -t is supported on Windows, the assumption that an
> >> empty DISPLAY implies a tty frame is false.
> >
> > Then why is the patch Windows-specific?  If the above assumption is
> > false, it shouldn't be made on Posix platforms as well.
> 
> The `-c' option means "create a new frame".  On Posix platforms, we
> default to creating an X frame, but if no DISPLAY is available, then we
> create a tty frame.  On Windows, presumably tty can't be used.

What do you mean by "tty can't be used"?  The Windows port does
support the -nw option, and `-c' could then connect to a -nw session,
no?




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

* Re: emacsclient's option decoding code
  2008-11-11  9:39                                   ` Juanma Barranquero
  2008-11-11 15:37                                     ` Juanma Barranquero
@ 2008-11-11 21:24                                     ` Eli Zaretskii
  2008-11-11 23:36                                       ` Juanma Barranquero
  2008-11-12  2:31                                       ` Stefan Monnier
  1 sibling, 2 replies; 67+ messages in thread
From: Eli Zaretskii @ 2008-11-11 21:24 UTC (permalink / raw)
  To: Juanma Barranquero; +Cc: cyd, emacs-devel

> Date: Tue, 11 Nov 2008 10:39:28 +0100
> From: "Juanma Barranquero" <lekktu@gmail.com>
> Cc: cyd@stupidchicken.com, emacs-devel@gnu.org
> 
> On Tue, Nov 11, 2008 at 05:09, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> >>  - emacsclient -c causes the same error, because emacsclient thinks
> >> that "-c" + null DISPLAY => "-t"; I proposed the attached patch (1),
> >> because even if -t is supported on Windows, the assumption that an
> >> empty DISPLAY implies a tty frame is false.
> >
> > Then why is the patch Windows-specific?  If the above assumption is
> > false, it shouldn't be made on Posix platforms as well.
> >
> > Can someone explain why this assumption is in the code?
> 
> Chong has already explained the assumption. And IMO that assumption is
> correct for POSIX

I can't say I see why such an assumption should be made for Posix,
either.  Why not ask the user to specify whether she wants a GUI frame
or a tty frame?

> I think the behavior I propose is more consistent. Also, if we someday
> implement X support on the Windows port, my patch would naturally
> work, while the current one would have to be fixed.

I'm okay with your patch, FWIW, but I don't like the #ifndef WINDOWSNT
part in emacsclient.c.  I 'd like the behavior to be the same on all
platforms, as much as possible.  In this case, "as much as possible"
means that emacsclient should be able on Windows to create a GUI frame
if a GUI Emacs is running and a tty frame if a tty Emacs is running.

Can we do that?

> IMHO the desire to fix the -c/-t problem on Windows has muddled the
> waters a bit. As I said in a previous message, if we want
> `make-frame-on-display' to allow `nil' to default to the current
> frame, that seems sensible. But making an exception for Windows and
> allowing "", or in fact any other value, string or otherwise, is ugly
> and just a consequence of the fact that (frame-parameter nil 'display)
> returns "" on Windows.

Would it be better to return "0.0" instead of nil or ""?




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

* Re: emacsclient's option decoding code
  2008-11-11 21:24                                     ` Eli Zaretskii
@ 2008-11-11 23:36                                       ` Juanma Barranquero
  2008-11-12  4:22                                         ` Eli Zaretskii
  2008-11-12 19:37                                         ` Eli Zaretskii
  2008-11-12  2:31                                       ` Stefan Monnier
  1 sibling, 2 replies; 67+ messages in thread
From: Juanma Barranquero @ 2008-11-11 23:36 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: cyd, emacs-devel

On Tue, Nov 11, 2008 at 22:24, Eli Zaretskii <eliz@gnu.org> wrote:

> I can't say I see why such an assumption should be made for Posix,
> either.  Why not ask the user to specify whether she wants a GUI frame
> or a tty frame?

That's the whole point of -t. -c plus no DISPLAY is just a corner
case, and defaulting to -t in this case is just a way of trying to be
helpful.

That said, I don't mind if that is removed.

> In this case, "as much as possible"
> means that emacsclient should be able on Windows to create a GUI frame
> if a GUI Emacs is running and a tty frame if a tty Emacs is running.
>
> Can we do that?

It already creates the right kind of frame. But still -t is needed, at
least on non-Windows emacsen which support X and tty frames
simultaneously.

> Would it be better to return "0.0" instead of nil or ""?

I don't like the idea of conflating Windows and X frames. If someday
Windows Emacs supports also X frames, what would it return for an X
frame on the default display?

             Juanma




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

* Re: emacsclient's option decoding code
  2008-11-11 21:24                                     ` Eli Zaretskii
  2008-11-11 23:36                                       ` Juanma Barranquero
@ 2008-11-12  2:31                                       ` Stefan Monnier
  2008-11-12  9:33                                         ` Juanma Barranquero
  2008-11-12 19:36                                         ` Eli Zaretskii
  1 sibling, 2 replies; 67+ messages in thread
From: Stefan Monnier @ 2008-11-12  2:31 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Juanma Barranquero, cyd, emacs-devel

>> Chong has already explained the assumption. And IMO that assumption is
>> correct for POSIX

> I can't say I see why such an assumption should be made for Posix,
> either.

Because it mimicks the behavior of Emacs itself (if you start it with
a $DISPLAY, it defaults to using an X11 frame and if there's no
$DISPLAY it uses the tty).

> platforms, as much as possible.  In this case, "as much as possible"
> means that emacsclient should be able on Windows to create a GUI frame
> if a GUI Emacs is running and a tty frame if a tty Emacs is running.

That could make sense for -c indeed, altough it's not what happens under
X11: under X11 a "-c" with no $DISPLAY is equivalent to -t, i.e. it
uses the tty from where "emacsclient" is run, rather than using the tty
where the Emacs server is currently running.

> Would it be better to return "0.0" instead of nil or ""?

I don't like using nil for "the w32 window system", indeed.  I think the
"" we currently use is better.  Can't we just make make-frame-on-display
accept an argument "" to mean "the w32 window system"?


        Stefan




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

* Re: emacsclient's option decoding code
  2008-11-11 23:36                                       ` Juanma Barranquero
@ 2008-11-12  4:22                                         ` Eli Zaretskii
  2008-11-12  9:41                                           ` Juanma Barranquero
  2008-11-12 19:37                                         ` Eli Zaretskii
  1 sibling, 1 reply; 67+ messages in thread
From: Eli Zaretskii @ 2008-11-12  4:22 UTC (permalink / raw)
  To: Juanma Barranquero; +Cc: cyd, emacs-devel

> Date: Wed, 12 Nov 2008 00:36:17 +0100
> From: "Juanma Barranquero" <lekktu@gmail.com>
> Cc: cyd@stupidchicken.com, emacs-devel@gnu.org
> 
> If someday Windows Emacs supports also X frames, what would it
> return for an X frame on the default display?

":0.0", I think.  And that's what I wanted Emacs to do on Windows.




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

* Re: emacsclient's option decoding code
  2008-11-12  2:31                                       ` Stefan Monnier
@ 2008-11-12  9:33                                         ` Juanma Barranquero
  2008-11-12 15:56                                           ` Stefan Monnier
  2008-11-12 19:36                                         ` Eli Zaretskii
  1 sibling, 1 reply; 67+ messages in thread
From: Juanma Barranquero @ 2008-11-12  9:33 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Eli Zaretskii, cyd, emacs-devel

On Wed, Nov 12, 2008 at 03:31, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> I don't like using nil for "the w32 window system", indeed.  I think the
> "" we currently use is better.

It's not really important, but I don't understand why "" would be
better. At the "Basic Parameters" node, it is said:

  `display'
       The display on which to open this frame.  It should be a string of
       the form `"HOST:DPY.SCREEN"', just like the `DISPLAY' environment
       variable.

At no point in the docs is the value of DISPLAY assumed to be other
than "HOST:DPY.SCREEN", or else incorrect.

Also, nil is a more meaningful value to say that we don't have a
DISPLAY value (which is what's happening in this case, but could
change if we ever support X frames on Windows Emacs).

We're not talking about a frame parameter that does offer information
about the kind of display we're connecting to (not even `display-type'
does so, alas). `display' is purposefully modeled on DISPLAY. With
insight, it seems like there should perhaps be another frame parameter
that explicitly said whether the frame is X, tty, Windows or whatever.
Code could check that and not make unwarranted suppositions about
`display'.

> Can't we just make make-frame-on-display
> accept an argument "" to mean "the w32 window system"?

Yes, at the cost of adding Windows specific stuff, either the current

	((eq system-type 'windows-nt)
	 ;; On Windows, ignore DISPLAY.
	 (make-frame parameters))

which I deeply dislike because it accepts *everything*, or the still more ugly

	((and (eq system-type 'windows-nt) (string= display ""))
	 ;; On Windows, ignore DISPLAY == ""
	 (make-frame parameters))

And that's just in make-frame-on-display. If we're going to treat ""
as a "display" on Windows, perhaps other code would have to be
"generalized" that way. With my proposal, it becomes

	((null display)
	 (make-frame parameters))

and the generalization of nil meaning "the current display" for all
environments, not just Windows, seems more fitting.

  Juanma




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

* Re: emacsclient's option decoding code
  2008-11-12  4:22                                         ` Eli Zaretskii
@ 2008-11-12  9:41                                           ` Juanma Barranquero
  2008-11-12 19:14                                             ` Eli Zaretskii
  0 siblings, 1 reply; 67+ messages in thread
From: Juanma Barranquero @ 2008-11-12  9:41 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: cyd, emacs-devel

On Wed, Nov 12, 2008 at 05:22, Eli Zaretskii <eliz@gnu.org> wrote:

> ":0.0", I think.

If I understand you right, you propose

 "0.0" for Windows frames
 ":0.0" for X frames on the default display (when, if ever, they are
implemented)

Well, obviously that works, but it has two downsides:

 - "0.0" will have to be checked specially, which adds more
Windows-specific code, not less
 - "0.0" is not particularly meaningful, other than as a take on
DISPLAY. On tty Emacs frames, `display' is nil. Why would it be "0.0"
on Windows frames? They're no more X than tty frames are.

  Juanma




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

* Re: emacsclient's option decoding code
  2008-11-12  9:33                                         ` Juanma Barranquero
@ 2008-11-12 15:56                                           ` Stefan Monnier
  2008-11-12 16:37                                             ` Juanma Barranquero
  0 siblings, 1 reply; 67+ messages in thread
From: Stefan Monnier @ 2008-11-12 15:56 UTC (permalink / raw)
  To: Juanma Barranquero; +Cc: Eli Zaretskii, cyd, emacs-devel

>> I don't like using nil for "the w32 window system", indeed.  I think the
>> "" we currently use is better.
> It's not really important, but I don't understand why "" would be
> better.

You might be right.  I tend to think of `display' as something similar
to `terminal', but they're not the same.  To tell you the truth, I'm not
sure what `display' is meant for exactly.

E.g., let's imagine a w32 version of Emacs which has been hacked to be
able to use X11 as well.  And let's additionally say that it can even
also use GNUstep, so it can have w32, x11, and ns frames, all displayed
on the same screen.  Both ns and x11 frames will have a `display' set to
the same value (typically ":0.0" or something along these lines),
whereas the w32 frames's display will be "" (or nil, or ...).
This discrepency doesn't seem good.

So maybe we should just make w32 use display=":0.0".

> and the generalization of nil meaning "the current display" for all
> environments, not just Windows, seems more fitting.

The current display should be obtained via (frame-parameter nil 'display).


        Stefan




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

* Re: emacsclient's option decoding code
  2008-11-12 15:56                                           ` Stefan Monnier
@ 2008-11-12 16:37                                             ` Juanma Barranquero
  2008-11-12 17:13                                               ` Stefan Monnier
                                                                 ` (2 more replies)
  0 siblings, 3 replies; 67+ messages in thread
From: Juanma Barranquero @ 2008-11-12 16:37 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Eli Zaretskii, cyd, emacs-devel

On Wed, Nov 12, 2008 at 16:56, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> You might be right.  I tend to think of `display' as something similar
> to `terminal', but they're not the same.  To tell you the truth, I'm not
> sure what `display' is meant for exactly.

Same here. I find weird (though perhaps understandable) that Emacs has
a clear separation between tty and graphics frames, but not such
between X and w32 (and ns, etc.) frames. By that I mean that there is
code which calls x-functions when compiled on GNU/Linux, and their
w32-* counterparts when compiled on Windows. That will be a problem if
someday we have a multi-GUI Emacs.

> E.g., let's imagine a w32 version of Emacs which has been hacked to be
> able to use X11 as well.  And let's additionally say that it can even
> also use GNUstep, so it can have w32, x11, and ns frames, all displayed
> on the same screen.  Both ns and x11 frames will have a `display' set to
> the same value (typically ":0.0" or something along these lines),
> whereas the w32 frames's display will be "" (or nil, or ...).
> This discrepency doesn't seem good.
>
> So maybe we should just make w32 use display=":0.0".

Fine by me. Someday, if multi-GUI materializes, we'll have to add a
frame parameter or another way for lisp code to query the GUI
associated with a given frame.

> The current display should be obtained via (frame-parameter nil 'display).

Yes. My doubt was whether "current display" makes as much sense for
Windows frames as it does for X ones.

But I don't want to insist; I'd be happy just by fixing some of the
issues, one way or another.

So, please answer these:

 - Should the "-c + no DISPLAY" => "-t" implication of emacsclient be
removed only for Windows (because it does in fact make sense on
POSIX), or be removed for good (Eli's view, I think)?

 - Do I change term/w32-win.el to use ":0.0" as argument to `x-open-connection'?

 - If yes, do we leave `make-frame-on-display' as it stands now, i.e.
accepting any DISPLAY string as valid when called from Windows, or do
we change it to accept just ":0.0" for the time being? (I favor the
second, which is simpler and cleaner: it's just removing the recent
three-line patch by Chong.)

 - Do we want `make-frame-on-display' to accept a null DISPLAY (on
every platform) to mean "create a frame on the current display"?

  Juanma




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

* Re: emacsclient's option decoding code
  2008-11-12 16:37                                             ` Juanma Barranquero
@ 2008-11-12 17:13                                               ` Stefan Monnier
  2008-11-13  1:16                                               ` Lennart Borgman
  2008-11-13  9:00                                               ` Juanma Barranquero
  2 siblings, 0 replies; 67+ messages in thread
From: Stefan Monnier @ 2008-11-12 17:13 UTC (permalink / raw)
  To: Juanma Barranquero; +Cc: Eli Zaretskii, cyd, emacs-devel

> Fine by me. Someday, if multi-GUI materializes, we'll have to add a
> frame parameter or another way for lisp code to query the GUI
> associated with a given frame.

   framep is a built-in function in `C source code'.
   
   (framep object)
   
   Return non-nil if object is a frame.
   Value is t for a termcap frame (a character-only terminal),
   `x' for an Emacs frame that is really an X window,
   `w32' for an Emacs frame that is a window on MS-Windows display,
   `ns' for an Emacs frame on a GNUstep or Macintosh Cocoa display,
   `pc' for a direct-write MS-DOS frame.
   See also `frame-live-p'.

>  - Should the "-c + no DISPLAY" => "-t" implication of emacsclient be
> removed only for Windows (because it does in fact make sense on
> POSIX), or be removed for good (Eli's view, I think)?

It's there for Unix where it makes sense.  We can remove it for Windows
until we know how to implement -t there.

>  - Do I change term/w32-win.el to use ":0.0" as argument to
>  `x-open-connection'?

I'd leave it as "" for now.

>  - If yes, do we leave `make-frame-on-display' as it stands now, i.e.
> accepting any DISPLAY string as valid when called from Windows, or do
> we change it to accept just ":0.0" for the time being? (I favor the
> second, which is simpler and cleaner: it's just removing the recent
> three-line patch by Chong.)

Yes, we should change it to only accept "", tho I'm not sure if it will
re-introduce the bug that was fixed by Chong's patch.

>  - Do we want `make-frame-on-display' to accept a null DISPLAY (on
> every platform) to mean "create a frame on the current display"?

I don't see any reason why we'd want that since
(frame-parameter nil 'display) already works just fine for that purpose
and says clearly what it is doing.


        Stefan




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

* Re: emacsclient's option decoding code
  2008-11-12  9:41                                           ` Juanma Barranquero
@ 2008-11-12 19:14                                             ` Eli Zaretskii
  2008-11-12 21:05                                               ` Juanma Barranquero
  0 siblings, 1 reply; 67+ messages in thread
From: Eli Zaretskii @ 2008-11-12 19:14 UTC (permalink / raw)
  To: Juanma Barranquero; +Cc: cyd, emacs-devel

> Date: Wed, 12 Nov 2008 10:41:08 +0100
> From: "Juanma Barranquero" <lekktu@gmail.com>
> Cc: cyd@stupidchicken.com, emacs-devel@gnu.org
> 
> On Wed, Nov 12, 2008 at 05:22, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > ":0.0", I think.
> 
> If I understand you right, you propose
> 
>  "0.0" for Windows frames
>  ":0.0" for X frames on the default display (when, if ever, they are
> implemented)

No, I meant ":0.0" for Windows as well, I just omitted the colon by
mistake.

But see my other mail in this thread with a somewhat different concept
of resolving these issues.

> Well, obviously that works, but it has two downsides:
> 
>  - "0.0" will have to be checked specially

"checked specially" where?

>  - "0.0" is not particularly meaningful, other than as a take on
> DISPLAY. On tty Emacs frames, `display' is nil. Why would it be "0.0"
> on Windows frames? They're no more X than tty frames are.

I thought that uniform behavior in common use-cases could make
maintenance easier.




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

* Re: emacsclient's option decoding code
  2008-11-12  2:31                                       ` Stefan Monnier
  2008-11-12  9:33                                         ` Juanma Barranquero
@ 2008-11-12 19:36                                         ` Eli Zaretskii
  2008-11-12 20:56                                           ` Stefan Monnier
  2008-11-13 20:03                                           ` Juri Linkov
  1 sibling, 2 replies; 67+ messages in thread
From: Eli Zaretskii @ 2008-11-12 19:36 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: lekktu, cyd, emacs-devel

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Juanma Barranquero <lekktu@gmail.com>,  cyd@stupidchicken.com,  emacs-devel@gnu.org
> Date: Tue, 11 Nov 2008 21:31:59 -0500
> 
> >> Chong has already explained the assumption. And IMO that assumption is
> >> correct for POSIX
> 
> > I can't say I see why such an assumption should be made for Posix,
> > either.
> 
> Because it mimicks the behavior of Emacs itself (if you start it with
> a $DISPLAY, it defaults to using an X11 frame and if there's no
> $DISPLAY it uses the tty).

But this is guesswork at best, and cannot possibly DTRT in every
possible use-case, because emacsclient have no way of knowing what the
Emacs server can or cannot do.  For example, the server could have
been built with no X11 support, or the "current terminal" specified by
the --tty option could be unavailable.  Or you could be talking to a
remote Emacs, in which case there's no reason to believe the client
will know anything about $DISPLAY on the remote machine.

IOW, unlike with launching Emacs itself, where we always know what we
can or cannot do (since it's the same single program), when
emacsclient is invoked, it currently tries to second-guess
capabilities of another program, and there just aren't any good means
to do that reliably.

The upshot is, or so it seems, that there's no reliable way of making
sure the client's request will be served as well as possible.

So I propose a different strategy: given a specific request from a
client, let's have the server try to serve the request in the best
possible way it can.  For example:

  . if --display=DISPLAY was specified, try the given DISPLAY, if that
    fails, try ":0.0", if that fails, try using a tty;

  . if -c was specified, try creating a GUI frame, and failing that,
    try a tty frame;

  . if --tty was specified, try make-frame-on-tty, if it fails, try
    using the current tty or a 	GUI frame.

This could be implemented simply by catching errors thrown by
make-frame-on-tty etc. and falling back on safer methods as outlined
above.

The advantages are that (1) it will work more reliably (basically, if
some kind of frame is possible, it will be used); (2) server.el does
not need to be littered by ugly system-dependent conditions, it could
assume Posix semantics and let less-capable platforms use the first
supported fallback; and (3) emacsclient's code that parses the
command-line options can be much simpler, since it needs not guess
what Emacs can or cannot do.

> > platforms, as much as possible.  In this case, "as much as possible"
> > means that emacsclient should be able on Windows to create a GUI frame
> > if a GUI Emacs is running and a tty frame if a tty Emacs is running.
> 
> That could make sense for -c indeed, altough it's not what happens under
> X11: under X11 a "-c" with no $DISPLAY is equivalent to -t, i.e. it
> uses the tty from where "emacsclient" is run, rather than using the tty
> where the Emacs server is currently running.

We could try to use emacsclient's tty, and if that fails, fall back on
the one where Emacs is running.

> > Would it be better to return "0.0" instead of nil or ""?
> 
> I don't like using nil for "the w32 window system", indeed.  I think the
> "" we currently use is better.  Can't we just make make-frame-on-display
> accept an argument "" to mean "the w32 window system"?

With my suggestion, none of this trickery is needed.




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

* Re: emacsclient's option decoding code
  2008-11-11 23:36                                       ` Juanma Barranquero
  2008-11-12  4:22                                         ` Eli Zaretskii
@ 2008-11-12 19:37                                         ` Eli Zaretskii
  1 sibling, 0 replies; 67+ messages in thread
From: Eli Zaretskii @ 2008-11-12 19:37 UTC (permalink / raw)
  To: Juanma Barranquero; +Cc: cyd, emacs-devel

> Date: Wed, 12 Nov 2008 00:36:17 +0100
> From: "Juanma Barranquero" <lekktu@gmail.com>
> Cc: cyd@stupidchicken.com, emacs-devel@gnu.org
> 
> On Tue, Nov 11, 2008 at 22:24, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > I can't say I see why such an assumption should be made for Posix,
> > either.  Why not ask the user to specify whether she wants a GUI frame
> > or a tty frame?
> 
> That's the whole point of -t. -c plus no DISPLAY is just a corner
> case, and defaulting to -t in this case is just a way of trying to be
> helpful.

IMO, this logic should be on the server side, not the client side.
The server knows best how to be helpful, because it knows its
capabilities.




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

* Re: emacsclient's option decoding code
  2008-11-12 19:36                                         ` Eli Zaretskii
@ 2008-11-12 20:56                                           ` Stefan Monnier
  2008-11-13  4:10                                             ` Eli Zaretskii
  2008-11-13 20:03                                           ` Juri Linkov
  1 sibling, 1 reply; 67+ messages in thread
From: Stefan Monnier @ 2008-11-12 20:56 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: lekktu, cyd, emacs-devel

> But this is guesswork at best, and cannot possibly DTRT in every
> possible use-case, because emacsclient have no way of knowing what the
> Emacs server can or cannot do.

Some of the decisions could benefit from being moved to/from the client
from/to the server, indeed.
But returning an error if the user requests something that can't be done
is often better than trying to be clever.

>   . if --display=DISPLAY was specified, try the given DISPLAY, if that
>     fails, try ":0.0", if that fails, try using a tty;

:0.0 may be miles away from the user's eyes, so if $DISPLAY fails,
better signal an error and let the user manually switch to :0.0 if
that's what he wants to do.

>   . if -c was specified, try creating a GUI frame, and failing that,
>     try a tty frame;

That's what the "fallback to -t if $DISPLAY is empty" does.

>   . if --tty was specified, try make-frame-on-tty, if it fails, try
>     using the current tty or a GUI frame.

Again, the current tty (or any other current terminal/frame) may be
miles away from the user's eyes.

> We could try to use emacsclient's tty, and if that fails, fall back on
> the one where Emacs is running.

It might be OK to provide an option to do that, but silently doing so
without the user's constent may be worse than signalling an error and
letting *her* switch to the appropriate tty.


        Stefan




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

* Re: emacsclient's option decoding code
  2008-11-12 19:14                                             ` Eli Zaretskii
@ 2008-11-12 21:05                                               ` Juanma Barranquero
  2008-11-13  4:12                                                 ` Eli Zaretskii
  0 siblings, 1 reply; 67+ messages in thread
From: Juanma Barranquero @ 2008-11-12 21:05 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: cyd, emacs-devel

On Wed, Nov 12, 2008 at 20:14, Eli Zaretskii <eliz@gnu.org> wrote:

>>  - "0.0" will have to be checked specially
>
> "checked specially" where?

Currently, ":0.0" would work for Windows because of the
Windows-specific "any DISPLAY string is valid" code. Removing that,
":0.0" would be passed to the following code:

	(t
	 (unless (string-match-p "\\`[^:]*:[0-9]+\\(\\.[0-9]+\\)?\\'" display)
	   (error "Invalid display, not HOST:SERVER or HOST:SERVER.SCREEN"))
	 (when (and (boundp 'x-initialized) (not x-initialized))
	   (setq x-display-name display)
	   (x-initialize-window-system))
	 (make-frame `((window-system . x)
		       (display . ,display) . ,parameters)))))

and it would fail for Windows. At some point or other, you've gonna
add a check for Windows, because, really, ":0.0" has no meaning right
now.

> I thought that uniform behavior in common use-cases could make
> maintenance easier.

I agree.  But the things we've been discussing until now aren't more
uniform IMO; they just move the Windows specific code around or
introduce ad-hoc hacks to force Windows frames to simulate being what
they are not...

  Juanma




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

* Re: emacsclient's option decoding code
  2008-11-12 16:37                                             ` Juanma Barranquero
  2008-11-12 17:13                                               ` Stefan Monnier
@ 2008-11-13  1:16                                               ` Lennart Borgman
  2008-11-13  4:18                                                 ` Eli Zaretskii
  2008-11-13  9:00                                               ` Juanma Barranquero
  2 siblings, 1 reply; 67+ messages in thread
From: Lennart Borgman @ 2008-11-13  1:16 UTC (permalink / raw)
  To: Juanma Barranquero; +Cc: cyd, Eli Zaretskii, Stefan Monnier, emacs-devel

On Wed, Nov 12, 2008 at 5:37 PM, Juanma Barranquero <lekktu@gmail.com> wrote:
>> E.g., let's imagine a w32 version of Emacs which has been hacked to be
>> able to use X11 as well.  And let's additionally say that it can even
>> also use GNUstep, so it can have w32, x11, and ns frames, all displayed
>> on the same screen.  Both ns and x11 frames will have a `display' set to
>> the same value (typically ":0.0" or something along these lines),
>> whereas the w32 frames's display will be "" (or nil, or ...).
>> This discrepency doesn't seem good.
>>
>> So maybe we should just make w32 use display=":0.0".
>
> Fine by me. Someday, if multi-GUI materializes, we'll have to add a
> frame parameter or another way for lisp code to query the GUI
> associated with a given frame.

I do not understand this discussion at all, but is there anything
preventing using a value that has the string "w32" in it instead of
just ":0.0"? It seems to me that would be easier to understand.




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

* Re: emacsclient's option decoding code
  2008-11-12 20:56                                           ` Stefan Monnier
@ 2008-11-13  4:10                                             ` Eli Zaretskii
  2008-11-13  4:29                                               ` mail
  0 siblings, 1 reply; 67+ messages in thread
From: Eli Zaretskii @ 2008-11-13  4:10 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: lekktu, cyd, emacs-devel

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: lekktu@gmail.com,  cyd@stupidchicken.com,  emacs-devel@gnu.org
> Date: Wed, 12 Nov 2008 15:56:25 -0500
> 
> > But this is guesswork at best, and cannot possibly DTRT in every
> > possible use-case, because emacsclient have no way of knowing what the
> > Emacs server can or cannot do.
> 
> Some of the decisions could benefit from being moved to/from the client
> from/to the server, indeed.
> But returning an error if the user requests something that can't be done
> is often better than trying to be clever.

I don't see why erroring out is better.

> >   . if --display=DISPLAY was specified, try the given DISPLAY, if that
> >     fails, try ":0.0", if that fails, try using a tty;
> 
> :0.0 may be miles away from the user's eyes, so if $DISPLAY fails,
> better signal an error and let the user manually switch to :0.0 if
> that's what he wants to do.

If :0.0 is miles away, there's no difference between an error and
display miles away: in both cases, the user needs to fix something and
reinvoke.  What I suggest is better because if :0.0 is NOT miles away,
it does TRT, while erroring does not.

> It might be OK to provide an option to do that, but silently doing so
> without the user's constent may be worse than signalling an error and
> letting *her* switch to the appropriate tty.

We do it today already, with the kind of guesswork we have in
emacsclient.




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

* Re: emacsclient's option decoding code
  2008-11-12 21:05                                               ` Juanma Barranquero
@ 2008-11-13  4:12                                                 ` Eli Zaretskii
  2008-11-13  8:34                                                   ` Juanma Barranquero
  0 siblings, 1 reply; 67+ messages in thread
From: Eli Zaretskii @ 2008-11-13  4:12 UTC (permalink / raw)
  To: Juanma Barranquero; +Cc: cyd, emacs-devel

> Date: Wed, 12 Nov 2008 22:05:11 +0100
> From: "Juanma Barranquero" <lekktu@gmail.com>
> Cc: cyd@stupidchicken.com, emacs-devel@gnu.org
> 
> 	(t
> 	 (unless (string-match-p "\\`[^:]*:[0-9]+\\(\\.[0-9]+\\)?\\'" display)
> 	   (error "Invalid display, not HOST:SERVER or HOST:SERVER.SCREEN"))
> 	 (when (and (boundp 'x-initialized) (not x-initialized))
> 	   (setq x-display-name display)
> 	   (x-initialize-window-system))
> 	 (make-frame `((window-system . x)
> 		       (display . ,display) . ,parameters)))))
> 
> and it would fail for Windows. At some point or other, you've gonna
> add a check for Windows, because, really, ":0.0" has no meaning right
> now.

I meant for it to be checked inside a w32 specific code of make-frame.

> I agree.  But the things we've been discussing until now aren't more
> uniform IMO; they just move the Windows specific code around or
> introduce ad-hoc hacks to force Windows frames to simulate being what
> they are not...

I suggested a different approach, but it sounds like it's being
rejected.




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

* Re: emacsclient's option decoding code
  2008-11-13  1:16                                               ` Lennart Borgman
@ 2008-11-13  4:18                                                 ` Eli Zaretskii
  0 siblings, 0 replies; 67+ messages in thread
From: Eli Zaretskii @ 2008-11-13  4:18 UTC (permalink / raw)
  To: Lennart Borgman; +Cc: lekktu, cyd, monnier, emacs-devel

> Date: Thu, 13 Nov 2008 02:16:20 +0100
> From: "Lennart Borgman" <lennart.borgman@gmail.com>
> Cc: cyd@stupidchicken.com, Eli Zaretskii <eliz@gnu.org>,
> 	Stefan Monnier <monnier@iro.umontreal.ca>, emacs-devel@gnu.org
> 
> I do not understand this discussion at all, but is there anything
> preventing using a value that has the string "w32" in it instead of
> just ":0.0"?

Yes: it has all of the disadvantages of ":0.0" and none of the
advantages.

The issue is not whether the string is understandable by humans, the
issue is whether existing Lisp code will live with the string better.




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

* Re: emacsclient's option decoding code
  2008-11-13  4:10                                             ` Eli Zaretskii
@ 2008-11-13  4:29                                               ` mail
  2008-11-13 20:54                                                 ` Eli Zaretskii
  0 siblings, 1 reply; 67+ messages in thread
From: mail @ 2008-11-13  4:29 UTC (permalink / raw)
  To: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Stefan Monnier <monnier@iro.umontreal.ca>
>> 
>> :0.0 may be miles away from the user's eyes, so if $DISPLAY fails,
>> better signal an error and let the user manually switch to :0.0 if
>> that's what he wants to do.
>
> If :0.0 is miles away, there's no difference between an error and
> display miles away: in both cases, the user needs to fix something and
> reinvoke.  What I suggest is better because if :0.0 is NOT miles away,
> it does TRT, while erroring does not.
>

if :0.0 is miles away, the user almost certainly *doesn't* want to pop
up random windows on it. On the other hand, if it's right next to him,
he can quite easily set DISPLAY and try again. If the environment of
emacsclient doesn't include a DISPLAY, then emacsclient should not
arbitrarily use another DISPLAY.

-- 
The steady state of disks is full.
		-- Ken Thompson





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

* Re: emacsclient's option decoding code
  2008-11-13  4:12                                                 ` Eli Zaretskii
@ 2008-11-13  8:34                                                   ` Juanma Barranquero
  0 siblings, 0 replies; 67+ messages in thread
From: Juanma Barranquero @ 2008-11-13  8:34 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: cyd, emacs-devel

On Thu, Nov 13, 2008 at 05:12, Eli Zaretskii <eliz@gnu.org> wrote:

> I suggested a different approach, but it sounds like it's being
> rejected.

Honestly, at this point I don't really know what is being proposed
anymore or whether some kind of agreement has been reached at all...

  Juanma




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

* Re: emacsclient's option decoding code
  2008-11-12 16:37                                             ` Juanma Barranquero
  2008-11-12 17:13                                               ` Stefan Monnier
  2008-11-13  1:16                                               ` Lennart Borgman
@ 2008-11-13  9:00                                               ` Juanma Barranquero
  2008-11-13 15:45                                                 ` Dan Nicolaescu
  2 siblings, 1 reply; 67+ messages in thread
From: Juanma Barranquero @ 2008-11-13  9:00 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Eli Zaretskii, cyd, emacs-devel

On Wed, Nov 12, 2008 at 17:37, Juanma Barranquero <lekktu@gmail.com> wrote:

OK, let's try to clear somewhat the issues.

About these changes:

>  - Do I change term/w32-win.el to use ":0.0" as argument to `x-open-connection'?
>
>  - If yes, do we leave `make-frame-on-display' as it stands now, i.e.
> accepting any DISPLAY string as valid when called from Windows, or do
> we change it to accept just ":0.0" for the time being? (I favor the
> second, which is simpler and cleaner: it's just removing the recent
> three-line patch by Chong.)

The attached patch uses ":0.0" on Windows (which seems to be the
preferred answer) and *removes* Window-specific code in
`make-frame-on-display' (in fact, it removes also a tiny bit of
X-specific code).

It is still not possible to do "emacsclient -c my-file" on Windows
because of the -c/-t problem, but that will have to wait for some
consensus.

It is OK to install this?

  Juanma


2008-11-13  Juanma Barranquero  <lekktu@gmail.com>

	* frame.el (make-frame-on-display): Remove Windows-specific hack.
	Pass the current window-system to `make-frame', not a hard-coded `x'.

	* term/w32-win.el (w32-initialize-window-system): Use ":0.0" instead
	of "" to represent the Windows display.


Index: lisp/frame.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/frame.el,v
retrieving revision 1.289
diff -u -2 -r1.289 frame.el
--- lisp/frame.el	7 Nov 2008 14:52:04 -0000	1.289
+++ lisp/frame.el	13 Nov 2008 08:48:28 -0000
@@ -617,7 +617,4 @@
 	 (make-frame `((window-system . ns)
 		       (display . ,display) . ,parameters)))
-	((eq system-type 'windows-nt)
-	 ;; On Windows, ignore DISPLAY.
-	 (make-frame parameters))
 	(t
 	 (unless (string-match-p "\\`[^:]*:[0-9]+\\(\\.[0-9]+\\)?\\'" display)
@@ -626,5 +623,5 @@
 	   (setq x-display-name display)
 	   (x-initialize-window-system))
-	 (make-frame `((window-system . x)
+	 (make-frame `((window-system . ,window-system)
 		       (display . ,display) . ,parameters)))))

Index: lisp/term/w32-win.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/term/w32-win.el,v
retrieving revision 1.105
diff -u -2 -r1.105 w32-win.el
--- lisp/term/w32-win.el	11 Aug 2008 01:23:07 -0000	1.105
+++ lisp/term/w32-win.el	13 Nov 2008 08:17:25 -0000
@@ -246,5 +246,5 @@
             (replace-regexp-in-string "[.*]" "-" (invocation-name))))

-  (x-open-connection "" x-command-line-resources
+  (x-open-connection ":0.0" x-command-line-resources
                      ;; Exit with a fatal error if this fails and we
                      ;; are the initial display




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

* Re: emacsclient's option decoding code
  2008-11-13  9:00                                               ` Juanma Barranquero
@ 2008-11-13 15:45                                                 ` Dan Nicolaescu
  2008-11-13 16:05                                                   ` Juanma Barranquero
  0 siblings, 1 reply; 67+ messages in thread
From: Dan Nicolaescu @ 2008-11-13 15:45 UTC (permalink / raw)
  To: Juanma Barranquero; +Cc: cyd, Stefan Monnier, emacs-devel

"Juanma Barranquero" <lekktu@gmail.com> writes:

  > On Wed, Nov 12, 2008 at 17:37, Juanma Barranquero <lekktu@gmail.com> wrote:
  > 
  > OK, let's try to clear somewhat the issues.
  > 
  > About these changes:
  > 
  > >  - Do I change term/w32-win.el to use ":0.0" as argument to `x-open-connection'?
  > >
  > >  - If yes, do we leave `make-frame-on-display' as it stands now, i.e.
  > > accepting any DISPLAY string as valid when called from Windows, or do
  > > we change it to accept just ":0.0" for the time being? (I favor the
  > > second, which is simpler and cleaner: it's just removing the recent
  > > three-line patch by Chong.)
  > 
  > The attached patch uses ":0.0" on Windows (which seems to be the
  > preferred answer) and *removes* Window-specific code in
  > `make-frame-on-display' (in fact, it removes also a tiny bit of
  > X-specific code).
  > 
  > It is still not possible to do "emacsclient -c my-file" on Windows
  > because of the -c/-t problem, but that will have to wait for some
  > consensus.
  > 
  > It is OK to install this?
  > 
  >   Juanma
  > 
  > 
  > 2008-11-13  Juanma Barranquero  <lekktu@gmail.com>
  > 
  > 	* frame.el (make-frame-on-display): Remove Windows-specific hack.
  > 	Pass the current window-system to `make-frame', not a hard-coded `x'.
  > 
  > 	* term/w32-win.el (w32-initialize-window-system): Use ":0.0" instead
  > 	of "" to represent the Windows display.
  > 
  > 
  > Index: lisp/frame.el
  > ===================================================================
  > RCS file: /sources/emacs/emacs/lisp/frame.el,v
  > retrieving revision 1.289
  > diff -u -2 -r1.289 frame.el
  > --- lisp/frame.el	7 Nov 2008 14:52:04 -0000	1.289
  > +++ lisp/frame.el	13 Nov 2008 08:48:28 -0000
  > @@ -617,7 +617,4 @@
  >  	 (make-frame `((window-system . ns)
  >  		       (display . ,display) . ,parameters)))
  > -	((eq system-type 'windows-nt)
  > -	 ;; On Windows, ignore DISPLAY.
  > -	 (make-frame parameters))
  >  	(t
  >  	 (unless (string-match-p "\\`[^:]*:[0-9]+\\(\\.[0-9]+\\)?\\'" display)
  > @@ -626,5 +623,5 @@
  >  	   (setq x-display-name display)
  >  	   (x-initialize-window-system))
  > -	 (make-frame `((window-system . x)
  > +	 (make-frame `((window-system . ,window-system)

This is suspicious.  Does it work when doing 
emacs -Q -nw -f server-start
emacsclient -c

Sorry, I haven't followed this long thread, so I don't know what
problems this is trying to fix.  But given where we are in the release
process, it would be good if changes to the X code would be tested
on X in all possible scenarios.




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

* Re: emacsclient's option decoding code
  2008-11-13 15:45                                                 ` Dan Nicolaescu
@ 2008-11-13 16:05                                                   ` Juanma Barranquero
  2008-11-13 16:05                                                     ` Juanma Barranquero
  2008-11-13 16:08                                                     ` Dan Nicolaescu
  0 siblings, 2 replies; 67+ messages in thread
From: Juanma Barranquero @ 2008-11-13 16:05 UTC (permalink / raw)
  To: Dan Nicolaescu; +Cc: cyd, Stefan Monnier, emacs-devel

On Thu, Nov 13, 2008 at 16:45, Dan Nicolaescu <dann@ics.uci.edu> wrote:

> This is suspicious.  Does it work when doing
> emacs -Q -nw -f server-start
> emacsclient -c

No, and I don't have a way to test it.

> Sorry, I haven't followed this long thread, so I don't know what
> problems this is trying to fix.  But given where we are in the release
> process, it would be good if changes to the X code would be tested
> on X in all possible scenarios.

Of course. Could you please apply the patch and try it?

  Juanma




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

* Re: emacsclient's option decoding code
  2008-11-13 16:05                                                   ` Juanma Barranquero
@ 2008-11-13 16:05                                                     ` Juanma Barranquero
  2008-11-13 16:08                                                     ` Dan Nicolaescu
  1 sibling, 0 replies; 67+ messages in thread
From: Juanma Barranquero @ 2008-11-13 16:05 UTC (permalink / raw)
  To: Dan Nicolaescu; +Cc: cyd, Stefan Monnier, emacs-devel

On Thu, Nov 13, 2008 at 17:05, Juanma Barranquero <lekktu@gmail.com> wrote:

> No, and I don't have a way to test it.

s/No/I don't know/

  Juanma




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

* Re: emacsclient's option decoding code
  2008-11-13 16:05                                                   ` Juanma Barranquero
  2008-11-13 16:05                                                     ` Juanma Barranquero
@ 2008-11-13 16:08                                                     ` Dan Nicolaescu
  1 sibling, 0 replies; 67+ messages in thread
From: Dan Nicolaescu @ 2008-11-13 16:08 UTC (permalink / raw)
  To: Juanma Barranquero; +Cc: cyd, Stefan Monnier, emacs-devel

"Juanma Barranquero" <lekktu@gmail.com> writes:

  > On Thu, Nov 13, 2008 at 16:45, Dan Nicolaescu <dann@ics.uci.edu> wrote:
  > 
  > > This is suspicious.  Does it work when doing
  > > emacs -Q -nw -f server-start
  > > emacsclient -c
  > 
  > No, and I don't have a way to test it.
  > 
  > > Sorry, I haven't followed this long thread, so I don't know what
  > > problems this is trying to fix.  But given where we are in the release
  > > process, it would be good if changes to the X code would be tested
  > > on X in all possible scenarios.
  > 
  > Of course. Could you please apply the patch and try it?

Sorry, I don't have time.




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

* Re: emacsclient's option decoding code
  2008-11-12 19:36                                         ` Eli Zaretskii
  2008-11-12 20:56                                           ` Stefan Monnier
@ 2008-11-13 20:03                                           ` Juri Linkov
  2008-11-13 20:40                                             ` mail
  1 sibling, 1 reply; 67+ messages in thread
From: Juri Linkov @ 2008-11-13 20:03 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: lekktu, cyd, Stefan Monnier, emacs-devel

>   . if -c was specified, try creating a GUI frame, and failing that,
>     try a tty frame;
>
>   . if --tty was specified, try make-frame-on-tty, if it fails, try
>     using the current tty or a GUI frame.

This looks mostly clear for me.  The only thing I don't understand is why
we need -c?  Emacs has no option for creating a GUI frame and has -nw for
running on a tty, so why should emacsclient be different?  The following
rules are more consistent with the traditional Emacs invocation:

  . if -nw was specified, try make-frame-on-tty, if it fails, try
    using the current tty or a GUI frame;

  . if no option specified, try creating a GUI frame, and failing that,
    try a tty frame.

for consistency with

  emacsclient            = emacs          (create a new GUI frame)
  emacsclient -nw        = emacs -nw      (run emacs on a tty)
  emacsclient FILE       = emacs FILE     (open a file in a new GUI frame)
  emacsclient -nw FILE   = emacs -nw FILE (open a file in a tty)

-- 
Juri Linkov
http://www.jurta.org/emacs/




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

* Re: emacsclient's option decoding code
  2008-11-13 20:03                                           ` Juri Linkov
@ 2008-11-13 20:40                                             ` mail
  2008-11-13 21:08                                               ` Thorsten Bonow
  2008-11-13 22:01                                               ` Thorsten Bonow
  0 siblings, 2 replies; 67+ messages in thread
From: mail @ 2008-11-13 20:40 UTC (permalink / raw)
  To: emacs-devel

Juri Linkov <juri@jurta.org> writes:

>>   . if -c was specified, try creating a GUI frame, and failing that,
>>     try a tty frame;
>>
>>   . if --tty was specified, try make-frame-on-tty, if it fails, try
>>     using the current tty or a GUI frame.
>
> This looks mostly clear for me.  The only thing I don't understand is why
> we need -c?  Emacs has no option for creating a GUI frame and has -nw for
> running on a tty, so why should emacsclient be different?  The following
> rules are more consistent with the traditional Emacs invocation:
>
>   . if -nw was specified, try make-frame-on-tty, if it fails, try
>     using the current tty or a GUI frame;
>
>   . if no option specified, try creating a GUI frame, and failing that,
>     try a tty frame.
>
> for consistency with
>
>   emacsclient            = emacs          (create a new GUI frame)
>   emacsclient -nw        = emacs -nw      (run emacs on a tty)
>   emacsclient FILE       = emacs FILE     (open a file in a new GUI frame)
>   emacsclient -nw FILE   = emacs -nw FILE (open a file in a tty)

But emacsclient has to additionally support "use the existing frame",
which is what it does (and has always done) when no arguments are
specified.

-- 
No group of professionals meets except to conspire against the public at large.
		-- Mark Twain





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

* Re: emacsclient's option decoding code
  2008-11-13  4:29                                               ` mail
@ 2008-11-13 20:54                                                 ` Eli Zaretskii
  2008-11-13 21:27                                                   ` mail
  0 siblings, 1 reply; 67+ messages in thread
From: Eli Zaretskii @ 2008-11-13 20:54 UTC (permalink / raw)
  To: mail; +Cc: emacs-devel

> From: mail@justinbogner.com
> Date: Wed, 12 Nov 2008 21:29:46 -0700
> 
> if :0.0 is miles away, the user almost certainly *doesn't* want to pop
> up random windows on it. On the other hand, if it's right next to him,
> he can quite easily set DISPLAY and try again. If the environment of
> emacsclient doesn't include a DISPLAY, then emacsclient should not
> arbitrarily use another DISPLAY.

It's not just another DISPLAY, it's the default DISPLAY.

The general idea is, if you cannot do something fancy, do it less
fancy.




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

* Re: emacsclient's option decoding code
  2008-11-13 20:40                                             ` mail
@ 2008-11-13 21:08                                               ` Thorsten Bonow
  2008-11-13 22:01                                               ` Thorsten Bonow
  1 sibling, 0 replies; 67+ messages in thread
From: Thorsten Bonow @ 2008-11-13 21:08 UTC (permalink / raw)
  To: emacs-devel

>>>>> "mail" == mail  <mail@justinbogner.com> writes:

    mail> Juri Linkov <juri@jurta.org> writes:
    >>> . if -c was specified, try creating a GUI frame, and failing that, try a
    >>> tty frame;
    >>>
    >>> . if --tty was specified, try make-frame-on-tty, if it fails, try using
    >>> the current tty or a GUI frame.
    >>
    >> This looks mostly clear for me.  The only thing I don't understand is why
    >> we need -c?  Emacs has no option for creating a GUI frame and has -nw for
    >> running on a tty, so why should emacsclient be different?  The following
    >> rules are more consistent with the traditional Emacs invocation:
    >>
    >> . if -nw was specified, try make-frame-on-tty, if it fails, try using the
    >> current tty or a GUI frame;
    >>
    >> . if no option specified, try creating a GUI frame, and failing that, try
    >> a tty frame.
    >>
    >> for consistency with
    >>
    >> emacsclient = emacs (create a new GUI frame) emacsclient -nw = emacs -nw
    >> (run emacs on a tty) emacsclient FILE = emacs FILE (open a file in a new
    >> GUI frame) emacsclient -nw FILE = emacs -nw FILE (open a file in a tty)

    mail> But emacsclient has to additionally support "use the existing frame",
    mail> which is what it does (and has always done) when no arguments are
    mail> specified.

Hi,

I kid myself thinking that I asked for a "-c" option first, back in 2003 :-)

I would like to remind you of the discussion back than and my reasons for asking
for it. Removing the "--create-frame" option and make emacs behave as proposed
above would cripple my setup---again.

I'm quoting from "gnu/emacs client --create-frame somehow?"
(http://lists.gnu.org/archive/html/help-gnu-emacs/2003-12/msg00007.html):

>>>>> "Stefan" == Stefan Monnier <address@bogus.example.com> writes:

    >> What I'm looking for is some way of simulating a --create-frame
    >> option and then calling emacsclient/gnuclient --create-frame
    >> FILE

    Stefan> Since server.el and emacsclient.c do not offer any way to
    Stefan> add new options like --create-frame, your best bet is
    Stefan> probably something like

    Stefan>   emacsclient --eval '(find-file-other-frame "FILE")'

    Yes, this opens up a new frame, but then I am loosing the
    [+line-number] option of gnu/emacsclient. My problem started when
    writing with LaTeX in Emacs, previewing with xdvi: With the
    inverse search feature, switching between a line in the document
    and the preview of this part of your document is possible. So I
    wouldn't want to loose the line-number information or open up a new
    frame every time I switch. But in other cases---say when clicking
    on a "mailto:"-tag in my web-browser, I want a new frame for
    composing an email.

    Stefan> But maybe you have a point that it should be possible to
    Stefan> add new options, kind of like what is done with
    Stefan> `command-switch-alist'.

[...]

To put it in a nutshell, even under X, I sometimes want to let emacsclient create a
frame, but sometimes not.

Toto


-- 
Contact information and PGP key at
http://www.withouthat.org/~toto/contact.html

[...] the dangerous role of the troublemakers in history has
often fallen to the Jewish people. Anne Frankly, it should be
noted, in passing, that a great deal of good for the advancement
of mankind has been accomplished between circumcision, where they
cut off the tip of your dick, to crucifixion, where they throw
the whole Jew away.

Friedman, Kinky (1995), "God Bless John Wayne", Simon and
Shuster, New York




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

* Re: emacsclient's option decoding code
  2008-11-13 20:54                                                 ` Eli Zaretskii
@ 2008-11-13 21:27                                                   ` mail
  2008-11-13 22:30                                                     ` Eli Zaretskii
  0 siblings, 1 reply; 67+ messages in thread
From: mail @ 2008-11-13 21:27 UTC (permalink / raw)
  To: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: mail@justinbogner.com
>> Date: Wed, 12 Nov 2008 21:29:46 -0700
>> 
>> if :0.0 is miles away, the user almost certainly *doesn't* want to pop
>> up random windows on it. On the other hand, if it's right next to him,
>> he can quite easily set DISPLAY and try again. If the environment of
>> emacsclient doesn't include a DISPLAY, then emacsclient should not
>> arbitrarily use another DISPLAY.
>
> It's not just another DISPLAY, it's the default DISPLAY.
>
> The general idea is, if you cannot do something fancy, do it less
> fancy.
>

But the default DISPLAY *is* just another display. What if I run my
normal X session in a VNC? Then the "default" display may well be :10.0 and
:0.0 could be anyone's display. What if I've ssh'd into a computer where
somebody else is running X and tried to run emacsclient? Relying on me
not having enough permissions to create a window on somebody else's
display in order to prevent the wrong thing from happening seems a bit
iffy.

My point is that trying to decide on a reasonable situation to fall back
to depends on both the client's environment and the server's
environment, so trying to be too smart about it on either one is
dangerous.

-- 
share, n.:
	To give in, endure humiliation.





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

* Re: emacsclient's option decoding code
  2008-11-13 20:40                                             ` mail
  2008-11-13 21:08                                               ` Thorsten Bonow
@ 2008-11-13 22:01                                               ` Thorsten Bonow
  1 sibling, 0 replies; 67+ messages in thread
From: Thorsten Bonow @ 2008-11-13 22:01 UTC (permalink / raw)
  To: emacs-devel

>>>>> "mail" == mail  <mail@justinbogner.com> writes:

    mail> Juri Linkov <juri@jurta.org> writes:
    >>> . if -c was specified, try creating a GUI frame, and failing that, try a
    >>> tty frame;
    >>>
    >>> . if --tty was specified, try make-frame-on-tty, if it fails, try using
    >>> the current tty or a GUI frame.
    >>
    >> This looks mostly clear for me.  The only thing I don't understand is why
    >> we need -c?  Emacs has no option for creating a GUI frame and has -nw for
    >> running on a tty, so why should emacsclient be different?  The following
    >> rules are more consistent with the traditional Emacs invocation:
    >>
    >> . if -nw was specified, try make-frame-on-tty, if it fails, try using the
    >> current tty or a GUI frame;
    >>
    >> . if no option specified, try creating a GUI frame, and failing that, try
    >> a tty frame.
    >>
    >> for consistency with
    >>
    >> emacsclient = emacs (create a new GUI frame) emacsclient -nw = emacs -nw
    >> (run emacs on a tty) emacsclient FILE = emacs FILE (open a file in a new
    >> GUI frame) emacsclient -nw FILE = emacs -nw FILE (open a file in a tty)

    mail> But emacsclient has to additionally support "use the existing frame",
    mail> which is what it does (and has always done) when no arguments are
    mail> specified.

Hi,

I kid myself thinking that I asked for a "-c" option first, back in 2003 :-)

I would like to remind you of the discussion back than and my reasons for asking
for it. Removing the "--create-frame" option and make emacs behave as proposed
above would cripple my setup---again.

I'm quoting from "gnu/emacs client --create-frame somehow?"
(http://lists.gnu.org/archive/html/help-gnu-emacs/2003-12/msg00007.html):

>>>>> "Stefan" == Stefan Monnier <address@bogus.example.com> writes:

    >> What I'm looking for is some way of simulating a --create-frame
    >> option and then calling emacsclient/gnuclient --create-frame
    >> FILE

    Stefan> Since server.el and emacsclient.c do not offer any way to
    Stefan> add new options like --create-frame, your best bet is
    Stefan> probably something like

    Stefan>   emacsclient --eval '(find-file-other-frame "FILE")'

    Yes, this opens up a new frame, but then I am loosing the
    [+line-number] option of gnu/emacsclient. My problem started when
    writing with LaTeX in Emacs, previewing with xdvi: With the
    inverse search feature, switching between a line in the document
    and the preview of this part of your document is possible. So I
    wouldn't want to loose the line-number information or open up a new
    frame every time I switch. But in other cases---say when clicking
    on a "mailto:"-tag in my web-browser, I want a new frame for
    composing an email.

    Stefan> But maybe you have a point that it should be possible to
    Stefan> add new options, kind of like what is done with
    Stefan> `command-switch-alist'.

[...]

To put it in a nutshell, even under X, I sometimes want to let emacsclient create a
frame, but sometimes not.

Toto


-- 
Contact information and PGP key at
http://www.withouthat.org/~toto/contact.html

[...] the dangerous role of the troublemakers in history has
often fallen to the Jewish people. Anne Frankly, it should be
noted, in passing, that a great deal of good for the advancement
of mankind has been accomplished between circumcision, where they
cut off the tip of your dick, to crucifixion, where they throw
the whole Jew away.

Friedman, Kinky (1995), "God Bless John Wayne", Simon and
Shuster, New York




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

* Re: emacsclient's option decoding code
  2008-11-13 21:27                                                   ` mail
@ 2008-11-13 22:30                                                     ` Eli Zaretskii
  2008-11-13 23:54                                                       ` mail
  0 siblings, 1 reply; 67+ messages in thread
From: Eli Zaretskii @ 2008-11-13 22:30 UTC (permalink / raw)
  To: mail; +Cc: emacs-devel

> From: mail@justinbogner.com
> Date: Thu, 13 Nov 2008 14:27:43 -0700
> 
> My point is that trying to decide on a reasonable situation to fall back
> to depends on both the client's environment and the server's
> environment, so trying to be too smart about it on either one is
> dangerous.

Your point is well taken, but _my_ point is that trying to be smart on
the client's side is more dangerous than on the server's side, because
the server at least knows what it can and cannot do.  For example, we
could easily code it _not_ to try the default display if the client's
connection is from another machine (a.k.a ``miles away'').  Puff!
problem gone.




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

* Re: emacsclient's option decoding code
  2008-11-13 22:30                                                     ` Eli Zaretskii
@ 2008-11-13 23:54                                                       ` mail
  0 siblings, 0 replies; 67+ messages in thread
From: mail @ 2008-11-13 23:54 UTC (permalink / raw)
  To: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:
> Your point is well taken, but _my_ point is that trying to be smart on
> the client's side is more dangerous than on the server's side, because
> the server at least knows what it can and cannot do.  For example, we
> could easily code it _not_ to try the default display if the client's
> connection is from another machine (a.k.a ``miles away'').  Puff!
> problem gone.
>

This is a very good point.





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

* Re: emacsclient's option decoding code
  2008-11-07 14:42                             ` Juanma Barranquero
  2008-11-10 18:22                               ` Juanma Barranquero
@ 2008-11-15  2:58                               ` Evil Boris
  1 sibling, 0 replies; 67+ messages in thread
From: Evil Boris @ 2008-11-15  2:58 UTC (permalink / raw)
  To: emacs-devel


"Juanma Barranquero" <lekktu@gmail.com> writes:
> IMO making a frame in the current display and making a frame in a
> remote display are different operations and the point of call should
> know about it. We don't usually conflate making tty and X frames,
> either.

I have been watching the arguments fly by and want to point out that, at
least on X, IMHO, one cannot tell which X frames are local and which are
not, for example, due to X tunnelling.  So I am not convinced that
either the client or the server can determine what to do if the precise
recipe requested by the user fails/is unavaible.  If the commands are
used interactively, I would lean towards having them fail with an error,
rather than try a default behavior that may or may not make sense.  My
concern however is that sometimes emacsclient is invoked as the editor in
mail client, or in reverse search in a TeX previewer, or as an editor
for log msgs in SVN etc.  It is unclear to me what the right behavior is
there---opening a frame in an inaccessible place seems like potential
trouble. 

--Boris





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

end of thread, other threads:[~2008-11-15  2:58 UTC | newest]

Thread overview: 67+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-11-01 14:04 emacsclient's option decoding code Eli Zaretskii
2008-11-02  2:13 ` Stefan Monnier
2008-11-02 21:45 ` Chong Yidong
2008-11-02 22:00   ` Juanma Barranquero
2008-11-02 23:18     ` Chong Yidong
2008-11-03  0:50       ` Juanma Barranquero
2008-11-03  4:14         ` Eli Zaretskii
2008-11-03 12:12           ` Juanma Barranquero
2008-11-03 20:06             ` Eli Zaretskii
2008-11-03 20:19               ` Chong Yidong
2008-11-04  4:20                 ` Eli Zaretskii
2008-11-04 16:40                   ` Chong Yidong
2008-11-06 11:56                     ` Juanma Barranquero
2008-11-03 21:26               ` Juanma Barranquero
2008-11-04  4:19                 ` Eli Zaretskii
2008-11-06 12:08                   ` Juanma Barranquero
2008-11-07  9:53                     ` Eli Zaretskii
2008-11-07 11:42                       ` Juanma Barranquero
2008-11-07 11:47                         ` Juanma Barranquero
2008-11-07 14:36                           ` Eli Zaretskii
2008-11-07 14:42                             ` Juanma Barranquero
2008-11-10 18:22                               ` Juanma Barranquero
2008-11-10 18:36                                 ` Chong Yidong
2008-11-10 23:33                                   ` Juanma Barranquero
2008-11-11  4:09                                 ` Eli Zaretskii
2008-11-11  4:14                                   ` Chong Yidong
2008-11-11 21:16                                     ` Eli Zaretskii
2008-11-11  9:39                                   ` Juanma Barranquero
2008-11-11 15:37                                     ` Juanma Barranquero
2008-11-11 21:24                                     ` Eli Zaretskii
2008-11-11 23:36                                       ` Juanma Barranquero
2008-11-12  4:22                                         ` Eli Zaretskii
2008-11-12  9:41                                           ` Juanma Barranquero
2008-11-12 19:14                                             ` Eli Zaretskii
2008-11-12 21:05                                               ` Juanma Barranquero
2008-11-13  4:12                                                 ` Eli Zaretskii
2008-11-13  8:34                                                   ` Juanma Barranquero
2008-11-12 19:37                                         ` Eli Zaretskii
2008-11-12  2:31                                       ` Stefan Monnier
2008-11-12  9:33                                         ` Juanma Barranquero
2008-11-12 15:56                                           ` Stefan Monnier
2008-11-12 16:37                                             ` Juanma Barranquero
2008-11-12 17:13                                               ` Stefan Monnier
2008-11-13  1:16                                               ` Lennart Borgman
2008-11-13  4:18                                                 ` Eli Zaretskii
2008-11-13  9:00                                               ` Juanma Barranquero
2008-11-13 15:45                                                 ` Dan Nicolaescu
2008-11-13 16:05                                                   ` Juanma Barranquero
2008-11-13 16:05                                                     ` Juanma Barranquero
2008-11-13 16:08                                                     ` Dan Nicolaescu
2008-11-12 19:36                                         ` Eli Zaretskii
2008-11-12 20:56                                           ` Stefan Monnier
2008-11-13  4:10                                             ` Eli Zaretskii
2008-11-13  4:29                                               ` mail
2008-11-13 20:54                                                 ` Eli Zaretskii
2008-11-13 21:27                                                   ` mail
2008-11-13 22:30                                                     ` Eli Zaretskii
2008-11-13 23:54                                                       ` mail
2008-11-13 20:03                                           ` Juri Linkov
2008-11-13 20:40                                             ` mail
2008-11-13 21:08                                               ` Thorsten Bonow
2008-11-13 22:01                                               ` Thorsten Bonow
2008-11-15  2:58                               ` Evil Boris
2008-11-07 14:35                         ` Eli Zaretskii
2008-11-03 14:45           ` Juanma Barranquero
2008-11-03 19:59             ` Eli Zaretskii
2008-11-03 20:03             ` Eli Zaretskii

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).