[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Some comments about changes in revno 110444
From: |
Eli Zaretskii |
Subject: |
Some comments about changes in revno 110444 |
Date: |
Mon, 08 Oct 2012 13:01:34 +0200 |
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 ();
- Some comments about changes in revno 110444,
Eli Zaretskii <=