From mboxrd@z Thu Jan 1 00:00:00 1970 Path: main.gmane.org!not-for-mail From: storm@cua.dk (Kim F. Storm) Newsgroups: gmane.emacs.devel Subject: Re: Image support for Carbon Emacs (Re: Consolidation of image support) Date: 14 Jan 2004 01:20:31 +0100 Sender: emacs-devel-bounces+emacs-devel=quimby.gnus.org@gnu.org Message-ID: References: <20040113.211033.74742347.mituharu@math.s.chiba-u.ac.jp> NNTP-Posting-Host: deer.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: sea.gmane.org 1074052008 27776 80.91.224.253 (14 Jan 2004 03:46:48 GMT) X-Complaints-To: usenet@sea.gmane.org NNTP-Posting-Date: Wed, 14 Jan 2004 03:46:48 +0000 (UTC) Cc: emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+emacs-devel=quimby.gnus.org@gnu.org Wed Jan 14 04:46:38 2004 Return-path: Original-Received: from quimby.gnus.org ([80.91.224.244]) by deer.gmane.org with esmtp (Exim 3.35 #1 (Debian)) id 1AgbzK-0002BW-00 for ; Wed, 14 Jan 2004 04:46:38 +0100 Original-Received: from monty-python.gnu.org ([199.232.76.173]) by quimby.gnus.org with esmtp (Exim 3.35 #1 (Debian)) id 1AgbzK-0006O3-00 for ; Wed, 14 Jan 2004 04:46:38 +0100 Original-Received: from localhost ([127.0.0.1] helo=monty-python.gnu.org) by monty-python.gnu.org with esmtp (Exim 4.24) id 1Agbwq-0007LU-7S for emacs-devel@quimby.gnus.org; Tue, 13 Jan 2004 22:44:04 -0500 Original-Received: from list by monty-python.gnu.org with tmda-scanned (Exim 4.24) id 1AgYEY-0000IA-GX for emacs-devel@gnu.org; Tue, 13 Jan 2004 18:46:06 -0500 Original-Received: from mail by monty-python.gnu.org with spam-scanned (Exim 4.24) id 1AgYDm-00006O-4l for emacs-devel@gnu.org; Tue, 13 Jan 2004 18:45:50 -0500 Original-Received: from [195.41.46.235] (helo=pfepa.post.tele.dk) by monty-python.gnu.org with esmtp (Exim 4.24) id 1AgXpt-00047m-9n for emacs-devel@gnu.org; Tue, 13 Jan 2004 18:20:37 -0500 Original-Received: from kfs-l.imdomain.dk.cua.dk (0x503e2644.bynxx3.adsl-dhcp.tele.dk [80.62.38.68]) by pfepa.post.tele.dk (Postfix) with SMTP id 5205B47FF65; Wed, 14 Jan 2004 00:20:30 +0100 (CET) Original-To: YAMAMOTO Mitsuharu In-Reply-To: <20040113.211033.74742347.mituharu@math.s.chiba-u.ac.jp> Original-Lines: 116 User-Agent: Gnus/5.09 (Gnus v5.9.0) Emacs/21.3.50 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.2 Precedence: list List-Id: Emacs development discussions. List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+emacs-devel=quimby.gnus.org@gnu.org Xref: main.gmane.org gmane.emacs.devel:19168 X-Report-Spam: http://spam.gmane.org/gmane.emacs.devel:19168 YAMAMOTO Mitsuharu writes: > >>>>> On Thu, 18 Dec 2003 22:57:00 +0900, YAMAMOTO Mitsuharu said: > > My assignment paper seems to be accepted. Because I've never made > larger patches to emacs, I'd like to post the patch for comments, both > on style and code. (That was suggested by Kim F. Storm. Thanks.) I have looked briefly on the code, and overall it looks really good. There are some things in the ChangeLogs which can be improved; see below. I cannot test your patch, but I suggest that you commit it, and see how it is received (provided that you are prepared to handle the complaints and bug reports :-) When you commit your changes to CVS, pls. commit one file at a time, and use the corresponding changelog entry for that file as cvs comment. You should write a small entry for etc/NEWS. Here are my comments on your change logs. Note that the is is not a complete list; there are other places that need similar changes. * dispextern.h: Change HAVE_CARBON to MAC_OS to cope with Mac OS 8/9 version. This applies to all of dispextern.h, so the following line is not needed: (struct glyph_string): Likewise. emacs.c (main): Likewise. frame.c (x_set_frame_parameters, x_report_frame_params) (x_set_fullscreen, x_set_border_width, Vdefault_frame_alist): Likewise. xdisp.c (define_frame_cursor1, expose_window, expose_frame): Likewise. The above entries should be moved to the relevant file sections below. For these, "Likewise" should be "Change HAVE_CARBON to MAC_OS". * macgui.h [MAC_OSX] (Carbon/Carbon.h): Add #include. Change this to: * macgui.h [MAC_OSX]: Include Carbon/Carbon.h. [MAC_OSX] (mktime, DEBUG, Z, free, malloc, realloc, max, min) (init_process): Add #undef and #define enclosing the above #include Move [MAX_OSX] to before the : and maybe simplify text to (mktime, DEBUG, Z, free, malloc, realloc, max, min) (init_process) [MAC_OSX] : Avoid conflicts with Carbon/Carbon.h. dispextern.h [MAC_OSX] (Carbon/Carbon.h): Remove #include and enclosing #undef and #define (now in macgui.h). macfns.c [MAC_OSX] (Carbon/Carbon.h): Likewise. macmenu.c [MAC_OSX] (Carbon/Carbon.h): Likewise. macterm.c [MAC_OSX] (Carbon/Carbon.h): Likewise. macterm.h [MAC_OSX] (Carbon/Carbon.h): Likewise. Again, move these lines to the relevant file sections. Also, simplify text to: [MAC_OSX] Do not include Carbon/Carbon.h (now in macgui.h). * macgui.h [!MAC_OSX] (QDOffscreen.h, Controls.h): Add #include. Write: * macgui.h [!MAC_OSX]: Include QDOffscreen.h and Controls.h. [MAC_OSX] (INFINITY): Add #undef to avoid the definition in math.h Write: (INFINITY) [MAC_OSX]: Avoid conflict with definition in math.h. * macterm.h (RED16_FROM_ULONG, GREEN16_FROM_ULONG, BLUE16_FROM_ULONG): New #define to extract 16-bit depth color Write: * macterm.h (RED16_FROM_ULONG, GREEN16_FROM_ULONG) (BLUE16_FROM_ULONG): New #define to extract 16-bit depth color In the long list of changes for macfns.c and macterm.c, you don't need to strictly follow the sequence of the code, so you can should collapse similar entries like these [not the complete list]: (x_set_cursor_type): Sync with xfns.c. (x_make_gc): Sync with xfns.c but enclose unused `border_tile' with #if 0. (Fxw_color_values): Sync with xfns.c. (valid_image_p, image_value_type, parse_image_spec): Sync with xfns.c. (image_ascent): Sync with xfns.c. (x_clear_image, x_alloc_image_color): Sync with xfns.c. into just one group: (x_set_cursor_type, x_make_gc, Fxw_color_values, valid_image_p) (image_value_type, parse_image_spec, image_ascent) (x_clear_image, x_alloc_image_color): Sync with xfns.c. Likewise for "Add declaration", "New function (from xfns.c)", etc. -- Kim F. Storm http://www.cua.dk