Package: emacs;
Reported by: Pengji Zhang <me <at> pengjiz.com>
Date: Fri, 28 Mar 2025 10:56:01 UTC
Severity: normal
Tags: patch
Done: Eli Zaretskii <eliz <at> gnu.org>
Bug is archived. No further changes may be made.
Message #11 received at 77336 <at> debbugs.gnu.org (full text, mbox):
From: Pengji Zhang <me <at> pengjiz.com> To: Eli Zaretskii <eliz <at> gnu.org> Cc: 77336 <at> debbugs.gnu.org Subject: Re: bug#77336: [PATCH] Fix mouse highlighting for compact mode lines Date: Sun, 30 Mar 2025 19:44:59 +0800
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes: > Thanks, but why do we need to make the value of this mode-line-element > property be the element itself? Why not give it some simple value, > like the ordinal number of the element (which should be incremented > whenever we process another element)? So we could call this property > mode-line-elt-number and assign the elements values 1, 2, 3, etc. Is casting the 'Lisp_Object' to a number acceptable here? Please have a look at the updated patch. I tried an incremental counter but gave up. Because 'display_mode_element' could be called recursively, we may have to change its signature or introduce a new global variable. That feels a bit less clean to me. > I'm concerned that having the elements themselves be values will > prevent them from being GC'd, which might increase the memory > pressure. By contrast, fixnums are immediate values that don't need > to be GC'ed, and the element itself will not have any references after > the mode line is processed. I thought all objects would be GC'd. Since fixnums are not, I guess now we do not need to remove the text property before displaying the string? Please let me know what you think and I will update the patch if needed.
[0001-Fix-mouse-highlighting-for-compact-mode-lines.patch (text/x-patch, inline)]
From 73bc97111ace409a5de27930d0dadcd18490fa5e Mon Sep 17 00:00:00 2001
From: Pengji Zhang <me <at> pengjiz.com>
Date: Sun, 30 Mar 2025 19:24:10 +0800
Subject: [PATCH] Fix mouse highlighting for compact mode lines (bug#77336)
When 'mode-line-compact' is non-nil, the mode line string is
displayed as a whole. That confuses the computation of ranges
of mouse highlighting on the mode line because all the glyphs
have the same Lisp object source. As such, in this commit we
instead split the mode line string by sources, and display those
elements one by one, so the boundaries of each element could be
correctly detected for the purpose of mouse highlighting.
* src/xdisp.c (display_mode_line): Display mode line elements
one by one when 'mode-line-compact' is non-nil.
(display_mode_element): Record source of the stored string via a
text property.
(syms_of_xdisp): New symbol for the text property.
---
src/xdisp.c | 132 ++++++++++++++++++++++++++++++++++++----------------
1 file changed, 93 insertions(+), 39 deletions(-)
diff --git a/src/xdisp.c b/src/xdisp.c
index 2c676c09827..d712e3d02ae 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -27754,57 +27754,102 @@ display_mode_line (struct window *w, enum face_id face_id, Lisp_Object format)
{
/* The window is wide enough; just display the mode line we
just computed. */
- display_string (NULL, mode_string, Qnil,
- 0, 0, &it, 0, 0, 0,
- STRING_MULTIBYTE (mode_string));
+ Lisp_Object start = make_fixnum (0), end;
+ Lisp_Object elt;
+ AUTO_LIST2 (props, Qmode_line_element, Qnil);
+
+ /* Display the mode line string one element by one element.
+ This is to make the ranges of mouse highlighting
+ correct. */
+ do
+ {
+ end = Fnext_single_property_change (start, Qmode_line_element,
+ mode_string, Qnil);
+ elt = Fsubstring (mode_string, start, end);
+ Fremove_text_properties (make_fixnum (0),
+ make_fixnum (SCHARS (elt)),
+ props, elt);
+ display_string (NULL, elt, Qnil, 0, 0, &it, 0, 0, 0,
+ STRING_MULTIBYTE (elt));
+ start = end;
+ }
+ while (!NILP (end));
}
else
{
/* Compress the mode line. */
- ptrdiff_t i = 0, i_byte = 0, start = 0;
+ ptrdiff_t i = 0, i_byte = 0;
int prev = 0;
+ Lisp_Object start = make_fixnum (0), end;
+ Lisp_Object elt = empty_unibyte_string;
+ AUTO_LIST2 (props, Qmode_line_element, Qnil);
- while (i < SCHARS (mode_string))
+ /* Display the mode line string one element by one element.
+ This is to make the ranges of mouse highlighting
+ correct. */
+ do
{
- int c = fetch_string_char_advance (mode_string, &i, &i_byte);
- if (c == ' ' && prev == ' ')
+ end = Fnext_single_property_change (start,
+ Qmode_line_element,
+ mode_string,
+ make_fixnum (SCHARS (mode_string)));
+ while (i < XFIXNUM (end))
{
- Lisp_Object prev_pos = make_fixnum (i - 1);
-
- /* SPC characters with 'display' properties are not
- really "empty", since they have non-trivial visual
- effects on the mode line. */
- if (NILP (Fget_text_property (prev_pos, Qdisplay,
- mode_string)))
+ int c = fetch_string_char_advance (mode_string, &i, &i_byte);
+ if (c == ' ' && prev == ' ')
{
- display_string (NULL,
- Fsubstring (mode_string,
- make_fixnum (start),
- prev_pos),
- Qnil, 0, 0, &it, 0, 0, 0,
- STRING_MULTIBYTE (mode_string));
- /* Skip past the rest of the space characters. */
- while (c == ' ' && i < SCHARS (mode_string)
- && NILP (Fget_text_property (make_fixnum (i),
- Qdisplay,
- mode_string)))
+ Lisp_Object prev_pos = make_fixnum (i - 1);
+ Lisp_Object display = Fget_text_property (prev_pos,
+ Qdisplay,
+ mode_string);
+
+ /* SPC characters with 'display' properties are not
+ really "empty", since they have non-trivial visual
+ effects on the mode line. */
+ if (NILP (display))
{
- c = fetch_string_char_advance (mode_string,
- &i, &i_byte);
+ elt = concat2 (elt, Fsubstring (mode_string,
+ start,
+ prev_pos));
+
+ /* Skip past the rest of the space characters. */
+ Lisp_Object display = Fget_text_property (make_fixnum (i),
+ Qdisplay,
+ mode_string);
+ while (c == ' ' && i < XFIXNUM (end)
+ && NILP (display))
+ {
+ c = fetch_string_char_advance (mode_string,
+ &i, &i_byte);
+ display = Fget_text_property (make_fixnum (i),
+ Qdisplay,
+ mode_string);
+ }
+
+ /* Skip the final space no matter how the loop
+ above ends. */
+ if (c == ' ' && NILP (display))
+ start = end;
+ else
+ start = make_fixnum (i - 1);
}
- start = i - 1;
}
+ prev = c;
}
- prev = c;
- }
- /* Display the final bit. */
- if (start < i)
- display_string (NULL,
- Fsubstring (mode_string, make_fixnum (start),
- make_fixnum (i)),
- Qnil, 0, 0, &it, 0, 0, 0,
- STRING_MULTIBYTE (mode_string));
+ /* Append the final bit. */
+ if (XFIXNUM (start) < XFIXNUM (end))
+ elt = concat2 (elt, Fsubstring (mode_string, start, end));
+
+ Fremove_text_properties (make_fixnum (0),
+ make_fixnum (SCHARS (elt)),
+ props, elt);
+ display_string (NULL, elt, Qnil, 0, 0, &it, 0, 0, 0,
+ STRING_MULTIBYTE (elt));
+ elt = empty_unibyte_string;
+ start = end;
+ }
+ while (XFIXNUM (end) < SCHARS (mode_string));
}
}
pop_kboard ();
@@ -28023,7 +28068,10 @@ display_mode_element (struct it *it, int depth, int field_width, int precision,
n += store_mode_line_noprop (SSDATA (elt), -1, prec);
break;
case MODE_LINE_STRING:
- n += store_mode_line_string (NULL, elt, true, 0, prec, Qnil);
+ {
+ AUTO_LIST2 (src, Qmode_line_element, make_fixnum (XLI (elt)));
+ n += store_mode_line_string (NULL, elt, true, 0, prec, src);
+ }
break;
case MODE_LINE_DISPLAY:
n += display_string (NULL, elt, Qnil, 0, 0, it,
@@ -28076,8 +28124,9 @@ display_mode_element (struct it *it, int depth, int field_width, int precision,
Lisp_Object mode_string
= Fsubstring (elt, make_fixnum (charpos),
make_fixnum (endpos));
+ AUTO_LIST2 (src, Qmode_line_element, make_fixnum (XLI (elt)));
n += store_mode_line_string (NULL, mode_string, false,
- 0, 0, Qnil);
+ 0, 0, src);
}
break;
case MODE_LINE_DISPLAY:
@@ -28157,6 +28206,7 @@ display_mode_element (struct it *it, int depth, int field_width, int precision,
{
Lisp_Object tem = build_string (spec);
props = Ftext_properties_at (make_fixnum (charpos), elt);
+ props = plist_put (props, Qmode_line_element, make_fixnum (XLI (elt)));
/* Should only keep face property in props */
n += store_mode_line_string (NULL, tem, false,
field, prec, props);
@@ -28372,6 +28422,9 @@ display_mode_element (struct it *it, int depth, int field_width, int precision,
n += store_mode_line_noprop ("", field_width - n, 0);
break;
case MODE_LINE_STRING:
+ /* NOTE: Padding implicitly has 'mode-line-element' property
+ to be nil. Normally padding spaces are between two real
+ elements, so this should be good. */
n += store_mode_line_string ("", Qnil, false, field_width - n, 0,
Qnil);
break;
@@ -37666,6 +37719,7 @@ syms_of_xdisp (void)
DEFSYM (Qfontification_functions, "fontification-functions");
DEFSYM (Qlong_line_optimizations_in_fontification_functions,
"long-line-optimizations-in-fontification-functions");
+ DEFSYM (Qmode_line_element, "mode-line-element");
/* Name of the symbol which disables Lisp evaluation in 'display'
properties. This is used by enriched.el. */
--
2.49.0
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.