[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC] some reworking of struct window
From: |
Eli Zaretskii |
Subject: |
Re: [RFC] some reworking of struct window |
Date: |
Thu, 21 Mar 2013 20:21:48 +0200 |
> Date: Thu, 21 Mar 2013 13:39:27 +0400
> From: Dmitry Antipov <address@hidden>
>
> 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?
Re: [RFC] some reworking of struct window, Dmitry Antipov, 2013/03/22