Bug in `next-line' when last char in line has after-string property

Bug in `next-line' when last char in line has after-string property

[Ben North]

[-- Attachment #1: Type: text/plain, Size: 816 bytes --]

The attached file shows a bug in 22.0.92.  To reproduce, launch "emacs
-Q".  (The bug is exhibited with or without the "-nw" option.)
`find-file' the attached file.  Make sure your window is narrow enough
that the initial line of `-' characters wraps.  Go to the end of the
buffer (M->) and do `eval-last-sexp' (C-x C-e).  The desired behaviour
is that point ends up in the first column of the second line.  It in
fact ends up in the first column of the third line.

You can carry on playing with this: go to the very start of the buffer,
and hit C-n --- point moves down two lines instead of one.  I looked
into this a bit, which is what lead to the attached test-case using only
functions implemented in C, but I don't know the C code well enough to
dig further I'm afraid.  Hope the report is useful nonetheless.

[-- Attachment #2: line-motion-bug.el --]
[-- Type: application/octet-stream, Size: 509 bytes --]

; ------------------------------------------------------------------------------------------------------------------------------------

(progn (setq overlay
             (let ((end-of-first-line (save-excursion (goto-char 1) (end-of-line) (point))))
               (make-overlay (1- end-of-first-line) end-of-first-line))
             truncate-lines t)
       (overlay-put overlay 'after-string "X")
       (progn
         (goto-char 1)
         (goto-char (line-end-position))
         (vertical-motion 1)))

[-- Attachment #3: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: Bug in `next-line' when last char in line has after-string property

[Kim F. Storm]

Ben North <ben@redfrontdoor.org> writes:

> The attached file shows a bug in 22.0.92.  To reproduce, launch "emacs
> -Q".  (The bug is exhibited with or without the "-nw" option.)
> `find-file' the attached file.  Make sure your window is narrow enough
> that the initial line of `-' characters wraps.  Go to the end of the
> buffer (M->) and do `eval-last-sexp' (C-x C-e).  The desired behaviour
> is that point ends up in the first column of the second line.  It in
> fact ends up in the first column of the third line.


Here is a patch which fixes this specific problem, but I don't know
whether it will cause other problems, as the "overshoot" check
on strings was added to fix some other bug.

This just shows that the current overshoot check in vertial-motion
is pretty fragile ... e.g. what if there is another overlay string
after the current string which _does_ have a newline in it.

Hm, we probably need to rework this _after_ the release.


*** indent.c	20 Nov 2006 09:32:04 +0100	1.187
--- indent.c	04 Jan 2007 14:02:39 +0100	
***************
*** 2074,2080 ****
      {
        int it_start;
        int oselective;
!       int it_overshoot_expected_p;
  
        SET_TEXT_POS (pt, PT, PT_BYTE);
        start_display (&it, w, pt);
--- 2074,2080 ----
      {
        int it_start;
        int oselective;
!       int it_overshoot_expected;
  
        SET_TEXT_POS (pt, PT, PT_BYTE);
        start_display (&it, w, pt);
***************
*** 2100,2111 ****
  	  while (s < e && *s != '\n')
  	    ++s;
  
! 	  it_overshoot_expected_p = (s == e);
  	}
        else
! 	it_overshoot_expected_p = (it.method == GET_FROM_IMAGE
! 				   || it.method == GET_FROM_STRETCH
! 				   || it.method == GET_FROM_COMPOSITION);
  
        reseat_at_previous_visible_line_start (&it);
        it.current_x = it.hpos = 0;
--- 2100,2115 ----
  	  while (s < e && *s != '\n')
  	    ++s;
  
! 	  /* If there is no newline in the string, we need to check
! 	     whether there is a newline immediately after the string
! 	     in move_it_to below.  This may happen if there is an
! 	     overlay with an after-string just before the newline.  */
! 	  it_overshoot_expected = (s == e) ? -1 : 0;
  	}
        else
! 	it_overshoot_expected = (it.method == GET_FROM_IMAGE
! 				 || it.method == GET_FROM_STRETCH
! 				 || it.method == GET_FROM_COMPOSITION);
  
        reseat_at_previous_visible_line_start (&it);
        it.current_x = it.hpos = 0;
***************
*** 2119,2125 ****
  	 truncate-lines is on and PT is beyond right margin.
  	 Don't go back if the overshoot is expected (see above).  */
        if (IT_CHARPOS (it) > it_start && XINT (lines) > 0
! 	  && !it_overshoot_expected_p)
  	move_it_by_lines (&it, -1, 0);
  
        it.vpos = 0;
--- 2123,2132 ----
  	 truncate-lines is on and PT is beyond right margin.
  	 Don't go back if the overshoot is expected (see above).  */
        if (IT_CHARPOS (it) > it_start && XINT (lines) > 0
! 	  && (!it_overshoot_expected
! 	      || (it_overshoot_expected < 0
! 		  && it.method == GET_FROM_BUFFER
! 		  && it.c == '\n')))
  	move_it_by_lines (&it, -1, 0);
  
        it.vpos = 0;

>
> You can carry on playing with this: go to the very start of the buffer,
> and hit C-n --- point moves down two lines instead of one.  I looked
> into this a bit, which is what lead to the attached test-case using only
> functions implemented in C, but I don't know the C code well enough to
> dig further I'm afraid.  Hope the report is useful nonetheless.
>
> ; ------------------------------------------------------------------------------------------------------------------------------------
>
> (progn (setq overlay
>              (let ((end-of-first-line (save-excursion (goto-char 1) (end-of-line) (point))))
>                (make-overlay (1- end-of-first-line) end-of-first-line))
>              truncate-lines t)
>        (overlay-put overlay 'after-string "X")
>        (progn
>          (goto-char 1)
>          (goto-char (line-end-position))
>          (vertical-motion 1)))
> _______________________________________________
> Emacs-devel mailing list
> Emacs-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/emacs-devel

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

Re: Bug in `next-line' when last char in line has after-string property

[Richard Stallman]

    This just shows that the current overshoot check in vertial-motion
    is pretty fragile ... e.g. what if there is another overlay string
    after the current string which _does_ have a newline in it.

I think you should install this; it might cause another bug,
but it will probably be more obscure than the one you are fixing.

    Hm, we probably need to rework this _after_ the release.

Yes, I think so.

Re: Bug in `next-line' when last char in line has after-string property

[Kim F. Storm]

Richard Stallman <rms@gnu.org> writes:

>     This just shows that the current overshoot check in vertial-motion
>     is pretty fragile ... e.g. what if there is another overlay string
>     after the current string which _does_ have a newline in it.
>
> I think you should install this; it might cause another bug,
> but it will probably be more obscure than the one you are fixing.

Done.

Everybody, please watch out for problems with line movement in buffers
with overlay and text-property strings.

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

Re: Bug in `next-line' when last char in line has after-string property

[Ralf Angeli]

* Kim F. Storm (2007-01-05) writes:

> Everybody, please watch out for problems with line movement in buffers
> with overlay and text-property strings.

If you execute the following code which inserts a few strings into the
buffer *foo* and then puts three overlays in the buffer where each one
is covering a line break in the buffer and using a line break in the
display string, the line in the middle will be left out if one tries
to move point up using `C-p' several times.  Downwards with `C-n' is
okay.

This is not a regression, but also happened like this before your last
change.  It just caught my eye when checking for problems with the new
code.

(progn
  (pop-to-buffer (get-buffer-create "*foo*"))
  (dotimes (i 5)
    (insert (make-string 10 ?x) "\n"))
  (let ((ov (make-overlay 17 24)))
    (overlay-put ov 'display "aaa\n"))
  (let ((ov (make-overlay 29 35)))
    (overlay-put ov 'display "aaa\n"))
  (let ((ov (make-overlay 40 46)))
    (overlay-put ov 'display "aaa\n")))

-- 
Ralf

Re: Bug in `next-line' when last char in line has after-string property

[Kim F. Storm]

Ralf Angeli <angeli@caeruleus.net> writes:

> * Kim F. Storm (2007-01-05) writes:
>
>> Everybody, please watch out for problems with line movement in buffers
>> with overlay and text-property strings.
>
> If you execute the following code which inserts a few strings into the
> buffer *foo* and then puts three overlays in the buffer where each one
> is covering a line break in the buffer and using a line break in the
> display string, the line in the middle will be left out if one tries
> to move point up using `C-p' several times.  Downwards with `C-n' is
> okay.
>
> This is not a regression, but also happened like this before your last
> change.  It just caught my eye when checking for problems with the new
> code.
>
> (progn
>   (pop-to-buffer (get-buffer-create "*foo*"))
>   (dotimes (i 5)
>     (insert (make-string 10 ?x) "\n"))
>   (let ((ov (make-overlay 17 24)))
>     (overlay-put ov 'display "aaa\n"))
>   (let ((ov (make-overlay 29 35)))
>     (overlay-put ov 'display "aaa\n"))
>   (let ((ov (make-overlay 40 46)))
>     (overlay-put ov 'display "aaa\n")))

Indeed.  It is pretty easy to construct expamples like that which
break the current code.  It needs a big overhaul after the release.

But thanks for testing -- and testing that the latest change 
didn't make things worse :-)

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