unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Bug in vertical-motion vs overlay with display property
@ 2013-04-12 23:34 Karl Chen
  2013-04-13  9:03 ` Eli Zaretskii
  0 siblings, 1 reply; 4+ messages in thread
From: Karl Chen @ 2013-04-12 23:34 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: alp.tekin.aker, Emacs Developement List

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.

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

   Note that `emacs --batch' doesn't reproduce the problem.


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 <eliz@gnu.org>
   AuthorDate: 2012-11-21 21:28:14 +0200
   Commit:     Eli Zaretskii <eliz@gnu.org>
   CommitDate: 2012-11-21 21:28:14 +0200

       Fix bug #12930 with vertical-motion through a display string.

        src/indent.c (Fvertical_motion): If the starting position is covered
        by a display string, return to one position before that, to avoid
        overshooting it inside move_it_to.
   ---
    src/ChangeLog | 6 ++++++
    src/indent.c  | 8 +++++++-
    2 files changed, 13 insertions(+), 1 deletion(-)

   diff --git a/src/ChangeLog b/src/ChangeLog
   index 020948e..c51f58a 100644
   --- a/src/ChangeLog
   +++ b/src/ChangeLog
   @@ -1,3 +1,9 @@
   +2012-11-21  Eli Zaretskii  <eliz@gnu.org>
   +
   +       * indent.c (Fvertical_motion): If the starting position is covered
   +       by a display string, return to one position before that, to avoid
   +       overshooting it inside move_it_to.  (Bug#12930)
   +
    2012-11-20  Daniel Colascione  <dancol@dancol.org>

           * w32fns.c (Fx_file_dialog):
   diff --git a/src/indent.c b/src/indent.c
   index bbc944d..3332228 100644
   --- a/src/indent.c
   +++ b/src/indent.c
   @@ -2057,7 +2057,13 @@ whether or not it is currently displayed in some window.  */)
              comment said this is "so we don't move too far" (2005-01-19
              checkin by kfs).  But this does nothing useful that I can
              tell, and it causes Bug#2694 .  -- cyd */
   -       move_it_to (&it, PT, -1, -1, -1, MOVE_TO_POS);
   +       /* When the position we started from is covered by a display
   +          string, move_it_to will overshoot it, while vertical-motion
   +          wants to put the cursor _before_ the display string.  So in
   +          that case, we move to buffer position before the display
   +          string, and avoid overshooting.  */
   +       move_it_to (&it, disp_string_at_start_p ? PT - 1 : PT,
   +                   -1, -1, -1, MOVE_TO_POS);

          /* IT may move too far if truncate-lines is on and PT lies
            beyond the right margin.  IT may also move too far if the

Thoughts?

Thanks,
Karl



^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-04-13 21:10 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-04-12 23:34 Bug in vertical-motion vs overlay with display property Karl Chen
2013-04-13  9:03 ` Eli Zaretskii
2013-04-13 15:13   ` Karl Chen
2013-04-13 21:10     ` Eli Zaretskii

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).