An interesting line-motion bug.

An interesting line-motion bug.

[Karl Fogel]

[-- Attachment #1: Type: text/plain, Size: 574 bytes --]

The attached file shows a reproduction recipe for a line-motion 
bug that had been nagging at me for some time.  The file also 
includes some debugging information and a preliminary diagnosis.

I don't know this area of the code very well.  While I can think 
of possible ways to fix this, each potential fix I've thought of 
so far raises questions that I don't feel secure about answering. 
If someone who knows this area well can make The Right Fix 
quickly, then great.  If not, then I would be happy to keep 
studying and make a patch for review.

Best regards,
-Karl


[-- Attachment #2: line-motion-bug-reproduction.el --]
[-- Type: application/emacs-lisp, Size: 4564 bytes --]

Re: An interesting line-motion bug.

[Karl Fogel]

In the attachment I just posted, the GDB line includes a filename 
"README.branch".  That's because I've been saving my debugging 
work on a personal branch.  Just replace "README.branch" with 
"line-motion-bug-reproduction.el" (which is the filename that I 
attached to my post).

Best regards,
-Karl

Re: An interesting line-motion bug.

[Eli Zaretskii]

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

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

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

Re: An interesting line-motion bug.

[Karl Fogel]

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.