From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: [PATCH 3/9] Implement cygw32 Date: Tue, 07 Aug 2012 21:02:00 +0300 Message-ID: <83obmmfunb.fsf@gnu.org> References: Reply-To: Eli Zaretskii NNTP-Posting-Host: plane.gmane.org X-Trace: dough.gmane.org 1344362552 32502 80.91.229.3 (7 Aug 2012 18:02:32 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Tue, 7 Aug 2012 18:02:32 +0000 (UTC) Cc: emacs-devel@gnu.org To: Daniel Colascione Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Tue Aug 07 20:02:32 2012 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1Syo6x-0008EF-Mi for ged-emacs-devel@m.gmane.org; Tue, 07 Aug 2012 20:02:31 +0200 Original-Received: from localhost ([::1]:36786 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Syo6w-0005Ec-9l for ged-emacs-devel@m.gmane.org; Tue, 07 Aug 2012 14:02:30 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:48226) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Syo6q-0005EV-F2 for emacs-devel@gnu.org; Tue, 07 Aug 2012 14:02:29 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Syo6n-0003pV-8w for emacs-devel@gnu.org; Tue, 07 Aug 2012 14:02:24 -0400 Original-Received: from mtaout23.012.net.il ([80.179.55.175]:59418) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Syo6m-0003pP-Rt for emacs-devel@gnu.org; Tue, 07 Aug 2012 14:02:21 -0400 Original-Received: from conversion-daemon.a-mtaout23.012.net.il by a-mtaout23.012.net.il (HyperSendmail v2007.08) id <0M8E00A00CQPK200@a-mtaout23.012.net.il> for emacs-devel@gnu.org; Tue, 07 Aug 2012 21:02:01 +0300 (IDT) Original-Received: from HOME-C4E4A596F7 ([87.69.4.28]) by a-mtaout23.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0M8E00A63CRCJX10@a-mtaout23.012.net.il>; Tue, 07 Aug 2012 21:02:01 +0300 (IDT) In-reply-to: X-012-Sender: halo1@inter.net.il X-detected-operating-system: by eggs.gnu.org: Solaris 10 (beta) X-Received-From: 80.179.55.175 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:152274 Archived-At: > Date: Tue, 07 Aug 2012 01:19:27 -0700 > From: Daniel Colascione > > +(defun w32-using-nt () > + "Return non-nil if running on a Windows NT descendant. > +That includes all Windows systems except for 9X/Me." > + (getenv "SystemRoot")) I don't like to rely on environment variables for system-type detection. It's all to easy to remove or add environment variables. If there's no better way that is already there, I'd prefer a new primitive that is based on os_subtype variable in C, or something similar. (Yes, I know the current code uses getenv as well, but we might as well fix it while at that.) > --- a/lisp/w32-fns.el > +++ b/lisp/w32-fns.el > @@ -26,34 +26,20 @@ > > ;;; Code: > (require 'w32-vars) > +(require 'w32-common-fns) Why do we need this require? loadup.el loads w32-common-fns before w32-fns, so it's already loaded here. If you need this for compilation, won't eval-when-compile be better? > +wchar_t* > +to_unicode (Lisp_Object str, Lisp_Object* buf) A minor style issue: please leave a blank between '*' and the preceding token. > + *buf = code_convert_string_norecord (str, Qutf_16_le, 1); > + /* We need to make a another copy (in addition to the one made by > + code_convert_string_norecord) to ensure that the final string is > + _doubly_ zero terminated --- that is, that the string is > + terminated by two zero bytes and one utf-16le null character. > + Because strings are already terminated with a single zero byte, > + we just add one additional zero. */ > + str = make_uninit_string (SBYTES (*buf) + 1); > + memcpy (SDATA (str), SDATA (*buf), SBYTES (*buf)); > + SDATA (str) [SBYTES (*buf)] = '\0'; > + *buf = str; Can't you append a zero byte to the original str, before encoding it with code_convert_string_norecord? Copying it after encoding looks inelegant. > +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)); > +} This function is only used from C. Do we really need its Lisp binding? > +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); > +} I wonder why this couldn't be implemented in Lisp. Isn't it just decoding from UTF-16 followed by mirroring the backslashes? If there's more to it than that, please reflect that in the doc string. > +#ifndef CYGW32_H > +#define CYGW32_H > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "lisp.h" > +#include "coding.h" I think it's a bad mojo for a header to include other Emacs headers. Is it really needed? > +/* Access the wide-character string stored in a Lisp string object. */ > +#define WCSDATA(x) ((wchar_t*) SDATA (x)) This is IMO yucky, and I'd like to avoid this if possible. Can you explain why it is needed? > +#if defined(CYGWIN) && defined(HAVE_NTGUI) > +#define NTGUI_UNICODE /* Cygwin runs only on UNICODE-supporting systems */ > +#define _WIN32_WINNT 0x500 /* Win2k */ > +#endif /* CYGWIN && HAVE_NTGUI */ I'd like to make tests of NTGUI_UNICODE at run time rather than at compile time. That's because we are gradually moving Emacs to using Unicode APIs when available, so making these tests at run time will allow to share more code between the Cygwin and native Windows builds. There's already a variable w32_console_unicode_input in w32inevt.c that is used for a similar purpose, and I believe there will be soon another for a GUI input. Can you define a similar variable for menus or whatever, and either initialize it on some file that is compiled both in the native and the Cygwin builds, or just unconditionally set it to non-zero in some Cygwin specific function? Then test it whenever you need a Unicode API. > --- a/src/emacs.c > +++ b/src/emacs.c > @@ -37,9 +37,20 @@ along with GNU Emacs. If not, see . */ > > #ifdef WINDOWSNT > #include > -#include /* just for w32.h */ It looks like you are deleting the inclusion of windows.h. Why? > -#if defined (WINDOWSNT) > +#if defined (HAVE_NTGUI) > LANGUAGE_CHANGE_EVENT, /* A LANGUAGE_CHANGE_EVENT is > - generated on WINDOWSNT or Mac OS > + generated on HAVE_NTGUI or Mac OS "On HAVE_NTGUI" does not sound right... Thanks.