[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC] some reworking of struct window

From: Dmitry Antipov
Subject: Re: [RFC] some reworking of struct window
Date: Fri, 22 Mar 2013 10:13:42 +0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130307 Thunderbird/17.0.4

On 03/21/2013 10:21 PM, Eli Zaretskii wrote:

@@ -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);
          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.

No, because clear_window_matrices doesn't check whether w->buffer is not
nil and call clear_glyph_matrix anyway.

I assume that the most of display-related functions should not be called
for the deleted windows. E.g. this code just silences possible error:

if (WINDOWP (w->object))
  foo (w->object);
else if (BUFFERP (w->object))
  bar (w->object);

The following is better since XBUFFER implies eassert if --enable-checking:

if (WINDOWP (w->object))
  foo (w->object);
  bar (XBUFFER (w->object));


if (WINDOWP (w->object))
  foo (w->object);
else if (BUFFERP (w->object))
  bar (w->object);
  emacs_abort ();

The latter example leaves the conditional call to emacs_abort even without
--enable-checking. I suspect that this is too paranoid, and would prefer:

eassert (!NILP (w->object));
if (WINDOWP (w->object))
  foo (w->object);
  bar (w->object);

-  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?

Since sync_window_with_frame_matrix_rows assumes live window,
eassert (BUFFERP (w->object)) may be better; but...

And if you do need this type, why
not use it instead of WINDOWP in the various tests above and below?

this is not always the same because w->type == WINDOW_xxx_COMBINATION
may be true for the deleted window too.

@@ -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.


"An enumerator with = defines its enumeration constant as the value of the 
expression. If the first enumerator has no =, the value of its enumeration 
is 0. Each subsequent enumerator with no = defines its enumeration constant as 
value of the constant expression obtained by adding 1 to the value of the 
enumeration constant".

This is from the latest C99 draft 
(http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf "Enumeration specifiers"), but I believe it was so since long time ago.

-  /* P's buffer slot may change from nil to a buffer.  */
-  adjust_window_count (p, 1);

Why did you remove this call?

Because I also remove wset_buffer (p, Qnil) few lines below, and per-buffer 
window counters
should be balanced.

+/* Window types, see the comment above.  */
+enum window_type
+  {
+  };

The test BUFFERP (w->object) already gives you the same info as
WINDOW_LEAF type, no?

Of course, it's possible to avoid enum window_type at all and use something 

unsigned combination : 1;
unsigned horizontal : 1;

in struct window. Then we will have

w->type == WINDOW_LEAF equal to !w->combination;
w->type == WINDOW_VERTICAL_COMBINATION equal to w->combination && 
w->type == WINDOW_HORIZONTAL_COMBINATION equal to w->combination && 

I'll try this design too.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]