From: Eli Zaretskii <eliz@gnu.org>
To: Daniel Colascione <dancol@dancol.org>
Cc: emacs-devel@gnu.org
Subject: Some comments about changes in revno 110444
Date: Mon, 08 Oct 2012 13:01:34 +0200 [thread overview]
Message-ID: <83sj9p448h.fsf@gnu.org> (raw)
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 ();
next reply other threads:[~2012-10-08 11:01 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-08 11:01 Eli Zaretskii [this message]
2012-10-08 12:53 ` Some comments about changes in revno 110444 Daniel Colascione
2012-10-08 13:50 ` Eli Zaretskii
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=83sj9p448h.fsf@gnu.org \
--to=eliz@gnu.org \
--cc=dancol@dancol.org \
--cc=emacs-devel@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.