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: Some comments about changes in revno 110444 Date: Mon, 08 Oct 2012 13:01:34 +0200 Message-ID: <83sj9p448h.fsf@gnu.org> Reply-To: Eli Zaretskii NNTP-Posting-Host: plane.gmane.org X-Trace: ger.gmane.org 1349694163 2990 80.91.229.3 (8 Oct 2012 11:02:43 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Mon, 8 Oct 2012 11:02:43 +0000 (UTC) Cc: emacs-devel@gnu.org To: Daniel Colascione Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Mon Oct 08 13:02:47 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 1TLB6h-000289-J9 for ged-emacs-devel@m.gmane.org; Mon, 08 Oct 2012 13:02:43 +0200 Original-Received: from localhost ([::1]:47473 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TLB6b-0005El-IQ for ged-emacs-devel@m.gmane.org; Mon, 08 Oct 2012 07:02:37 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:53718) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TLB6E-0004Ul-Rk for emacs-devel@gnu.org; Mon, 08 Oct 2012 07:02:20 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TLB69-0002bp-RH for emacs-devel@gnu.org; Mon, 08 Oct 2012 07:02:14 -0400 Original-Received: from mtaout20.012.net.il ([80.179.55.166]:46085) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TLB69-0002bk-IV for emacs-devel@gnu.org; Mon, 08 Oct 2012 07:02:09 -0400 Original-Received: from conversion-daemon.a-mtaout20.012.net.il by a-mtaout20.012.net.il (HyperSendmail v2007.08) id <0MBK00400MLREC00@a-mtaout20.012.net.il> for emacs-devel@gnu.org; Mon, 08 Oct 2012 13:01:41 +0200 (IST) Original-Received: from HOME-C4E4A596F7 ([87.69.4.28]) by a-mtaout20.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0MBK0048LMMSA520@a-mtaout20.012.net.il>; Mon, 08 Oct 2012 13:01:40 +0200 (IST) X-012-Sender: halo1@inter.net.il X-detected-operating-system: by eggs.gnu.org: Solaris 10 (beta) X-Received-From: 80.179.55.166 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:154226 Archived-At: This hunk removes w32inevt from SOME_MACHINE_OBJECTS. Why was that needed? SOME_MACHINE_OBJECTS = dosfns.o msdos.o \ xterm.o xfns.o xmenu.o xselect.o xrdb.o xsmfns.o fringe.o image.o \ - fontset.o dbusbind.o \ + fontset.o dbusbind.o cygw32.o \ nsterm.o nsfns.o nsmenu.o nsselect.o nsimage.o nsfont.o \ - w32.o w32console.o w32fns.o w32heap.o w32inevt.o \ + w32.o w32console.o w32fns.o w32heap.o \ w32menu.o w32proc.o w32reg.o w32select.o w32term.o w32xfns.o \ w16select.o widget.o xfont.o ftfont.o xftfont.o ftxfont.o gtkutil.o \ xsettings.o xgselect.o termcap.o To avoid confusion, I suggest to change the name of the variable here: + /* No, not utf-16-le: that one has a BOM. */ + DEFSYM (Qutf_16_le, "utf-16le"); to Qutf_16le. Isn't the below supposed to be done automagically by make-docfile, which puts these in globals.h? +EXFUN (Fcygwin_convert_path_to_windows, 2); +EXFUN (Fcygwin_convert_path_from_windows, 2); I don't think this (from w32select.h) is a good idea: +#define HAVE_W32SELECT 1 We only have definitions for HAVE_* macros in config.h and conf_post.h. Having them in other headers is confusing, IMO. Can we move this to conf_post.h instead? You moved the definitions of these from w32heap.c to w32fns.c: +/* The major and minor versions of NT. */ +int w32_major_version; +int w32_minor_version; +int w32_build_number; but they are still referenced by w32.c via get_* macros defined on w32heap.h. Can we clean this up, please? Regarding this: +#ifdef NTGUI_UNICODE + wchar_t filename_buf[MAX_PATH + 1]; I believe Unicode names on Windows are not limited to MAX_PATH characters. It looks like there are some unnecessary hops here: + CHECK_STRING (prompt); + CHECK_STRING (dir); + + dir = Fexpand_file_name (dir, Qnil); + + if (STRINGP (filename)) + filename = Ffile_name_nondirectory (filename); + else + filename = empty_unibyte_string; + +#ifdef CYGWIN + dir = Fcygwin_convert_path_to_windows (dir, Qt); + if (SCHARS (filename) > 0) + filename = Fcygwin_convert_path_to_windows (filename, Qnil); +#endif + CHECK_STRING (dir); + CHECK_STRING (filename); + + /* The code in file_dialog_callback that attempts to set the text + of the file name edit window when handling the CDN_INITDONE + WM_NOTIFY message does not work. Setting filename to "Current + Directory" in the only_dir_p case here does work however. */ + if (SCHARS (filename) == 0 && ! NILP (only_dir_p)) + filename = build_string ("Current Directory"); + + /* Convert the values we've computed so far to system form. */ +#ifdef NTGUI_UNICODE + to_unicode (prompt, &prompt); + to_unicode (dir, &dir); + to_unicode (filename, &filename); AFAIU, inside Fcygwin_convert_path_to_windows, you convert the names of the file and directory to UTF-16, then decode it back into a multibyte string, and finally encode it again to UTF-16 when you call to_unicode. Perhaps the back and forth conversion to and from UTF-16 can be avoided? In any case, a call to CHECK_STRING after we already checked the arguments seems both redundant and user-unfriendly: these tests are designed to reject errors in arguments passed to primitives, so signaling an error because something went wrong in processing valid arguments doesn't sound like a good idea. This error will not report the offending file name: + if (SBYTES (filename) + 1 > sizeof (filename_buf)) + error ("filename too long"); I think report_file_error is better here. Why do you need the fprintf part here: #ifdef EMACSDEBUG void _DebPrint (const char *fmt, ...) { char buf[1024]; va_list args; va_start (args, fmt); vsprintf (buf, fmt, args); va_end (args); #if CYGWIN fprintf (stderr, "%s", buf); #endif OutputDebugString (buf); } What happened to number 21 in the change below? #define WM_EMACS_HIDE_CARET (WM_EMACS_START + 18) #define WM_EMACS_SETCURSOR (WM_EMACS_START + 19) #define WM_EMACS_PAINT (WM_EMACS_START + 20) -#define WM_EMACS_BRINGTOTOP (WM_EMACS_START + 21) -#define WM_EMACS_END (WM_EMACS_START + 22) +#define WM_EMACS_BRINGTOTOP (WM_EMACS_START + 22) +#define WM_EMACS_INPUT_READY (WM_EMACS_START + 23) +#define WM_EMACS_END (WM_EMACS_START + 24) Why did you need the call to notify_msg_ready below? @@ -285,7 +309,7 @@ prepend_msg (W32Msg *lpmsg) nQueue++; lpNew->lpNext = lpHead; lpHead = lpNew; - + notify_msg_ready ();