unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Dmitry Antipov <dmantipov@yandex.ru>
Cc: emacs-devel@gnu.org
Subject: Latest changes in redisplay code
Date: Mon, 04 Feb 2013 18:25:20 +0200	[thread overview]
Message-ID: <83pq0g9hcf.fsf@gnu.org> (raw)

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	other threads:[~2013-02-04 16:25 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-04 16:25 Eli Zaretskii [this message]
2013-02-04 17:04 ` Latest changes in redisplay code martin rudalics
2013-02-05  5:52 ` Dmitry Antipov
2013-02-05 18:01   ` Eli Zaretskii
2013-02-05 20:58     ` martin rudalics
2013-02-06 15:39       ` Stefan Monnier
2013-02-06  7:37     ` Dmitry Antipov
2013-02-06 18:08       ` Eli Zaretskii
2013-02-07 15:42         ` Dmitry Antipov
2013-02-08 14:11           ` Eli Zaretskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=83pq0g9hcf.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=dmantipov@yandex.ru \
    --cc=emacs-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).