Package: emacs;
Reported by: Thuna <thuna.cing <at> gmail.com>
Date: Thu, 6 Feb 2025 23:30:02 UTC
Severity: wishlist
Tags: patch
Done: Eli Zaretskii <eliz <at> gnu.org>
Bug is archived. No further changes may be made.
Message #69 received at 76107 <at> debbugs.gnu.org (full text, mbox):
From: Eli Zaretskii <eliz <at> gnu.org> To: Thuna <thuna.cing <at> gmail.com> Cc: 76107 <at> debbugs.gnu.org Subject: Re: bug#76107: Consider image specs when scanning for a column Date: Thu, 20 Mar 2025 13:17:03 +0200
> From: Thuna <thuna.cing <at> gmail.com>
> Cc: 76107 <at> debbugs.gnu.org
> Date: Tue, 18 Mar 2025 02:59:37 +0100
>
> I have been using the patch for a while now. I have just been using it
> for regular tasks, so I have not been explicitly testing any edge cases.
>
> I have not yet had any crashes, and as far as I can see, image sizes are
> being calculated correctly.
Thanks, that's good news.
> So far, I have only had two related issues: The width calculation is
> incorrect when dealing with characters which are not full-sized (they
> are being calculated as though they were full-sized)
Sorry, I don't understand: is this due to my patch? My patch isn't
supposed to touch the case of characters, only that of inline images.
Was this problem present before my patch? If not, can you show an
example of it?
> and left margin[1] is being included in the column number, but only
> when there is an image from line start to point.
>
> For the second situation, here's the MRE:
> 1. emacs -q
> 2. Switch to a new buffer, turn on org-mode, insert the text
> > foo \( \mathbb{R}^{*} \)
> 3. Call org-latex-preview on the latex fragment.
> 4. Eval (current-column) at the start of line, end of "foo", and the end
> of line, you should see 0, 3, and 6.
> 5. Turn on display-line-numbers-mode, and repeat above, you should now
> see 0, 3, and 10.
>
> Alternatively, you could have the buffer contents be
> > --------
> > * Foo
> > foo \( \mathbb{R}^{*} \)
> and turn on org-indent-mode (but not display-line-numbers-mode). You'll
> then see that the column at the end of the latex preview is the same as
> if it were calculated from the left edge of the buffer (using the dashes
> as reference).
Hmm... you say "there is an image from line start to point", but in
the example you show the image is not at the beginning of a line. Did
I misunderstand what you are describing?
Anyway, I think I fixed this subtle issue in the patch below, which
should _replace_ the previous one. If you want to apply the new
change incrementally, then just add this one line:
it.hpos = 1;
after this line in the patched code you were using until now:
it.last_visible_x = 1000000; /* prevent image clipping */
> [1] That's probably what it is? The specific thing I am
> referring to is org-indent-mode and display-line-numbers-mode, which I
> assume works using left margin.
No, that's not the margin. In org-indent-mode, the whitespace is the
line-prefix, and in display-line-numbers-mode it's the space reserved
for the line numbers.
Thanks again for testing this improvement. Here's the full patch
relative to the current master:
diff --git a/src/indent.c b/src/indent.c
index 01cea22..cb09f3e 100644
--- a/src/indent.c
+++ b/src/indent.c
@@ -466,13 +466,21 @@ current_column (void)
}
+static void restore_window_buffer (Lisp_Object);
+
/* Check the presence of a display property and compute its width.
+ POS and POS_BYTE are the character and byte positions of the
+ display property and COL is the column number at POS.
If a property was found and its width was found as well, return
its width (>= 0) and set the position of the end of the property
in ENDPOS.
- Otherwise just return -1. */
+ Otherwise just return -1.
+ WINDOW is the window to use when it is important; nil stands for
+ the selected window. */
static int
-check_display_width (ptrdiff_t pos, ptrdiff_t col, ptrdiff_t *endpos)
+check_display_width (Lisp_Object window,
+ ptrdiff_t pos, ptrdiff_t pos_byte, ptrdiff_t col,
+ ptrdiff_t *endpos)
{
Lisp_Object val, overlay;
@@ -482,30 +490,80 @@ check_display_width (ptrdiff_t pos, ptrdiff_t col, ptrdiff_t *endpos)
int width = -1;
Lisp_Object plist = Qnil;
- /* Handle '(space ...)' display specs. */
- if (CONSP (val) && EQ (Qspace, XCAR (val)))
- { /* FIXME: Use calc_pixel_width_or_height. */
- Lisp_Object prop;
- EMACS_INT align_to_max =
- (col < MOST_POSITIVE_FIXNUM - INT_MAX
- ? (EMACS_INT) INT_MAX + col
- : MOST_POSITIVE_FIXNUM);
-
- plist = XCDR (val);
- if ((prop = plist_get (plist, QCwidth),
- RANGED_FIXNUMP (0, prop, INT_MAX))
- || (prop = plist_get (plist, QCrelative_width),
- RANGED_FIXNUMP (0, prop, INT_MAX)))
- width = XFIXNUM (prop);
- else if (FLOATP (prop) && 0 <= XFLOAT_DATA (prop)
- && XFLOAT_DATA (prop) <= INT_MAX)
- width = (int)(XFLOAT_DATA (prop) + 0.5);
- else if ((prop = plist_get (plist, QCalign_to),
- RANGED_FIXNUMP (col, prop, align_to_max)))
- width = XFIXNUM (prop) - col;
- else if (FLOATP (prop) && col <= XFLOAT_DATA (prop)
- && (XFLOAT_DATA (prop) <= align_to_max))
- width = (int)(XFLOAT_DATA (prop) + 0.5) - col;
+ if (CONSP (val))
+ {
+ Lisp_Object xcar = XCAR (val);
+
+ /* Handle '(space ...)' display specs. */
+ if (EQ (Qspace, xcar))
+ { /* FIXME: Use calc_pixel_width_or_height. */
+ Lisp_Object prop;
+ EMACS_INT align_to_max =
+ (col < MOST_POSITIVE_FIXNUM - INT_MAX
+ ? (EMACS_INT) INT_MAX + col
+ : MOST_POSITIVE_FIXNUM);
+
+ plist = XCDR (val);
+ if ((prop = plist_get (plist, QCwidth),
+ RANGED_FIXNUMP (0, prop, INT_MAX))
+ || (prop = plist_get (plist, QCrelative_width),
+ RANGED_FIXNUMP (0, prop, INT_MAX)))
+ width = XFIXNUM (prop);
+ else if (FLOATP (prop) && 0 <= XFLOAT_DATA (prop)
+ && XFLOAT_DATA (prop) <= INT_MAX)
+ width = (int)(XFLOAT_DATA (prop) + 0.5);
+ else if ((prop = plist_get (plist, QCalign_to),
+ RANGED_FIXNUMP (col, prop, align_to_max)))
+ width = XFIXNUM (prop) - col;
+ else if (FLOATP (prop) && col <= XFLOAT_DATA (prop)
+ && (XFLOAT_DATA (prop) <= align_to_max))
+ width = (int)(XFLOAT_DATA (prop) + 0.5) - col;
+ }
+ /* Handle images. */
+ else if (EQ (Qimage, xcar)
+ || EQ (Qslice, xcar)
+ /* 'insert-sliced-image' creates property of the form
+ ((slice ...) image ...) */
+ || (CONSP (xcar) && EQ (Qslice, XCAR (xcar))))
+ {
+ specpdl_ref count = SPECPDL_INDEX ();
+ struct window *w = decode_live_window (window);
+
+ /* If needed, set the window's buffer temporarily to the
+ current buffer and its window-point to POS. */
+ if (XBUFFER (w->contents) != current_buffer)
+ {
+ Lisp_Object oldbuf
+ = list4 (window, w->contents,
+ make_fixnum (marker_position (w->pointm)),
+ make_fixnum (marker_byte_position (w->pointm)));
+ record_unwind_protect (restore_window_buffer, oldbuf);
+ wset_buffer (w, Fcurrent_buffer ());
+ set_marker_both (w->pointm, w->contents, pos, pos_byte);
+ }
+
+ struct text_pos startpos;
+ struct it it;
+ SET_TEXT_POS (startpos, pos, pos_byte);
+ void *itdata = bidi_shelve_cache ();
+ record_unwind_protect_void (unwind_display_working_on_window);
+ display_working_on_window_p = true;
+ start_display (&it, w, startpos);
+ it.last_visible_x = 1000000; /* prevent image clipping */
+ /* Forcing HPOS non-zero prevents the display code from
+ generating line/wrap-prefix and line numbers, which
+ will skew the X coordinate we use below. */
+ it.hpos = 1;
+ /* The POS+1 value is a trick: move_it_in_display_line
+ will not return until it finished processing the entire
+ image, even if it covers more than one buffer position. */
+ move_it_in_display_line (&it, pos + 1, -1, MOVE_TO_POS);
+ /* The caller wants the width in units of the frame's
+ canonical character width. */
+ width = ((double)it.current_x / FRAME_COLUMN_WIDTH (it.f)) + 0.5;
+ bidi_unshelve_cache (itdata, 0);
+ unbind_to (count, Qnil);
+ }
}
/* Handle 'display' strings. */
else if (STRINGP (val))
@@ -652,7 +710,7 @@ scan_for_column (ptrdiff_t *endpos, EMACS_INT *goalcol,
{ /* Check display property. */
ptrdiff_t endp;
- int width = check_display_width (scan, col, &endp);
+ int width = check_display_width (window, scan, scan_byte, col, &endp);
if (width >= 0)
{
col += width;
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.