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.
View this message in rfc822 format
From: help-debbugs <at> gnu.org (GNU bug Tracking System) To: Eli Zaretskii <eliz <at> gnu.org> Cc: tracker <at> debbugs.gnu.org Subject: bug#77336: closed ([PATCH] Fix mouse highlighting for compact mode lines) Date: Sat, 05 Apr 2025 09:37:02 +0000
[Message part 1 (text/plain, inline)]
Your message dated Sat, 05 Apr 2025 12:36:46 +0300 with message-id <86tt73vtjl.fsf <at> gnu.org> and subject line Re: bug#77336: [PATCH] Fix mouse highlighting for compact mode lines has caused the debbugs.gnu.org bug report #77336, regarding [PATCH] Fix mouse highlighting for compact mode lines to be marked as done. (If you believe you have received this mail in error, please contact help-debbugs <at> gnu.org.) -- 77336: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=77336 GNU Bug Tracking System Contact help-debbugs <at> gnu.org with problems
[Message part 2 (message/rfc822, inline)]
From: Pengji Zhang <me <at> pengjiz.com> To: bug-gnu-emacs <at> gnu.org Subject: [PATCH] Fix mouse highlighting for compact mode lines Date: Fri, 28 Mar 2025 18:55:30 +0800[Message part 3 (text/plain, inline)]Hello, I found that with 'mode-line-compact' being non-nil, mouse highlighting would not work properly. To reproduce: - emacs -Q - (setq mode-line-compact t) or (setq mode-line-compact 'long) - Hover mouse cursor over the major mode part on the mode line Now the minor mode lighters are also highlighted. That is presumably because when 'mode-line-compact' is non-nil, the whole mode line string is displayed at once. So the glyphs produced all have the same 'object' field. That confuses the computation of boundaries for mouse highlighting. In this patch we instead split the mode line string into elements and display them one by one. Thanks![0001-Fix-mouse-highlighting-for-compact-mode-lines.patch (text/x-patch, inline)]From 5c7538aceb7e80ea44be7d74890d36e88d36169c Mon Sep 17 00:00:00 2001 From: Pengji Zhang <me <at> pengjiz.com> Date: Sun, 23 Mar 2025 20:19:09 +0800 Subject: [PATCH] Fix mouse highlighting for compact mode lines 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 f2b158f00e3..193f9767c6f 100644 --- a/src/xdisp.c +++ b/src/xdisp.c @@ -27745,57 +27745,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 (); @@ -28014,7 +28059,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, 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, @@ -28067,8 +28115,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, elt); n += store_mode_line_string (NULL, mode_string, false, - 0, 0, Qnil); + 0, 0, src); } break; case MODE_LINE_DISPLAY: @@ -28148,6 +28197,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, elt); /* Should only keep face property in props */ n += store_mode_line_string (NULL, tem, false, field, prec, props); @@ -28363,6 +28413,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; @@ -37657,6 +37710,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
[Message part 5 (message/rfc822, inline)]
From: Eli Zaretskii <eliz <at> gnu.org> To: Pengji Zhang <me <at> pengjiz.com> Cc: 77336-done <at> debbugs.gnu.org Subject: Re: bug#77336: [PATCH] Fix mouse highlighting for compact mode lines Date: Sat, 05 Apr 2025 12:36:46 +0300> From: Pengji Zhang <me <at> pengjiz.com> > Cc: 77336 <at> debbugs.gnu.org > Date: Wed, 02 Apr 2025 21:02:19 +0800 > > Eli Zaretskii <eliz <at> gnu.org> writes: > > >> From: Pengji Zhang <me <at> pengjiz.com> > >> Cc: 77336 <at> debbugs.gnu.org > >> Date: Mon, 31 Mar 2025 11:31:36 +0800 > >> > >> Eli Zaretskii <eliz <at> gnu.org> writes: > >> > >> >> Is casting the 'Lisp_Object' to a number acceptable here? > >> > > >> > No, because Lisp_Object could be a struct (if Emacs was configured with > >> > "--enable-check-lisp-object-type"). > >> > >> Sorry I did not make it clear. I meant 'make_fixnum(XLI(elt))'. I > >> suppose 'XLI' will handle that case correctly? > > > > I'd prefer not to do that unless there's no other reasonable solution. > > > >> I am actually concerned that there could be a collision between two > >> different 'elt's, e.g., a symbol and a string, because 'make_fixnum' > >> alters the tag of 'elt'. I think it might be fine because 'elt' should > >> always be a 'Lisp_String' when this is called. > > > > Sorry, I don't understand: what do you mean by "alters the tag of > > 'elt'", and how could make_fixnum affect 'elt'? > > It does not affect 'elt'. I meant that 'make_fixnum' may change the > value of the number from the tagged pointer: > > INLINE Lisp_Object > make_fixnum (EMACS_INT n) > { > ... > { > n &= INTMASK; > n += (int0 << VALBITS); > } > return XIL (n); > } > > So if I understand it correctly, that means 'make_fixnum(XLI(x))' could > give the same result for, e.g., a fixnum and a string. > > >> > A global variable is what I had in mind. I don't see why it would be > >> > less clean. > >> > >> It is just that maintaining a global state is not as tasteful to me. If > >> 'make_fixnum(XLI(elt))' is not good here, I am happy to go this way. > > > > Yes, a global variable is cleaner than using XLI for this purpose. > > Thanks. Please see the attached patch. Thanks, I've now installed this on the master branch, and I'm therefore closing this bug.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.