Re: [alinsoar@voila.fr: a bug in global-hl-line-mode]

Re: [alinsoar@voila.fr: a bug in global-hl-line-mode]

[A Soare]

> I believe it was fixed by this change, but it would be nice
> if A Soare would verify that it does.
> 
> 2007-03-10  Kim F. Storm  <storm@cua.dk>
> 
> 	* xdisp.c (redisplay_window): Don't automatically select a new window
> 	start for a contination line during mouse-click.
> 

I cheched, and selection is ok. But the cursor position is wrong.

When I am at the middle of the line of the output syntax, and I press left button, the cursor jumps on the column where it has to jump, but the cursor's line is wrong.

Re: [alinsoar@voila.fr: a bug in global-hl-line-mode]

[Kim F. Storm]

A Soare <alinsoar@voila.fr> writes:

>> I believe it was fixed by this change, but it would be nice
>> if A Soare would verify that it does.
>> 
>> 2007-03-10  Kim F. Storm  <storm@cua.dk>
>> 
>> 	* xdisp.c (redisplay_window): Don't automatically select a new window
>> 	start for a contination line during mouse-click.
>> 
>
> I cheched, and selection is ok. But the cursor position is wrong.
>
> When I am at the middle of the line of the output syntax, and I
> press left button, the cursor jumps on the column where it has to
> jump, but the cursor's line is wrong.

The cursor may seems to end up on the wrong line, but it should be
placed on the BUFFER POSITION where you clicked with the mouse.

This is because the screen may scroll a few lines as a result of the
click in such a line, so it just looks wrong.

Please double check!


That scrolling is very hard to get rid of -- so that's for after the
release.


-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

delete-overlay causes recentering (was: [alinsoar@voila.fr: a bug in global-hl-line-mode])

[Johan Bockgård]

Thread started at
http://lists.gnu.org/archive/html/emacs-devel/2007-03/msg00360.html


storm@cua.dk (Kim F. Storm) writes:

> The cursor may seems to end up on the wrong line, but it should be
> placed on the BUFFER POSITION where you clicked with the mouse.
>
> This is because the screen may scroll a few lines as a result of the
> click in such a line, so it just looks wrong.
>
> Please double check!
>
>
> That scrolling is very hard to get rid of -- so that's for after the
> release.

It seems that in the situation above the scrolling is caused by
delete-overlay, not by the click.


Here's another example:

$ emacs -Q

Evaluate the code below. Then press f8, f9, f8, f9, ...

;;;;;;;;;;;;;;;;
(defvar ov nil)

(defun foo ()
  (interactive)
  (set-window-start (selected-window) 834)
  (goto-char 1237)
  (setq ov (make-overlay 1237 1242))
  (overlay-put ov 'face 'highlight))

(defun bar ()
  (interactive)
  (delete-overlay ov))

(global-set-key [f8] 'foo)
(global-set-key [f9] 'bar)

(progn
  (switch-to-buffer "*foo*")
  (dotimes (n 30)
    (insert "Emacs is the extensible, customizable, "
            "self-documenting real-time display editor. "))
  (goto-char (point-min))
  (insert " "))
;;;;;;;;;;;;;;;;


delete-overlay causes a recentering. (Actually, so do mouse clicks in
this case.)

There is no scrolling if the last edit was after the window start,
however (remove `(goto-char (point-min))' or make a change at the end
of the buffer)!

I can also see the "spurious scrolling bug" mentioned in FOR-RELEASE
in the *foo* buffer. Occasionally the text jumps while just looking at
it.

-- 
Johan Bockgård

Re: delete-overlay causes recentering

[martin rudalics]

> Here's another example:
> 
> $ emacs -Q
> 
> Evaluate the code below. Then press f8, f9, f8, f9, ...

Can you give an example where this shows up with `truncate-lines' non-nil?

Re: delete-overlay causes recentering

[Johan Bockgård]

martin rudalics <rudalics@gmx.at> writes:

>> Here's another example:
>>
>> $ emacs -Q
>>
>> Evaluate the code below. Then press f8, f9, f8, f9, ...
>
> Can you give an example where this shows up with `truncate-lines'
> non-nil?

No.

-- 
Johan Bockgård

Re: delete-overlay causes recentering

[Chong Yidong]

bojohan+news@dd.chalmers.se (Johan Bockgård) writes:

> Evaluate the code below. Then press f8, f9, f8, f9, ...
>
> (defvar ov nil)
>
> (defun foo ()
>   (interactive)
>   (set-window-start (selected-window) 834)
>   (goto-char 1237)
>   (setq ov (make-overlay 1237 1242))
>   (overlay-put ov 'face 'highlight))
>
> (defun bar ()
>   (interactive)
>   (delete-overlay ov))
>
> (global-set-key [f8] 'foo)
> (global-set-key [f9] 'bar)
>
> (progn
>   (switch-to-buffer "*foo*")
>   (dotimes (n 30)
>     (insert "Emacs is the extensible, customizable, "
>             "self-documenting real-time display editor. "))
>   (goto-char (point-min))
>   (insert " "))
>
> delete-overlay causes a recentering. (Actually, so do mouse clicks in
> this case.)

The behavior arose from the following change:

2006-04-20  Kim F. Storm  <storm@cua.dk>

	* xdisp.c (redisplay_window): Fix last change.

	* xdisp.c (redisplay_window): If current window start is not at the
	beginning of a line, select a new window start if buffer is modified
	and window start is in the modified region, but the first change is
	before window start.


*** emacs/src/xdisp.c	2006/04/13 01:21:48	1.1086
--- emacs/src/xdisp.c	2006/04/20 08:46:56	1.1088
***************
*** 12689,12696 ****
        /* IT may overshoot PT if text at PT is invisible.  */
        else if (IT_CHARPOS (it) > PT && CHARPOS (startp) <= PT)
  	w->force_start = Qt;
- 
- 
      }
  
    /* Handle case where place to start displaying has been specified,
--- 12689,12694 ----
***************
*** 12860,12865 ****
--- 12858,12892 ----
  	       || (XFASTINT (w->last_modified) >= MODIFF
  		   && XFASTINT (w->last_overlay_modified) >= OVERLAY_MODIFF)))
      {
+ 
+       /* If first window line is a continuation line, and window start
+ 	 is inside the modified region, but the first change is before
+ 	 current window start, we must select a new window start.*/
+       if (NILP (w->start_at_line_beg))
+ 	{
+ 	  /* Make sure beg_unchanged and end_unchanged are up to date.
+ 	     Do it only if buffer has really changed.  This may or may
+ 	     not have been done by try_window_id (see which) already. */
+ 	  if (MODIFF > SAVE_MODIFF
+ 	      /* This seems to happen sometimes after saving a buffer.  */
+ 	      || BEG_UNCHANGED + END_UNCHANGED > Z_BYTE)
+ 	    {
+ 	      if (GPT - BEG < BEG_UNCHANGED)
+ 		BEG_UNCHANGED = GPT - BEG;
+ 	      if (Z - GPT < END_UNCHANGED)
+ 		END_UNCHANGED = Z - GPT;
+ 	    }
+ 
+ 	  if (CHARPOS (startp) > BEG + BEG_UNCHANGED
+ 	      && CHARPOS (startp) <= Z - END_UNCHANGED)
+ 	    {
+ 	      /* There doesn't seems to be a simple way to find a new
+ 		 window start that is near the old window start, so
+ 		 we just recenter.  */
+ 	      goto recenter;
+ 	    }
+ 	}
+ 
  #if GLYPH_DEBUG
        debug_method_add (w, "same window start");
  #endif

Re: delete-overlay causes recentering

[Chong Yidong]

Chong Yidong <cyd@stupidchicken.com> writes:

> The behavior arose from the following change:
>
> 2006-04-20  Kim F. Storm  <storm@cua.dk>
>
> 	* xdisp.c (redisplay_window): Fix last change.
>
> 	* xdisp.c (redisplay_window): If current window start is not at the
> 	beginning of a line, select a new window start if buffer is modified
> 	and window start is in the modified region, but the first change is
> 	before window start.

Also, the original bug report that led to this change is here:

http://lists.gnu.org/archive/html/emacs-pretest-bug/2006-04/msg00136.html

Re: delete-overlay causes recentering

[Chong Yidong]

Chong Yidong <cyd@stupidchicken.com> writes:

>> The behavior arose from the following change:
>>
>> 2006-04-20  Kim F. Storm  <storm@cua.dk>
>>
>> 	* xdisp.c (redisplay_window): Fix last change.
>>
>> 	* xdisp.c (redisplay_window): If current window start is not at the
>> 	beginning of a line, select a new window start if buffer is modified
>> 	and window start is in the modified region, but the first change is
>> 	before window start.

The problem is in the part of the code at xdisp.c:13145:

    /* Make sure beg_unchanged and end_unchanged are up to date.
       Do it only if buffer has really changed.  This may or may
       not have been done by try_window_id (see which) already. */
    if (MODIFF > SAVE_MODIFF
        /* This seems to happen sometimes after saving a buffer.  */
        || BEG_UNCHANGED + END_UNCHANGED > Z_BYTE)
      {
        if (GPT - BEG < BEG_UNCHANGED)
          BEG_UNCHANGED = GPT - BEG;
        if (Z - GPT < END_UNCHANGED)
          END_UNCHANGED = Z - GPT;
      }

This code seems to be copied from try_window_id.  I am not sure what
the original justification for this is, but it is a little too strict
when it comes to overlay changes.

That's because modify_overlay (buffer.c:3703), which is used to mark
overlay changes, calls BUF_COMPUTE_UNCHANGED, which alters
BUF_BEG_UNCHANGED and BUF_END_UNCHANGED directly without moving the
gap.  However, the result is that the "make sure beg_unchanged and
end_unchanged are up to date" check, which compares these to the
actual gap positions, thinks more of the buffer has been modified than
it actually has.  It then proceeds to recenter the display.

If we use the original values of BEG_UNCHANGED and END_UNCHANGED,
before it is "updated" by the above code, the spurious recentering
does not take place.  The original (2006) test case also seems to
behave OK -- in that case, we recenter correctly.  However, I can't be
certain this is correct because I don't know the original
justification for the above check.

WDYT?

Re: delete-overlay causes recentering

[Kim F. Storm]

Chong Yidong <cyd@stupidchicken.com> writes:

> If we use the original values of BEG_UNCHANGED and END_UNCHANGED,
> before it is "updated" by the above code, the spurious recentering
> does not take place.  The original (2006) test case also seems to
> behave OK -- in that case, we recenter correctly.  However, I can't be
> certain this is correct because I don't know the original
> justification for the above check.

The justification was to make BEG_UNCHANGED and END_UNCHANGED up to date
(similar to try_window_id) before the test in the following lines.

If you say that is not necessary, it is ok with me to remove that code
(but perhaps it would be better to just put #if 0 around it and add
a comment which explains why it is not necessary).

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: delete-overlay causes recentering

[Johan Bockgård]

Chong Yidong <cyd@stupidchicken.com> writes:

> If we use the original values of BEG_UNCHANGED and END_UNCHANGED,
> before it is "updated" by the above code, the spurious recentering
> does not take place.  The original (2006) test case also seems to
> behave OK -- in that case, we recenter correctly.  However, I can't be
> certain this is correct because I don't know the original
> justification for the above check.

Thanks a lot for looking at this, but the new behavior is worse:

When a window starts is in a continued line, it is recentered whenever
the window is unselected--by activating the minibuffer, switching to
another window, or switching to another frame.

-- 
Johan Bockgård

Re: delete-overlay causes recentering

[Chong Yidong]

bojohan+news@dd.chalmers.se (Johan Bockgård) writes:

> Chong Yidong <cyd@stupidchicken.com> writes:
>
>> If we use the original values of BEG_UNCHANGED and END_UNCHANGED,
>> before it is "updated" by the above code, the spurious recentering
>> does not take place.  The original (2006) test case also seems to
>> behave OK -- in that case, we recenter correctly.  However, I can't be
>> certain this is correct because I don't know the original
>> justification for the above check.
>
> Thanks a lot for looking at this, but the new behavior is worse:
>
> When a window starts is in a continued line, it is recentered whenever
> the window is unselected--by activating the minibuffer, switching to
> another window, or switching to another frame.

I know an easy way to fix this.  The original patch that introduced
this problem in 2006 was intended to prevent an even more serious
problem---redisplay being completely garbled when window starts in a
continued line.  I think it will have to wait till after the release.

Re: delete-overlay causes recentering

[Chong Yidong]

Chong Yidong <cyd@stupidchicken.com> writes:

> I know an easy way to fix this.  The original patch that introduced
   ^
  don't

Re: delete-overlay causes recentering

[Stefan Monnier]

>> I know an easy way to fix this.  The original patch that introduced
>    ^
>   don't

:-(


        Stefan

Re: delete-overlay causes recentering

[Richard Stallman]

    > If we use the original values of BEG_UNCHANGED and END_UNCHANGED,
    > before it is "updated" by the above code, the spurious recentering
    > does not take place.  The original (2006) test case also seems to
    > behave OK -- in that case, we recenter correctly.  However, I can't be
    > certain this is correct because I don't know the original
    > justification for the above check.

    Thanks a lot for looking at this, but the new behavior is worse:

    When a window starts is in a continued line, it is recentered whenever
    the window is unselected--by activating the minibuffer, switching to
    another window, or switching to another frame.

That is the spurious scrolling bug I have mentioned.  Thank you
for connecting it with specific code.  Now it should be possible
to track it down.

Yidong wrote:

    I don't know an easy way to fix this.

Can you describe the series of events that cause the
spurious recentering?

Re: delete-overlay causes recentering

[Chong Yidong]

Richard Stallman <rms@gnu.org> writes:

>     When a window starts is in a continued line, it is recentered whenever
>     the window is unselected--by activating the minibuffer, switching to
>     another window, or switching to another frame.
>
> That is the spurious scrolling bug I have mentioned.  Thank you
> for connecting it with specific code.  Now it should be possible
> to track it down.
>
> Yidong wrote:
>
>     I don't know an easy way to fix this.
>
> Can you describe the series of events that cause the
> spurious recentering?

KFS described the problem in the original change that led to this:

  When the filling command has reformatted the buffer text, the
  redisplay code still starts window display at the old window start
  -- which still is in the middle of a line, but not at the same
  relative position in the line as before ... so it gets confused.

  This is just one way in which a buffer change can mess things up
  when window start is not at the start of a line, so I think it is
  generally a bit difficult to find a method which will always select
  the intuitively best window start after such a change.

As a result, there will be times when the redisplay engine must
recenter, simply to avoid screen corruption.

On the other hand, I just realized that the "recentering due to
minibuffer-activation" problem is a simple oversight on my part, when
I was fixing the recentering to make it less aggressive.  I've checked
in a small fix.

Re: delete-overlay causes recentering

[Richard Stallman]

      When the filling command has reformatted the buffer text, the
      redisplay code still starts window display at the old window start
      -- which still is in the middle of a line, but not at the same
      relative position in the line as before ... so it gets confused.

      This is just one way in which a buffer change can mess things up
      when window start is not at the start of a line, so I think it is
      generally a bit difficult to find a method which will always select
      the intuitively best window start after such a change.

In that case, it is supposed to recenter.  However, the bug of
spurious recentering occurs when the buffer has not been changed at
all.  It should be easy to distinguish that case and recognize that
there is no need to recenter.


In any case, the bug is not fixed.  I observed it after C-c C-c to
send a message, in an Emacs I built yesterday which has your fix in
it.

Re: delete-overlay causes recentering

[Chong Yidong]

Richard Stallman <rms@gnu.org> writes:

> In any case, the bug is not fixed.  I observed it after C-c C-c to
> send a message, in an Emacs I built yesterday which has your fix in
> it.

There is probably some code in rmail.el that has to be updated.  I
don't use Rmail, and I don't know the code well enough to change it.
I also think it should not block the release; it'd hardly a critical
glitch.

Re: delete-overlay causes recentering

[Richard Stallman]

    > In any case, the bug is not fixed.  I observed it after C-c C-c to
    > send a message, in an Emacs I built yesterday which has your fix in
    > it.

    There is probably some code in rmail.el that has to be updated.

If you are right, that is a way to fix the bug.
Can you find the code that needs to be updated?

However, your suggestion leads me to imagine a different possible
cause.  The editing which sendmail-send-it does in various buffers
sets BEG_UNCHANGED and END_UNCHANGED in some of them, and the values
for the wrong buffer get used.  Would you like to see if that is it?