Re: vertical-motion bug

[Kim F. Storm]

Chong Yidong <address@hidden> writes:

> For what it's worth, reverting the following line in the 2006-06-14
> change makes things work again.  I don't know if this is the correct
> fix, since I don't know why that line was introduced in the first
> place.

I don't remember the exact reason, but it was related to one of the
(buggy) uses of propertized before or after strings for an overlay with
something non-trivial in those strings (a composition, IIRC).

So I fear that reverting that change will re-introduce the other bug.


FWIW, here are the examples that I have on file -- there may be more
in the mail archive...

; emacs -Q -D
(setq overlay (make-overlay 1 3))
(overlay-put overlay 'before-string (compose-string "BE"))
-> before-string is doubled.

(overlay-put overlay 'after-string (compose-string "AF"))
-> after-string is not displayed.

(overlay-put overlay 'before-string nil)
-> after-string is displayed but doubled.


 * Case 1: The before-string is shown twice. ssssssssssss
; emacs -Q -D
(progn
(setq overlay (make-overlay 1 3))
(overlay-put overlay 'before-string (propertize "BE" 'face 'bold))
(overlay-put overlay 'after-string (propertize "AF" 'display
                         (propertize "XY" 'face 'underline)))
(put-text-property 1 3 'display (compose-string "DISP"))
)

* Case 2: Cursor at a wrong position.
; emacs -q -D  ;; not `-Q' in order to show a message in *scratch*
(progn
(setq overlay (make-overlay 1 3))
(overlay-put overlay 'before-string (propertize "BEFORE_STRING" 'face 'bold))
(overlay-put overlay 'after-string (propertize "AF" 'display
                         (propertize "XY" 'face 'underline)))
(put-text-property 1 3 'invisible t)
)
;; make sure that the first line is wrapped
M-<


; xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
; emacs -q -D  ;; not `-Q' in order to show a message in *scratch*
(progn
(setq overlay (make-overlay 1 3))
(overlay-put overlay 'before-string (propertize "BE" 'face 'bold))
(overlay-put overlay 'after-string (propertize "AF" 'display
                         (propertize "XY" 'face 'underline)))
(put-text-property 1 3 'display "DISP")
)
;; make sure that the first line is wrapped
M-<

Then the cursor is displayed at a wrong position.

Another assertion violation can be observed by replacing the last
expression with (put-text-property 1 3 'display '(space :width 2))

; emacs -Q -D
(progn
(setq overlay (make-overlay 1 3))
(overlay-put overlay 'before-string (propertize "BE" 'face 'bold))
(overlay-put overlay 'after-string (propertize "AF" 'display
                         (propertize "XY" 'face 'underline)))
(compose-region 1 3)
)

; emacs -Q -D
(progn
(defconst breakpoint-xpm-data
  "/* XPM */
static char *magick[] = {
/* columns rows colors chars-per-pixel */
\"10 10 2 1\",
\"  c red\",
\"+ c None\",
/* pixels */
\"+++    +++\",
\"++      ++\",
\"+        +\",
\"          \",
\"          \",
\"          \",
\"          \",
\"+        +\",
\"++      ++\",
\"+++    +++\",
};"
  "XPM data used for breakpoint icon.")

(setq im (find-image `((:type xpm :data ,breakpoint-xpm-data))))
)

(progn ;; IMAGE
(setq overlay (make-overlay 1 3))
(overlay-put overlay 'before-string (propertize "BE" 'display im))
(overlay-put overlay 'after-string
             (propertize "AF" 'display
                         (propertize "XY" 'face 'underline)))
)

(progn ;; STRETCH
(setq overlay (make-overlay 1 3))
(overlay-put overlay 'before-string (propertize "BE" 'display '(space . (:width 
5))))
(overlay-put overlay 'after-string
             (propertize "AF" 'display
                         (propertize "XY" 'face 'underline)))
(overlay-put overlay 'priority 100)
)

(progn
(put-text-property 1 3 'display "DISP")
(put-text-property 2 3 'display "disp")
(put-text-property 3 4 'display "123")
)

(progn
 (goto-char (point-min))
 (insert-image im)
)

>
> *** emacs/src/xdisp.c.~1.1109.~       2006-07-15 11:38:46.000000000 -0400
> --- emacs/src/xdisp.c 2006-07-15 11:46:48.000000000 -0400
> ***************
> *** 5315,5321 ****
>     it->area = TEXT_AREA;
>     it->multibyte_p = !NILP (current_buffer->enable_multibyte_characters);
>     it->sp = 0;
> -   it->string_from_display_prop_p = 0;
>     it->face_before_selective_p = 0;
>   
>     if (set_stop_p)
> --- 5315,5320 ----
>
>>   From: Chong Yidong <address@hidden>
>>
>>   In a scratch buffer:
>>
>>     M-<
>>     M-: (insert (propertize "a" 'display "a\nb\nc\n")) RET
>>     M-<
>>     C-n (or M-: (vertical-motion 1))
>>
>>   Result: point moves down three lines.  The expected behavior, based on
>>   the `vertical-motion' docstring, is to move just one line.  This
>>   behavior was broken by something in the following change:
>>
>>   2006-06-14  Kim F. Storm  <address@hidden>
>>
>>   * dispextern.h (IT_STACK_SIZE): New macro specifying size of
>>   iterator stack (instead of hardcoded number).  Increase from 2 to
>>   4 to make room for propertized overlay strings before and after a
>>   display string, image or composition.
>>   (struct it): Add image_id and method members to iterator stack.
>>
>>   * xdisp.c (init_from_display_pos): Don't set it->method and
>>   overlay_string_index after pop_it.  Add asserts.
>>   (handle_stop): Look for overlay strings around a display string,
>>   image, or composition.  Handle properties on those strings.
>>   (next_overlay_string): Don't set string, pos or method after pop_it.
>>   (get_overlay_strings_1): Split from get_overlay_strings; don't
>>   modify it if no overlay strings are found.
>>   (get_overlay_strings): Use get_overlay_strings_1.  Always set
>>   it->string and it->method.
>>   (push_it): Push it->image_id and it->method.  Push it->object
>>   instead of it->string if method is GET_FROM_IMAGE.
>>   (pop_it): Pop it->image_id and it->method.  Ppo it->object
>>   instead of it->string if method is GET_FROM_IMAGE.
>>   Reset it->current.string_pos if popped it->string is nil.
>>   (reseat_1): Remove comment dated 19 May 2003.  It expressed doubt
>>   whether a given change was correct; but the change is correct.
>>   Clear it->string_from_display_prop_p.
>>   (set_iterator_to_next): Rely on it->method and it->image_id from
>>   iterator stack, instead of setting them explicitly after pop_it.

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