Subject: bug#67604: Motion problems with inline images

From: Eli Zaretskii <[email-masked]>
Date: Sat, 31 May 2025 15:18:08 +0300

> From: JD Smith <[email-masked]>
> Date: Fri, 30 May 2025 21:37:11 -0400
> Cc: [email-masked]
> 
> 
> 
> > On May 29, 2025, at 10:18 AM, Eli Zaretskii <[email-masked]> wrote:
> 
> > No, that's against the contract of the move_it_* functions.  They
> > return after they reached (or passed) the specified position and
> > processed the display element at that position, i.e. computed its
> > metrics.  And you cannot do that if you stop before the image.
> 
> Aha, that's the crux of the matter.  Is this contract spelled out somewhere in a comment?  If not, would be very useful to have.

Not really spelled out.  It's my conclusion from what is expected.

> > Sorry, you lost me here.  I asked a question: if the iterator moves to
> > the newline immediately after the image, does it->method become
> > GET_FROM_BUFFER?  
> 
> With my initial patch (`&& (it->line_wrap != WORD_WRAP || may_wrap)', the iterator does not move past the green image to the newline.  The reason is that the "just fits" branch is not taken.  It is during this (skipped) branch that we would have moved the iterator forward onto the newline:
> 
>  prev_method = it->method;
>  if (it->method == GET_FROM_BUFFER)
>    prev_pos = IT_CHARPOS (*it);
>  set_iterator_to_next (it, true);
> 
> Instead, we return with MOVE_LINE_CONTINUED, from the location mentioned above, after the move_trace call (but note: point still does the right thing and the cursor overflows into the fringe).
> 
> It is this combination of moving forward past the image, AND not recognizing that the desired position has been reached (so that it cannot be restored via atpos_it) that leads to the line skip bug.
> 
> > If yes, why don't we return MOVE_POS_MATCH_OR_ZV?
> > If it->method is not GET_FROM_BUFFER, why not?
> 
> Because we do not in fact move forward past the GET_FROM_IMAGE position.  Which sounds like a problem v.v. the "contract" you mention.  

So you are saying that the patch you suggested is incorrect, and we
should do something else, and in particular make sure the call to
set_iterator_to_next _is_ made?

> > As for the return value of move_it_in_display_line_to, it _is_
> > important to some of its callers.  So it's important to get it right.
> 
> I figured.  The conundrum seems to be:
> 
> 1. move_it_in_display_line_to promises to move _past_ an image/stretch if the requested TO_CHARPOS falls on one.
> 2. Moving past a just-fitting image (which in actuality has always wrapped to the next display line with its trailing space, via WORD_WRAP) leaves us with the wrong idea about the starting x position, leading to vertical motion bugs.
> 
> I may need to set this aside for a bit and come back later.  In the meantime, I'd appreciate your thoughts on the following:
> 
> If the move_it_in_display_line_to is obligated to iterate past an image (even one at the end of the line that will be wrapped) so as to properly compute its metrics, must IT really be past the image when the function returns, or can the function restore a wrap position left of the image before returning?  Does that count as having satisfied the contract (i.e. do the "metrics" survive wrap_it or atpos_it restoration)?

This is a notoriously dark corner of move_it_* functions under
word-wrap.  I think you will find traces of it in several places if
you search for WORD_WRAP, for example at the beginning of
move_it_in_display_line.  So I think this tricky aspect is expected by
the rest of the code, should not get in the way in this case.