Proposal: window-system-version function

Proposal: window-system-version function

[Dmitry Antipov]

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

This patch replaces global variable 'window-system-version' with the function
which returns the window system version for the specified frame. An obvious
reason for this is to have correct results when Emacs is running with both
TTY and X frames.

Dmitry

[-- Attachment #2: window_system_version.patch --]
[-- Type: text/plain, Size: 6068 bytes --]

=== modified file 'lisp/international/mule-diag.el'
--- lisp/international/mule-diag.el	2012-01-19 07:21:25 +0000
+++ lisp/international/mule-diag.el	2012-05-21 13:54:49 +0000
@@ -1105,7 +1105,7 @@
       (insert-section 2 "Display")
       (if window-system
 	  (insert (format "Window-system: %s, version %s"
-			  window-system window-system-version))
+			  window-system (window-system-version)))
 	(insert "Terminal: " (getenv "TERM")))
       (insert "\n\n")
 

=== modified file 'lisp/textmodes/artist.el'
--- lisp/textmodes/artist.el	2012-04-11 11:57:21 +0000
+++ lisp/textmodes/artist.el	2012-05-21 13:58:38 +0000
@@ -5371,20 +5371,21 @@
   (interactive)
   (require 'reporter)
   (if (y-or-n-p "Do you want to submit a bug report on Artist? ")
-      (let ((to   artist-maintainer-address)
-	    (vars '(window-system
-		    window-system-version
-		    ;;
-		    artist-rubber-banding
-		    artist-interface-with-rect
-		    artist-aspect-ratio
-		    ;; Now the internal ones
-		    artist-curr-go
-		    artist-key-poly-point-list
-		    artist-key-shape
-		    artist-key-draw-how
-		    artist-arrow-point-1
-		    artist-arrow-point-2)))
+      (let* ((to artist-maintainer-address)
+	     (window-system-version (window-system-version))
+	     (vars '(window-system
+		     window-system-version
+		     ;;
+		     artist-rubber-banding
+		     artist-interface-with-rect
+		     artist-aspect-ratio
+		     ;; Now the internal ones
+		     artist-curr-go
+		     artist-key-poly-point-list
+		     artist-key-shape
+		     artist-key-draw-how
+		     artist-arrow-point-1
+		     artist-arrow-point-2)))
 	;; Remove those variables from vars that are not bound
 	(mapc
 	 (function

=== modified file 'src/dispnew.c'
--- src/dispnew.c	2012-05-01 00:30:11 +0000
+++ src/dispnew.c	2012-05-21 13:55:48 +0000
@@ -6301,9 +6301,6 @@
   if (!inhibit_window_system && display_arg)
     {
       Vinitial_window_system = Qx;
-#ifdef HAVE_X11
-      Vwindow_system_version = make_number (11);
-#endif
 #ifdef GNU_LINUX
       /* In some versions of ncurses,
 	 tputs crashes if we have not called tgetent.
@@ -6319,7 +6316,6 @@
   if (!inhibit_window_system)
     {
       Vinitial_window_system = Qw32;
-      Vwindow_system_version = make_number (1);
       adjust_frame_glyphs_initially ();
       return;
     }
@@ -6333,7 +6329,6 @@
       )
     {
       Vinitial_window_system = Qns;
-      Vwindow_system_version = make_number (10);
       adjust_frame_glyphs_initially ();
       return;
     }
@@ -6575,10 +6570,6 @@
 use `display-graphic-p' or any of the other `display-*-p'
 predicates which report frame's specific UI-related capabilities.  */);
 
-  DEFVAR_LISP ("window-system-version", Vwindow_system_version,
-	       doc: /* The version number of the window system in use.
-For X windows, this is 11.  */);
-
   DEFVAR_BOOL ("cursor-in-echo-area", cursor_in_echo_area,
 	       doc: /* Non-nil means put cursor in minibuffer, at end of any message there.  */);
 
@@ -6614,8 +6605,5 @@
 #ifdef CANNOT_DUMP
   if (noninteractive)
 #endif
-    {
-      Vinitial_window_system = Qnil;
-      Vwindow_system_version = Qnil;
-    }
+    Vinitial_window_system = Qnil;
 }

=== modified file 'src/frame.c'
--- src/frame.c	2012-03-10 07:46:07 +0000
+++ src/frame.c	2012-05-21 13:54:49 +0000
@@ -256,6 +256,52 @@
     return type;
 }
 
+DEFUN ("window-system-version", Fwindow_system_version, Swindow_system_version, 0, 1, 0,
+       doc: /* The version of the window system that FRAME is displaying through.
+It's value is a number:
+ - 0 for a termcap frame,
+ - Major X protocol version for the frame on X display,
+ - Major OS version for the frame on MS-Windows display,
+ - 24 for the frame on direct-write MS-DOS display,
+ - 10 for the frame on a GNUstep or Macintosh Cocoa display.
+
+FRAME defaults to the currently selected frame.  */)
+     (Lisp_Object frame)
+{
+  struct frame *f;
+
+  if (NILP (frame))
+    frame = selected_frame;
+  CHECK_LIVE_FRAME (frame);
+  f = XFRAME (frame);
+
+  switch (f->output_method)
+    {
+    case output_initial:
+    case output_termcap:
+      return make_number (0);
+#ifdef HAVE_X_WINDOWS
+    case output_x_window:
+      return make_number (ProtocolVersion (FRAME_X_DISPLAY (f)));
+#endif
+#ifdef WINDOWSNT
+    case output_w32:
+      return make_number (w32_major_version);
+#endif
+#ifdef MSDOS
+    case output_msdos_raw:
+      return make_number (24);
+#endif
+#ifdef HAVE_NS
+    case output_mac:
+    case output_ns:
+      return make_number (10);
+#endif
+    default:
+      abort ();
+    }
+}
+
 struct frame *
 make_frame (int mini_p)
 {
@@ -4455,6 +4501,7 @@
   defsubr (&Sframep);
   defsubr (&Sframe_live_p);
   defsubr (&Swindow_system);
+  defsubr (&Swindow_system_version);
   defsubr (&Smake_terminal_frame);
   defsubr (&Shandle_switch_frame);
   defsubr (&Sselect_frame);

=== modified file 'src/msdos.c'
--- src/msdos.c	2012-04-09 13:05:48 +0000
+++ src/msdos.c	2012-05-21 13:54:49 +0000
@@ -1813,7 +1813,6 @@
 	}
 
       Vinitial_window_system = Qpc;
-      Vwindow_system_version = make_number (23); /* RE Emacs version */
       tty->terminal->type = output_msdos_raw;
 
       /* If Emacs was dumped on DOS/V machine, forget the stale VRAM

=== modified file 'src/w32fns.c'
--- src/w32fns.c	2012-05-18 08:36:50 +0000
+++ src/w32fns.c	2012-05-21 13:54:49 +0000
@@ -4729,7 +4729,6 @@
     error ("Cannot connect to server %s", SDATA (name));
 
   w32_in_use = 1;
-  XSETFASTINT (Vwindow_system_version, w32_major_version);
 
   return dpyinfo;
 }
@@ -4820,7 +4819,6 @@
 
   w32_in_use = 1;
 
-  XSETFASTINT (Vwindow_system_version, w32_major_version);
   return Qnil;
 }
 

=== modified file 'src/xfns.c'
--- src/xfns.c	2012-05-19 21:46:43 +0000
+++ src/xfns.c	2012-05-21 13:54:49 +0000
@@ -4063,7 +4063,6 @@
     error ("Cannot connect to X server %s", SDATA (name));
 
   x_in_use = 1;
-  XSETFASTINT (Vwindow_system_version, 11);
 
   return dpyinfo;
 }
@@ -4118,7 +4117,6 @@
 
   x_in_use = 1;
 
-  XSETFASTINT (Vwindow_system_version, 11);
   return Qnil;
 }
 

Re: Proposal: window-system-version function

[Stefan Monnier]

> This patch replaces global variable 'window-system-version' with the function
> which returns the window system version for the specified frame. An obvious
> reason for this is to have correct results when Emacs is running with both
> TTY and X frames.

The general idea of the patch is right, the implementation looks OK as
well, but I have one major issue with it: why do we ever need to know
the "version" of the window-system?

I've only ever seen it used to display diagnostic data, i.e. to
help debug some problem, but I've never found it useful when debugging.

IOW, I'd much rather mark window-system-version as obsolete.


        Stefan

Re: Proposal: window-system-version function

[Eli Zaretskii]

> Date: Mon, 21 May 2012 18:11:02 +0400
> From: Dmitry Antipov <dmantipov@yandex.ru>
> 
> This patch replaces global variable 'window-system-version' with the function
> which returns the window system version for the specified frame. An obvious
> reason for this is to have correct results when Emacs is running with both
> TTY and X frames.

Thanks.  However, this change breaks backward compatibility.  Perhaps
it would be better to leave the variable alone, and _add_ the
function; that would be backward-compatible.

> +It's value is a number:
> + - 0 for a termcap frame,
> + - Major X protocol version for the frame on X display,
> + - Major OS version for the frame on MS-Windows display,
> + - 24 for the frame on direct-write MS-DOS display,
> + - 10 for the frame on a GNUstep or Macintosh Cocoa display.

This is inaccurate at least for MS-DOS and MS-Windows.  Do we really
want to document the precise meaning of the values here?  Why is that
important?

> +#ifdef HAVE_X_WINDOWS
> +    case output_x_window:
> +      return make_number (ProtocolVersion (FRAME_X_DISPLAY (f)));
> +#endif
> +#ifdef WINDOWSNT
> +    case output_w32:
> +      return make_number (w32_major_version);
> +#endif
> +#ifdef MSDOS
> +    case output_msdos_raw:
> +      return make_number (24);
> +#endif

The MSDOS build supports HAVE_X_WINDOWS as well, so HAVE_X_WINDOWS and
MSDOS are not by themselves mutually exclusive.

> --- src/msdos.c	2012-04-09 13:05:48 +0000
> +++ src/msdos.c	2012-05-21 13:54:49 +0000
> @@ -1813,7 +1813,6 @@
>  	}
>  
>        Vinitial_window_system = Qpc;
> -      Vwindow_system_version = make_number (23); /* RE Emacs version */
                                               ^^^^^^^^^^^^^^^^^^^^^^^^
Which means I goofed.  Will fix in a moment.

Re: Proposal: window-system-version function

[Ken Raeburn]

On May 21, 2012, at 10:11, Dmitry Antipov wrote:
> This patch replaces global variable 'window-system-version' with the function
> which returns the window system version for the specified frame. An obvious
> reason for this is to have correct results when Emacs is running with both
> TTY and X frames.

And maybe someday running with TTY and X and NextStep frames, I hope….

Ken

Re: Proposal: window-system-version function

[Dmitry Antipov]

On 05/21/2012 08:20 PM, Eli Zaretskii wrote:

> Thanks.  However, this change breaks backward compatibility.  Perhaps
> it would be better to leave the variable alone, and _add_ the
> function; that would be backward-compatible.

I fixed Lisp code in lisp/international/mule-diag.el and
lisp/textmodes/artist.el; not sure about external packages which
may also use it.

Leaving the variable "as is" means that window-system-version
and (window-system-version) may have different values, which
is confusing. Ideally, window-system-version should be a frame-local
variable, but I'm wondering whether this is possible in Emacs.

>> +It's value is a number:
>> + - 0 for a termcap frame,
>> + - Major X protocol version for the frame on X display,
>> + - Major OS version for the frame on MS-Windows display,
>> + - 24 for the frame on direct-write MS-DOS display,
>> + - 10 for the frame on a GNUstep or Macintosh Cocoa display.
>
> This is inaccurate at least for MS-DOS and MS-Windows.  Do we really
> want to document the precise meaning of the values here?  Why is that
> important?

I'm not sure about exact values on a systems beyond *nix, and I suppose
that every Lisp-visible function should be documented.

>> +#ifdef HAVE_X_WINDOWS
>> +    case output_x_window:
>> +      return make_number (ProtocolVersion (FRAME_X_DISPLAY (f)));
>> +#endif
>> +#ifdef WINDOWSNT
>> +    case output_w32:
>> +      return make_number (w32_major_version);
>> +#endif
>> +#ifdef MSDOS
>> +    case output_msdos_raw:
>> +      return make_number (24);
>> +#endif
>
> The MSDOS build supports HAVE_X_WINDOWS as well, so HAVE_X_WINDOWS and
> MSDOS are not by themselves mutually exclusive.

The cpp stuff above has no mutually exclusive paths (no #elif or so),
so it should work if both HAVE_X_WINDOWS and MSDOS are defined.

Dmitry

Re: Proposal: window-system-version function

[Stefan Monnier]

Not sure if you've missed my earlier reply, but I want to mark
window-system-version as obsolete, so unless there's a strong objection
to that, there' no point discussing how to improve it.


        Stefan


>>>>> "Dmitry" == Dmitry Antipov <dmantipov@yandex.ru> writes:

> On 05/21/2012 08:20 PM, Eli Zaretskii wrote:

>> Thanks.  However, this change breaks backward compatibility.  Perhaps
>> it would be better to leave the variable alone, and _add_ the
>> function; that would be backward-compatible.

> I fixed Lisp code in lisp/international/mule-diag.el and
> lisp/textmodes/artist.el; not sure about external packages which
> may also use it.

> Leaving the variable "as is" means that window-system-version
> and (window-system-version) may have different values, which
> is confusing. Ideally, window-system-version should be a frame-local
> variable, but I'm wondering whether this is possible in Emacs.

>>> +It's value is a number:
>>> + - 0 for a termcap frame,
>>> + - Major X protocol version for the frame on X display,
>>> + - Major OS version for the frame on MS-Windows display,
>>> + - 24 for the frame on direct-write MS-DOS display,
>>> + - 10 for the frame on a GNUstep or Macintosh Cocoa display.
>> 
>> This is inaccurate at least for MS-DOS and MS-Windows.  Do we really
>> want to document the precise meaning of the values here?  Why is that
>> important?

> I'm not sure about exact values on a systems beyond *nix, and I suppose
> that every Lisp-visible function should be documented.

>>> +#ifdef HAVE_X_WINDOWS
>>> +    case output_x_window:
>>> +      return make_number (ProtocolVersion (FRAME_X_DISPLAY (f)));
>>> +#endif
>>> +#ifdef WINDOWSNT
>>> +    case output_w32:
>>> +      return make_number (w32_major_version);
>>> +#endif
>>> +#ifdef MSDOS
>>> +    case output_msdos_raw:
>>> +      return make_number (24);
>>> +#endif
>> 
>> The MSDOS build supports HAVE_X_WINDOWS as well, so HAVE_X_WINDOWS and
>> MSDOS are not by themselves mutually exclusive.

> The cpp stuff above has no mutually exclusive paths (no #elif or so),
> so it should work if both HAVE_X_WINDOWS and MSDOS are defined.

> Dmitry

Re: Proposal: window-system-version function

[Eli Zaretskii]

> Date: Tue, 22 May 2012 09:51:18 +0400
> From: Dmitry Antipov <dmantipov@yandex.ru>
> CC: emacs-devel@gnu.org

This probably becomes a moot point, given Stefan's response, but...

> On 05/21/2012 08:20 PM, Eli Zaretskii wrote:
> 
> > Thanks.  However, this change breaks backward compatibility.  Perhaps
> > it would be better to leave the variable alone, and _add_ the
> > function; that would be backward-compatible.
> 
> I fixed Lisp code in lisp/international/mule-diag.el and
> lisp/textmodes/artist.el; not sure about external packages which
> may also use it.

I'm not sure either, but gray hair tells me they might.

> Leaving the variable "as is" means that window-system-version
> and (window-system-version) may have different values, which
> is confusing.

Not confusing at all, since we would deprecate the variable.  But
breaking working code over such a minor issue is not a good idea, IMO.

> Ideally, window-system-version should be a frame-local
> variable, but I'm wondering whether this is possible in Emacs.

Yes, see "Frame Parameters".

> >> +It's value is a number:
> >> + - 0 for a termcap frame,
> >> + - Major X protocol version for the frame on X display,
> >> + - Major OS version for the frame on MS-Windows display,
> >> + - 24 for the frame on direct-write MS-DOS display,
> >> + - 10 for the frame on a GNUstep or Macintosh Cocoa display.
> >
> > This is inaccurate at least for MS-DOS and MS-Windows.  Do we really
> > want to document the precise meaning of the values here?  Why is that
> > important?
> 
> I'm not sure about exact values on a systems beyond *nix, and I suppose
> that every Lisp-visible function should be documented.

I didn't mean not to document it.  I meant to document it like this:

 Value is the version of the windowing system used for this frame, as
 a string.

I think this is enough, since the number itself won't tell anything
specific anyway.

> >> +#ifdef HAVE_X_WINDOWS
> >> +    case output_x_window:
> >> +      return make_number (ProtocolVersion (FRAME_X_DISPLAY (f)));
> >> +#endif
> >> +#ifdef WINDOWSNT
> >> +    case output_w32:
> >> +      return make_number (w32_major_version);
> >> +#endif
> >> +#ifdef MSDOS
> >> +    case output_msdos_raw:
> >> +      return make_number (24);
> >> +#endif
> >
> > The MSDOS build supports HAVE_X_WINDOWS as well, so HAVE_X_WINDOWS and
> > MSDOS are not by themselves mutually exclusive.
> 
> The cpp stuff above has no mutually exclusive paths (no #elif or so),
> so it should work if both HAVE_X_WINDOWS and MSDOS are defined.

Right, my bad.  I've misread the code.