Re: Bug in vertical-motion vs overlay with display property

[Eli Zaretskii]

> From: Karl Chen <address@hidden>
> CC: address@hidden,
>     Emacs Developement List <address@hidden>
> Date: Fri, 12 Apr 2013 19:34:24 -0400
> 
> Hi Eli/Emacs-devel,
> 
> I believe I've discovered a small bug in `vertical-motion' in
> indent.c.  It affects movement past blank lines when there is an
> overlay with a display/cursor property.

Thanks.  But such reports should go to address@hidden

> When using `fci-mode' from fill-column-indicator.el,
> `previous-line' sometimes goes past two blank lines instead of
> one.
> 
> Affected Emacs versions:
>  - Good: 24.2
>  - BAD:  24.3
>  - BAD:  24.3.50 as of 2013-04-08
> 
> 1. One can reproduce the problem using fill-column-indicator.el:
> 
>    $ wget 
> https://raw.github.com/alpaker/Fill-Column-Indicator/master/fill-column-indicator.el
>    $ emacs -Q
> 
>    M-x load-file fill-column-indicator.el
>    M-x fci-mode
>    <enter> <enter> <enter> <enter> <enter>
>    <up>
>    <up>  ;; BAD: this moves up 2 lines instead of 1 line
> 
> 
> 2. I minimized the above down to the following self-contained test
>    case:
> 
>    $ cat > aa.el <<EOF
>      (defun fci-redraw-region (start end _ignored)
>        (save-match-data
>          (save-excursion
>            (goto-char start)
>            (while
>                (search-forward "\n" end t)
>              (setq eol-str "X")
>              (setq o (make-overlay (match-beginning 0) (match-beginning 0)))
>              (overlay-put o 'after-string
>                           (propertize eol-str 'display (propertize eol-str 
> 'cursor t)))
>              ))))
>      (add-hook 'after-change-functions 'fci-redraw-region)
>      (insert "\n\n\n\n")
>      (vertical-motion -1)
>      (vertical-motion -1)
>      (kill-emacs (+ 100 (line-number-at-pos)))
>    EOF
> 
>    $ emacs-24.2 -nw -Q --load aa.el ; echo $?
>    103  # good
> 
>    $ emacs-24.3 -nw -Q --load aa.el ; echo $?
>    102  # BAD

Thanks for the test case.  This is fixed in trunk revision 112274.
The fixed code works correctly both with fill-column-indicator and
with the simplified recipe above.

> 3. Using the above I bisected the changes between 24.2 and 24.3
>    down to the following commit:
> 
>    commit b65a46be5055c338a9f8e7640ad97b8e592d5977
>    Refs: HEAD, refs/bisect/bad
>    Author:     Eli Zaretskii <address@hidden>
>    AuthorDate: 2012-11-21 21:28:14 +0200
>    Commit:     Eli Zaretskii <address@hidden>
>    CommitDate: 2012-11-21 21:28:14 +0200

Thanks, but in the future please try to identify the bzr revision
number or revision id of the offending commit.

For the record:

 revision-id: address@hidden
 revision number on the trunk: 110764.1.170

> Thoughts?

It's a bug.  The code I committed back then to fix another bug
considered a display string to be always set on buffer text.  But in
this case, the display string was set on an after-string, a
possibility that the code I wrote back then didn't take into
consideration.