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: Re: Latest changes in redisplay code
Date: Tue, 05 Feb 2013 20:01:21 +0200	[thread overview]
Message-ID: <83d2weabda.fsf@gnu.org> (raw)
In-Reply-To: <51109E0B.3000402@yandex.ru>

> Date: Tue, 05 Feb 2013 09:52:11 +0400
> From: Dmitry Antipov <dmantipov@yandex.ru>
> CC: Emacs development discussions <emacs-devel@gnu.org>
> 
> On 02/04/2013 08:25 PM, Eli Zaretskii wrote:
> 
> > 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.)
> 
> Could you please provide an example? I don't see the way to reproduce it.

Forget it, my testing was wrong.  I always forget that
line-number-display-limit is counted in characters, not lines.  And it
didn't help that the change in adjust_window_count was not mentioned
at all in the ChangeLog entry for this set.  (Please do make sure you
describe all the changes there, it's important for analyzing who
changed what and why.)

In any case, the potential for problems is still there, because now
w->base_line_pos gets reset to zero in many more places than it was
before.  Previously, only xdisp.c would manipulate this attribute in a
few well-defined places.  Now it can be reset by any function that
calls adjust_window_count, including wset_buffer, which is called from
many primitives, most of them outside xdisp.c.  Therefore, it's quite
possible that we will now have to count from the beginning of the
buffer much more frequently than we did before.  In large buffers,
this could be quite a hit.

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

You didn't answer that.

> > How can we make this process less dangerous?
> 
> Hm. Although I agree that I had to test 111647 more carefully, there always will
> be the things which can't be tested by just one developer (like 111594, for example).
> As for the redisplay, automated testing is practically impossible; at least, it would
> be nice to have a set of "non-typical" (beyond basic editing) use cases which should
> trigger some corner-case situations.

Granted, I'm well aware of that.  It's precisely _because_ of this
lack of a comprehensive test suite that I'm worried by such changes.

I can only suggest to try to minimize the danger by thorough
discussions of every non-trivial changeset.  I realize that this will
slow down development to some extent, but I see no other way to make
sure we don't introduce hard to find bugs.

The display code is tricky, and the number of obscure display features
in Emacs is mind-boggling.  The result is that no single individual
can possibly grasp all the possible implications of a change in core
data structures and algorithms.  Discussing such changes can go a long
way towards uncovering potential pitfalls in advance.

Btw, here's one more questionable change, from revision 111583:

  @@ -13784,25 +13777,21 @@ mark_window_display_accurate (Lisp_Objec
     for (; !NILP (window); window = w->next)
       {
	 w = XWINDOW (window);
  -      mark_window_display_accurate_1 (w, accurate_p);
  -
	 if (!NILP (w->vchild))
	  mark_window_display_accurate (w->vchild, accurate_p);
  -      if (!NILP (w->hchild))
  +      else if (!NILP (w->hchild))
	  mark_window_display_accurate (w->hchild, accurate_p);
  +      else if (BUFFERP (w->buffer))
  +       mark_window_display_accurate_1 (w, accurate_p);

Isn't it possible for a window to have both an hchild and a vchild?



  reply	other threads:[~2013-02-05 18:01 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-04 16:25 Latest changes in redisplay code Eli Zaretskii
2013-02-04 17:04 ` martin rudalics
2013-02-05  5:52 ` Dmitry Antipov
2013-02-05 18:01   ` Eli Zaretskii [this message]
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=83d2weabda.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).