emacs-devel
[Top][All Lists]
Advanced

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

Latest changes in redisplay code


From: Eli Zaretskii
Subject: Latest changes in redisplay code
Date: Mon, 04 Feb 2013 18:25:20 +0200

The few last changesets in this area sneaked in questionable changes
which IMO should have been discussed.  I gave one example here:

  http://debbugs.gnu.org/cgi/bugreport.cgi?bug=13623#8

But there are more of them.

  -    /* If this field is nil, it means we don't have a base line.
  -       If it is a buffer, it means don't display the line number
  -       as long as the window shows that buffer.  */
  -    Lisp_Object base_line_pos;
  [...]
  +    /* If this field is zero, it means we don't have a base line.
  +       If it is -1, it means don't display the line number as long
  +       as the window shows its buffer.  */
  +    ptrdiff_t base_line_pos;
  [...]
          /* If we decided that this buffer isn't suitable for line numbers,
             don't forget that too fast.  */
  -     if (EQ (w->base_line_pos, w->buffer))
  +     if (w->base_line_pos == -1)
            goto no_value;
  -     /* But do forget it, if the window shows a different buffer now.  */
  -     else if (BUFFERP (w->base_line_pos))
  -       wset_base_line_pos (w, Qnil);

This change loses the ability to track the buffer whose line numbers
are being displayed on the mode line.  This means we will now never
show line numbers in this window, even after the buffer it displays
changes.  (I just tried that, and it really is so -- another bug.)

          /* If the buffer is very big, don't waste time.  */
          if (INTEGERP (Vline_number_display_limit)
              && BUF_ZV (b) - BUF_BEGV (b) > XINT (Vline_number_display_limit))
            {
  -         wset_base_line_pos (w, Qnil);
  -         wset_base_line_number (w, Qnil);
  +         w->base_line_pos = 0;
  +         w->base_line_number = 0;
              goto no_value;
            }

You set base_line_pos here to zero, not to -1: why?

Revision 111584 also has a suspicious change:

  @@ -3189,7 +3182,7 @@ set_window_buffer (Lisp_Object window, L
     wset_window_end_pos (w, make_number (0));
     wset_window_end_vpos (w, make_number (0));
     memset (&w->last_cursor, 0, sizeof w->last_cursor);
  -  wset_window_end_valid (w, Qnil);
  +

Why was this setting dropped?

And what about these two:

  @@ -13758,7 +13756,7 @@ mark_window_display_accurate_1 (struct w
         else
          w->last_point = marker_position (w->pointm);

  -      wset_window_end_valid (w, w->buffer);
  +      w->window_end_valid = 1;
  @@ -27782,7 +27779,7 @@ note_mouse_highlight (struct frame *f, i
        And verify the buffer's text has not changed.  */
     b = XBUFFER (w->buffer);
     if (part == ON_TEXT
  -      && EQ (w->window_end_valid, w->buffer)
  +      && w->window_end_valid

Why don't we need to assign a buffer to w->window_end_valid?  What
purpose did this value serve, and why did you decide it was no longer
needed?

I'm afraid we are throwing a lot of babies with bath water here.  This
all is just based on cursory reading of two recent changesets, I
wonder what I might discover if I invest more time.

How can we make this process less dangerous?



reply via email to

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