Re: [PATCH 3/4] Implement cygw32

[Eli Zaretskii <eliz@gnu.org>]

> Date: Thu, 29 Dec 2011 06:03:20 -0800
> From: Daniel Colascione <dancol@dancol.org>
> 
> --- a/lisp/faces.el
> +++ b/lisp/faces.el
> @@ -89,7 +89,7 @@ ALTERNATIVE2 etc."
>  ;; This is defined originally in xfaces.c.
>  (defcustom face-font-registry-alternatives
>    (mapcar (lambda (arg) (mapcar 'purecopy arg))
> -  (if (eq system-type 'windows-nt)
> +  (if (featurep 'w32)
>        '(("iso8859-1" "ms-oemlatin")
>  	("gb2312.1980" "gb2312" "gbk" "gb18030")
>  	("jisx0208.1990" "jisx0208.1983" "jisx0208.1978")

Why not use window-system (the function) here?

> --- a/lisp/frame.el
> +++ b/lisp/frame.el
> @@ -522,7 +522,7 @@ The optional argument PARAMETERS specifies additional frame parameters."
>  	   (ns-initialize-window-system))
>  	 (make-frame `((window-system . ns)
>  		       (display . ,display) . ,parameters)))
> -	((eq system-type 'windows-nt)
> +	((eq window-system 'w32)
>  	 ;; On Windows, ignore DISPLAY.
>  	 (make-frame parameters))
>  	(t

Why not window-system the function?  You do want to continue
supporting multi-tty in this configuration, right?

> --- a/lisp/international/mule-cmds.el
> +++ b/lisp/international/mule-cmds.el
> @@ -2655,7 +2655,8 @@ See also `locale-charset-language-names', `locale-language-names',
>      ;; On Windows, override locale-coding-system,
>      ;; default-file-name-coding-system, keyboard-coding-system,
>      ;; terminal-coding-system with system codepage.
> -    (when (boundp 'w32-ansi-code-page)
> +    (when (and (eq system-type 'windows-nt)
> +               (boundp 'w32-ansi-code-page))
>        (let ((code-page-coding (intern (format "cp%d" w32-ansi-code-page))))

Can you explain the need for this change?

> --- a/lisp/loadup.el
> +++ b/lisp/loadup.el
> @@ -207,15 +207,18 @@
>        (load "term/common-win")
>        (load "term/x-win")))
>  
> -(if (eq system-type 'windows-nt)
> +(if (or (eq system-type 'windows-nt)
> +        (featurep 'w32))
>      (progn
> -      (load "w32-vars")
>        (load "term/common-win")
> +      (load "w32-vars")

Did you really need this order change?  If yes, why?  If not, I'd
prefer to leave the original order, as changing it could potentially
cause unintended consequences.

>        (load "term/w32-win")
> -      (load "ls-lisp")
>        (load "disp-table")
> -      (load "dos-w32")
> -      (load "w32-fns")))
> +      (load "w32-common-fns")
> +      (when (eq system-type 'windows-nt)
> +        (load "w32-fns")
> +        (load "ls-lisp")
> +        (load "dos-w32"))))

Likewise here: at the very least, keep the order in the group of
packages loaded for windows-nt.

> --- a/lisp/mouse.el
> +++ b/lisp/mouse.el
> @@ -1147,7 +1147,7 @@ regardless of where you click."
>    (or mouse-yank-at-point (mouse-set-point click))
>    (let ((primary
>  	 (cond
> -	  ((eq system-type 'windows-nt)
> +	  ((eq (framep (selected-frame)) 'w32)
>  	   ;; MS-Windows emulates PRIMARY in x-get-selection, but not
>  	   ;; in x-get-selection-value (the latter only accesses the
>  	   ;; clipboard).  So try PRIMARY first, in case they selected

You mean, the Cygwin build that uses w32 windowing will be unable to
support X selections?  That would be a pity.

> +(defun w32-handle-dropped-file (window file-name)
> +  (let ((f (if (eq system-type 'cygwin)
> +               (cygwin-convert-path-from-windows file-name t)
> +             (subst-char-in-string ?\\ ?/ file-name)))
> +        (coding (or file-name-coding-system
> +                    default-file-name-coding-system)))

What is the default value of default-file-name-coding-system in the
Cygwin build?  There could be a conflict here between e.g. UTF-8 in
Cygwin and the Windows codepage.

Also, it would be good to have a doc string for this function.

> +DEFUN ("cygwin-convert-path-to-windows",
> +       Fcygwin_convert_path_to_windows, Scygwin_convert_path_to_windows,
> +       1, 2, 0,
> +       doc: /* Convert PATH to a Windows path.  If ABSOLUTE-P if
> +               non-nil, return an absolute path.*/)
> +  (Lisp_Object path, Lisp_Object absolute_p)
> +{
> +  return from_unicode (
> +    conv_filename_to_w32_unicode (path, absolute_p == Qnil ? 0 : 1));
> +}
> +
> +DEFUN ("cygwin-convert-path-from-windows",
> +       Fcygwin_convert_path_from_windows, Scygwin_convert_path_from_windows,
> +       1, 2, 0,
> +       doc: /* Convert a Windows path to a Cygwin path.  If ABSOLUTE-P
> +               if non-nil, return an absolute path.*/)
> +  (Lisp_Object path, Lisp_Object absolute_p)
> +{
> +  return conv_filename_from_w32_unicode (to_unicode (path, &path),
> +                                         absolute_p == Qnil ? 0 : 1);
> +}

These two should probably be documented in the ELisp manual.

>  	  char buffer[16];
> -	  _snprintf (buffer, 16, "%d", system_status.BatteryLifePercent);
> +	  snprintf (buffer, 16, "%d", system_status.BatteryLifePercent);
>  	  load_percentage = build_string (buffer);

If you want to use snprintf without the leading underscore, please add
a suitable #define somewhere for the benefit of the MSVC compiler,
which doesn't have snprintf, only _snprintf.

Btw, will the above code be used by the Cygwin build?  Does that
include default-printer-name etc.?