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: [RFC] some reworking of struct window Date: Thu, 21 Mar 2013 20:21:48 +0200 Message-ID: <83620kzk8z.fsf@gnu.org> References: <514AD54F.8050309@yandex.ru> Reply-To: Eli Zaretskii NNTP-Posting-Host: plane.gmane.org X-Trace: ger.gmane.org 1363890102 26734 80.91.229.3 (21 Mar 2013 18:21:42 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Thu, 21 Mar 2013 18:21:42 +0000 (UTC) Cc: emacs-devel@gnu.org To: Dmitry Antipov Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Thu Mar 21 19:22:06 2013 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 1UIk7n-00020R-TM for ged-emacs-devel@m.gmane.org; Thu, 21 Mar 2013 19:22:04 +0100 Original-Received: from localhost ([::1]:51857 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UIk7Q-0001zc-IM for ged-emacs-devel@m.gmane.org; Thu, 21 Mar 2013 14:21:40 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:34269) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UIk7M-0001wV-1Q for emacs-devel@gnu.org; Thu, 21 Mar 2013 14:21:37 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UIk7J-0007cH-Un for emacs-devel@gnu.org; Thu, 21 Mar 2013 14:21:35 -0400 Original-Received: from mtaout22.012.net.il ([80.179.55.172]:44136) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UIk7J-0007c4-MO for emacs-devel@gnu.org; Thu, 21 Mar 2013 14:21:33 -0400 Original-Received: from conversion-daemon.a-mtaout22.012.net.il by a-mtaout22.012.net.il (HyperSendmail v2007.08) id <0MK000I00VQW5G00@a-mtaout22.012.net.il> for emacs-devel@gnu.org; Thu, 21 Mar 2013 20:21:32 +0200 (IST) Original-Received: from HOME-C4E4A596F7 ([87.69.4.28]) by a-mtaout22.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0MK000H48WBVN970@a-mtaout22.012.net.il>; Thu, 21 Mar 2013 20:21:32 +0200 (IST) In-reply-to: <514AD54F.8050309@yandex.ru> X-012-Sender: halo1@inter.net.il X-detected-operating-system: by eggs.gnu.org: Solaris 10 X-Received-From: 80.179.55.172 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:158037 Archived-At: > Date: Thu, 21 Mar 2013 13:39:27 +0400 > From: Dmitry Antipov > > This patch implements the further generalization of an idea suggested by Stefan > at http://lists.gnu.org/archive/html/emacs-devel/2013-02/msg00088.html. The main > motivations are 1) make 'struct window' even bit smaller and 2) simplify the code > like: > > if (!NILP (w->hchild)) > foo (w->hchild); > else if (!NILP (w->vchild)) > foo (w->vchild); > else /* assume leaf window with w->buffer */ > bar (w->buffer); > > to: > > if (WINDOWP (w->object)) > foo (w->object); > else /* assume leaf window with buffer at w->object */ > bar (w->object); Thanks. > As usual, reviews and comments are highly appreciated, especially for the > 'struct window' member which replaces hchild/vchild/buffer (I don't like > too generic 'object', but couldn't think up the better name). Most of the changes are mechanical, so the review could be much more efficient if you could point out non-trivial ones. > @@ -838,16 +838,8 @@ > { > while (w) > { > - if (!NILP (w->hchild)) > - { > - eassert (WINDOWP (w->hchild)); > - clear_window_matrices (XWINDOW (w->hchild), desired_p); > - } > - else if (!NILP (w->vchild)) > - { > - eassert (WINDOWP (w->vchild)); > - clear_window_matrices (XWINDOW (w->vchild), desired_p); > - } > + if (WINDOWP (w->object)) > + clear_window_matrices (XWINDOW (w->object), desired_p); > else > { > if (desired_p) Here, you effectively lost the assertion that w->object can only be a window or a buffer. With the new code, if it's neither a window nor a buffer, you will behave as if it were a buffer, without checking. > - eassert (NILP (w->hchild) && NILP (w->vchild)); > + eassert (w->type == WINDOW_LEAF); Why do you need to store the WINDOW_LEAF type explicitly, if you can test w->object for being a buffer? And if you do need this type, why not use it instead of WINDOWP in the various tests above and below? > @@ -2069,22 +2060,18 @@ > if (!NILP (parent) && NILP (w->combination_limit)) > { > p = XWINDOW (parent); > - if (((!NILP (p->vchild) && !NILP (w->vchild)) > - || (!NILP (p->hchild) && !NILP (w->hchild)))) > + if (p->type == w->type && p->type > WINDOW_LEAF) ^^^^^^^^^^^^^^^^^^^^^ I think you are not supposed to compare enumerated types, except for equality. How exactly the compiler assigns numerical values to enumerated types is implementation-defined, I think. > - /* P's buffer slot may change from nil to a buffer. */ > - adjust_window_count (p, 1); Why did you remove this call? > - if (!NILP (w->vchild)) > - { > - delete_all_child_windows (w->vchild); > - wset_vchild (w, Qnil); > - } > - else if (!NILP (w->hchild)) > - { > - delete_all_child_windows (w->hchild); > - wset_hchild (w, Qnil); > - } > - else if (!NILP (w->buffer)) > + if (WINDOWP (w->object)) > + { > + delete_all_child_windows (w->object); > + w->object = Qnil; > + } > + else > { > unshow_buffer (w); > unchain_marker (XMARKER (w->pointm)); Maybe instead of just "else" you should make sure w->object is a buffer. > +/* Window types, see the comment above. */ > + > +enum window_type > + { > + WINDOW_LEAF, > + WINDOW_HORIZONTAL_COMBINATION, > + WINDOW_VERTICAL_COMBINATION, > + }; The test BUFFERP (w->object) already gives you the same info as WINDOW_LEAF type, no?