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] system-type cygwin with window-system w32 Date: Mon, 18 Jul 2011 04:53:08 -0400 Message-ID: References: <4E2377E2.1020804@gmail.com> <4E23D2BF.7080309@gmail.com> Reply-To: Eli Zaretskii NNTP-Posting-Host: lo.gmane.org X-Trace: dough.gmane.org 1310979556 3612 80.91.229.12 (18 Jul 2011 08:59:16 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Mon, 18 Jul 2011 08:59:16 +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 Jul 18 10:59:12 2011 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([140.186.70.17]) by lo.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1QijfT-0002k1-Nd for ged-emacs-devel@m.gmane.org; Mon, 18 Jul 2011 10:59:12 +0200 Original-Received: from localhost ([::1]:54860 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QijfS-00031O-MJ for ged-emacs-devel@m.gmane.org; Mon, 18 Jul 2011 04:59:10 -0400 Original-Received: from eggs.gnu.org ([140.186.70.92]:36438) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QijZi-0001LD-7t for emacs-devel@gnu.org; Mon, 18 Jul 2011 04:53:18 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QijZd-0006yI-8p for emacs-devel@gnu.org; Mon, 18 Jul 2011 04:53:14 -0400 Original-Received: from fencepost.gnu.org ([140.186.70.10]:39010) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QijZc-0006yD-Sk for emacs-devel@gnu.org; Mon, 18 Jul 2011 04:53:08 -0400 Original-Received: from eliz by fencepost.gnu.org with local (Exim 4.71) (envelope-from ) id 1QijZc-0004xp-Ho; Mon, 18 Jul 2011 04:53:08 -0400 In-reply-to: <4E23D2BF.7080309@gmail.com> (message from Daniel Colascione on Sun, 17 Jul 2011 23:29:19 -0700) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-Received-From: 140.186.70.10 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:142091 Archived-At: > Date: Sun, 17 Jul 2011 23:29:19 -0700 > From: Daniel Colascione > CC: emacs-devel@gnu.org > > On 7/17/11 11:13 PM, Eli Zaretskii wrote: > > Why did you need to change filemode.c? Does it have anything to do > > with Cygwin on w32? > > S_ISCTG and such aren't being defined under Cygwin, causing compilation > errors. There's probably a better way to deal with the underlying problem. Yes, the files in lib/sys_stat.in.h is supposed to do that already. I'm curious why it didn't work for you. > > What is this (and related) stuff about? Why do you need to use HTML > > wrt the clipboard? > > Windows uses HTML as a data interchange format --- supporting it as a > clipboard format allows formatting to be preserved in pastes into other > programs. This code could easily be structured as a separate package, > however, and I'll end up doing that. Yes, let's have this as a separate changeset. > > I don't like the idea of moving this to w32fns.c, because it doesn't > > belong there. Can you come up with an alternative idea? > > The fundamental problem is that we now have two Windows platforms: > WINDOWSNT and (CYGWIN && HAVE_NTGUI). The common code has to live > somewhere; with my patch, we only build w32.o in the NTEMACS case > because w32.c contains mostly compatibility wrappers; the > non-compatibility portions I moved to w32fns.c, which we compile in both > cases. Cygwin-NT-specific code goes in the new file cygw32.c. > > Another option would be to further refactor the Win32 code into distinct > and explicit WINDOWSNT and HAVE_NTGUI files and introduce common headers > for common functionality. This approach would involve even more code > movement, however, which is why I initially avoided it. I'd prefer a separate file common to w2 and Cygwin-on-w32, if that's needed. w32fns.c tries to be as similar to xfns.c as possible, so putting there stuff that's not relevant would be a disadvantage. > >> +#define t(...) \ > >> + ({ \ > >> + fprintf (stderr, "T:%s:%u: ", \ > >> + __FUNCTION__, __LINE__); \ > >> + fprintf (stderr, __VA_ARGS__); \ > >> + fputc ('\n', stderr); \ > >> + }) > >> + > > > > What is this stuff about? > > Debug scaffolding --- in this case, generally useful, I think, at least > as a replacement for the numerous bespoke tracing macros scattered > everywhere in the code. Fine, but (a) please see if there's no macro already available that can be used instead; and (b) let's have this a separate changeset. > >> + /* DebPrint (("w32_msg_pump: %s time:%u\n", */ > >> + /* w32_name_of_message (msg.message), msg.time)); */ > >> + > > > > Can this be removed? These DebPrint messages are a PITA when > > debugging, so if it isn't absolutely necessary, let's not add new > > ones. > > Sure. I'd actually prefer, though, to leave the existing tracing, but > move it all to a common macro so that the debug spam is easier to > disable and enable as needed. Fine with me. > > This is based on reviewing only a part of the patch, I will have more > > later. The patch is very large and complicated, and the lack of a > > ChangeLog that describes the changes, particularly those which move > > code between different files, does not help... > > Of course. It's a work in progress --- a first stab, really. Once I > clean up the code a bit, I'll put it into a form that's easier to consume. My point was that there are several issues here that need to be discussed before you invest too much energy into them. So please consider starting these discussion sooner rather than later, and the additional information I didn't find in the ChangeLog would be instrumental at that time. TIA