Re: [PATCH] User-defined fringe tooltips (a request for review)

[Vladimir Kazanov <vekazanov@gmail.com>]

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

After figuring out a couple of nasty crashes I came up with a patch
which works for all display specs coming from buffer text itself.

But I've just realized that some fringe sources are still unhandled
here. For example, when the fringe indicator is defined in non-buffer
strings, like overlays with 'before-string/'after-string properties.
Just iterating through text positions doesn't scan any of that. This
means that we'd have to imitate more of the iterator logic when
looking for display specs containing help-echo.

Do we want to manage fringe help-echo text coming from various strings
as well? Certainly doable. I wonder if this asks for a separate patch.

Thanks


On Sat, 23 Dec 2023 at 13:40, Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Vladimir Kazanov <vekazanov@gmail.com>
> > Date: Sat, 23 Dec 2023 13:28:48 +0000
> > Cc: emacs-devel@gnu.org
> >
> > The way I see things now: the code has to iterate all charpos in the
> > right order, including invisible ones, and also check for display
> > properties in overlays. A glyph_row has information on where the row
> > starts and ends (row->start and row->end).
> >
> > I want to do the following in note_mouse_highlight:
> >
> > /* Get to the current glyph row */
> > struct glyph_row *row = MATRIX_ROW (w->current_matrix, vpos);
> > Lisp_Object left_caption = Qnil, right_caption = Qnil;
> >
> > ptrdiff_t charpos;
> > ptrdiff_t charpos_start = row->start.pos.charpos;
> > ptrdiff_t charpos_end = row->end.pos.charpos;
> > for (charpos = charpos_start; charpos <= charpos_end; charpos++)
> >   {
> >     /* This can be either in text props or overlays, so check both */
> >     Lisp_Object spec = get_char_property_and_overlay (make_fixnum (charpos),
> > Qdisplay, Qnil, NULL);
> >     /* ... parse the spec... */
> >    }
>
> Yes, I think you are right.  But I think it is better to use
> row->minpos and row->maxpos; see the comments in dispextern.h for the
> reasons why.
>
> > > If Fget_text_property returns non-nil for the 'display' property, you
> > > will then need to look at the value and see whether it specifies a
> > > help-echo for the fringe; if it does, extract the string and assign it
> > > to 'help_echo_string'.
> >
> > I've just realized that what you have in mind is reusing the standard
> > 'help-echo property? The one used for tooltips on the text itself?
>
> No, that's not what I meant.  I only meant to assign the tip string to
> help_echo_string like we do in other places in that function.
>
> > Either way, I'd need to test this thoroughly, I suspect there might be
> > interesting corner cases.
>
> There always are, IME.
>
> Thanks.



--
Regards,

Vladimir Kazanov

[-- Attachment #2: 0001-Tooltips-for-user-defined-fringe-indicators.patch --]
[-- Type: text/x-patch, Size: 6258 bytes --]

From e1716ba45ab0e13588258178634c2bcd68319d5c Mon Sep 17 00:00:00 2001
From: Vladimir Kazanov <vekazanov@gmail.com>
Date: Sun, 24 Dec 2023 11:13:10 +0000
Subject: [PATCH] Tooltips for user-defined fringe indicators

Tooltips for user-defined fringe indicators
* doc/lispref/display.texi: expand the manual
* etc/NEWS: mention tooltips
* etc/TODO: drop a todo
* src/xdisp.c (note_fringe_highlight): help for fringe display specs

---
 doc/lispref/display.texi |  9 ++---
 etc/NEWS                 |  6 ++++
 etc/TODO                 |  4 ---
 src/xdisp.c              | 72 ++++++++++++++++++++++++++++++++++++++--
 4 files changed, 80 insertions(+), 11 deletions(-)

diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index 8ad8b04f908..33cb1c25f84 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -5492,14 +5492,15 @@ Other Display Specs
 but it is done as a special case of marginal display (@pxref{Display
 Margins}).

-@item (left-fringe @var{bitmap} @r{[}@var{face}@r{]})
-@itemx (right-fringe @var{bitmap} @r{[}@var{face}@r{]})
+@item (left-fringe @var{bitmap} @r{[}@var{face}@r{]} @r{[}@var{help-echo}@r{]})
+@itemx (right-fringe @var{bitmap} @r{[}@var{face}@r{]} @r{[}@var{help-echo}@r{]})
 This display specification on any character of a line of text causes
 the specified @var{bitmap} be displayed in the left or right fringes
 for that line, instead of the characters that have the display
 specification.  The optional @var{face} specifies the face whose
-colors are to be used for the bitmap display.  @xref{Fringe Bitmaps},
-for the details.
+colors are to be used for the bitmap display.  The optional
+@var{help-echo} string can be used to display tooltips or help text in
+the echo area.  @xref{Fringe Bitmaps}, for the details.

 @item (space-width @var{factor})
 This display specification affects all the space characters within the
diff --git a/etc/NEWS b/etc/NEWS
index 6df17aa3f0a..87ba0e326b3 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1350,6 +1350,12 @@ values.
 \f
 * Lisp Changes in Emacs 30.1

++++
+** Tooltips for user fringe indicators.
+User fringe indicators defined in text display specifications now
+support user-defined tooltips. See the "Other Display Specifications"
+node in the Emacs Lisp Reference Manual.
+
 +++
 ** New 'pop-up-frames' action alist entry for 'display-buffer'.
 This has the same effect as the variable of the same name and takes
diff --git a/etc/TODO b/etc/TODO
index d2d124c9c8e..01f1423cf8e 100644
--- a/etc/TODO
+++ b/etc/TODO
@@ -156,10 +156,6 @@ Change them to use report-emacs-bug.
 **** lm-report-bug
 **** tramp-bug
 **** c-submit-bug-report
-
-** Allow fringe indicators to display a tooltip
-Provide a help-echo property?
-
 ** Add a defcustom that supplies a function to name numeric backup files
 Like 'make-backup-file-name-function' for non-numeric backup files.

diff --git a/src/xdisp.c b/src/xdisp.c
index 2a979c5cb9e..fd94abdd379 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -35468,6 +35468,68 @@ note_mode_line_or_margin_highlight (Lisp_Object window, int x, int y,
 }


+/* Take proper action when mouse has moved to the window WINDOW, with
+   window-local x-position X and y-position Y. This is only used for
+   displaying user-defined fringe indicator help-echo messages.    */
+
+static void
+note_fringe_highlight (Lisp_Object window, int x, int y,
+		       enum window_part part)
+{
+  if (!NILP (help_echo_string))
+    return;
+
+  /* Find a message to display through through the help-echo mechanism
+     whenever the mouse hovers over a fringe indicator. The message
+     can come from any position in the row, invisibile portions of the
+     buffer included. So we iterate over all positions that the
+     current text row depends onthe current row, checking both
+     properties and overlays.  */
+
+  /* Translate windows coordinates into a vertical window position. */
+  int hpos, vpos, area;
+  struct window *w = XWINDOW (window);
+  x_y_to_hpos_vpos (w, x, y, &hpos, &vpos, 0, 0, &area);
+
+  /* Get to the current glyph row. */
+  struct glyph_row *row = MATRIX_ROW (w->current_matrix, vpos);
+
+  /* No fringe indicators - no need to look things up  */
+  if (row->left_user_fringe_bitmap == 0 &&
+      row->right_user_fringe_bitmap == 0)
+    return;
+
+  /* Check display specs for both visual and invisible text in the
+     row. */
+  ptrdiff_t charpos;
+  ptrdiff_t charpos_start = row->minpos.charpos;
+  ptrdiff_t charpos_end = row->maxpos.charpos;
+  for (charpos = charpos_start; charpos <= charpos_end; charpos++)
+    {
+      /* Properties in question can be either in text props or
+         overlays, so check both. */
+      Lisp_Object spec = get_char_property_and_overlay (make_fixnum (charpos),
+							Qdisplay, Qnil, NULL);
+      if (NILP(spec) || !CONSP(spec))
+	continue;
+
+      /* Hovering over the right fringe - check the right-fringe
+         spec  */
+      /* Hovering over the left fringe - check the left-fringe
+         spec  */
+      if (!(EQ (XCAR (spec), Qleft_fringe) && part == ON_LEFT_FRINGE) &&
+	  !(EQ (XCAR (spec), Qright_fringe) && part == ON_RIGHT_FRINGE))
+	continue;
+
+      /* Get the caption while ingoring all non-strings */
+      Lisp_Object fringe_help_echo = CAR_SAFE (CDR_SAFE (CDR_SAFE (CDR_SAFE (spec))));
+      if (!STRINGP (fringe_help_echo))
+	continue;
+
+      help_echo_string = fringe_help_echo;
+    }
+}
+
 /* EXPORT:
    Take proper action when the mouse has moved to position X, Y on
    frame F with regards to highlighting portions of display that have
@@ -35695,10 +35757,14 @@ note_mouse_highlight (struct frame *f, int x, int y)
       }
     else
       cursor = FRAME_OUTPUT_DATA (f)->nontext_cursor;
-  else if (part == ON_LEFT_FRINGE || part == ON_RIGHT_FRINGE
-	   || part == ON_VERTICAL_SCROLL_BAR
-	   || part == ON_HORIZONTAL_SCROLL_BAR)
+  else if (part == ON_LEFT_FRINGE || part == ON_RIGHT_FRINGE) {
     cursor = FRAME_OUTPUT_DATA (f)->nontext_cursor;
+
+    note_fringe_highlight (window, x, y, part);
+  }
+  else if (part == ON_VERTICAL_SCROLL_BAR
+	   || part == ON_HORIZONTAL_SCROLL_BAR)
+      cursor = FRAME_OUTPUT_DATA (f)->nontext_cursor;
   else
     cursor = FRAME_OUTPUT_DATA (f)->text_cursor;
 #endif
--
2.34.1