* Re: An interesting line-motion bug.
2022-11-17 12:24 ` Eli Zaretskii
@ 2022-11-17 20:58 ` Karl Fogel
0 siblings, 0 replies; 4+ messages in thread
From: Karl Fogel @ 2022-11-17 20:58 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
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.
^ permalink raw reply [flat|nested] 4+ messages in thread