Re: An interesting line-motion bug.

[Karl Fogel <kfogel@red-bean.com>]

On 17 Nov 2022, Eli Zaretskii wrote:
>> From: Karl Fogel <kfogel@red-bean.com>
>> Date: Wed, 16 Nov 2022 23:38:32 -0600
>> 
>> ;; When you hit that breakpoint, point will already be on the 
>> newline
>> ;; at the end of the line ";; in a buffer in which" above.  By 
>> then,
>> ;; that newline has a display property that makes it look wide 
>> to
>> ;; Emacs -- specifically, one unit wider than a newline 
>> normally is.
>> ;; But Emacs never asked whether the display property is on a
>> ;; *newline*.  In other words, Emacs identifies this as a wide
>> ;; character and thus mistakenly computes a "width" that 
>> actually
>> ;; extends over a vertical (because newline) span.
>
> Why is it important that the display property "covers" a 
> newline?  A
> display property whose value is a string completely replaces the 
> text
> that it "covers", and that includes the newline in the buffer 
> text.

I don't think I said "cover" anywhere -- maybe you're referring to 
where I said "extends over"?  But I think I basically understand 
your question, so I'll try to answer it:

From a user's perspective, a display property is not present in 
the same way a buffer character is present.  For example, after 
running my reproduction sexp, if you then go to the beginning of 
the buffer and do an isearch for "⏎" [1], the search won't match 
the display properties on the newlines, but it *will* match that 
character in the lisp code of my recipe.  To the user, a display 
property "extends over" some buffer text but does not really 
replace that text.

Now, once you get down in to get_char_property_and_overlay() as 
called from indent.c:check_display_width(), and from that call to 
consequently landing in the "/* Handle 'display' strings.  */" 
case later in check_display_width(), it is reasonable to say that 
the display string functionally replaces the buffer text.  So 
where I wrote "Emacs never asked whether the display property is 
on a newline" above, I could instead have written "Emacs never 
asked whether the display property includes a newline", and 
perhaps that latter phrasing would have seemed more natural to 
you.

> What _is_ important is that the display property itself includes 
> a
> newline, which effectively resets the column to zero.

Yes.  Unfortunately, the fact that the caller might care about 
that column reset is not known to check_display_width(), and the 
latter's current calling discipline offers no way to return that 
information.

> Unfortunately, I don't know how to fix this basic issue.  The 
> code
> which does that is AFAIU unable to grasp the situation where 
> moving
> across text _decreases_ the column.

Well, instead of asking whether moving across text could decrease 
the column, another way to look at it is this:

check_display_width() only has one caller, scan_for_column(), and 
scan_for_column's promise is:

   "Scanning from the beginning of the current line, stop at the 
   buffer
    position ENDPOS or at the column GOALCOL or at the end of 
    line,
    whichever comes first."

The problem we're having is that it's failing to detect that third 
possibility: "or at the end of line".

A solution could be to update check_display_width() to return a 
bit more information than it currently returns.  For example, it 
could return a special flag value to indicate that there is a 
newline *somewhere* in a display property at POS, or it could even 
take a new 'ptrdiff_t *' parameter and set the value of that 
parameter to the actual position of the newline within the display 
string (or else to NULL if no newline).

> If someone has ideas how to support such situations, please 
> speak up.
> And keep in mind that this code is used much more than for C-n 
> and
> C-p.

*nod* Okay, so my worst fear is confirmed -- if you think this is 
hard too, then it's not just my ignorance making it hard :-).

But I have outlined a possible solution above.  If it doesn't seem 
obviously wrong to you, I'd be happy to make (and of course test) 
a patch for review.

Best regards,
-Karl

[1] Just do `(insert 9166)' if an MTA or MUA somewhere along the 
way messed things up such that the desired character is not 
present for you in my post.