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

[Vladimir Kazanov <vekazanov@gmail.com>]

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

Hi Eli,

Attached is a v2 patch (for discussion, not yet finished) that
demonstrates the approach. Examples of what works and what doesn't:

;; single spec, list of specs, vector of specs all work now
  (insert (propertize "foo" 'display '(right-fringe left-arrow warning
"right tooltip")))foo
  (insert (propertize "foo" 'display '((left-fringe right-arrow
warning "left tooltip"))))foo
  (insert (propertize "foo" 'display '[(right-fringe left-arrow
warning "right tooltip")]))

;; overlays with display props also work
  (overlay-put
   (make-overlay 1 2)
   'display `(left-fringe right-arrow warning "left tooltip"))

;; overlays with zero length DO NOT work (and provide no fringe indicators)
  (overlay-put
   (make-overlay 1 1)
   'display `(left-fringe right-arrow warning "left tooltip"))

;; 'before-string and 'after-string DO NOT work
  (overlay-put
   (make-overlay (point) (point))
   'before-string (propertize
                   "x" 'display
                   `(left-fringe right-arrow warning "left tooltip")))

Adding (when CONDITION . SPEC) support is easy, still figuring out
'before-string/'after-string.

Thanks

On Tue, 26 Mar 2024 at 22:16, Vladimir Kazanov <vekazanov@gmail.com> wrote:
>
> Hey,
>
> I just did a deep dive into the code again - looking for a cleaner solution.
>
> > The condition is only evaluated at glyph generation time, as for any
> > other display property.  When the tooltip should be displayed, the
> > condition is not relevant, since if it were false when the glyphs were
> > generated, you wouldn't have had the fringe bitmap shown in the first
> > place.
> >
> > However, I think there's an easy way of making the implementation
> > easier: just introduce a new overlay property called, say,
> > fringe-help-echo, and put that property on the same text as the
> > display property which produces the fringe.  Then the code in
> > note_fringe_highlight should simply check if the text which yielded
> > the fringe bitmap also has this special property on it, and if so,
> > display the tooltip.
>
> Maybe an even easier solution would work:
>
> 1. Record buffer position of the fringe display spec in struct it.
> Then move it to glyph_row the same way user_fringe_bitmap_id's are
> copied over.  It is just a ptrdiff so nothing out of place for
> glyph_rows.
>
> 2. In note_fringe_highlight it becomes easy to retrieve the fringe
> display spec using a single call to get_char_property_and_overlay(),
> no need to iterate over a line, or have the implementation detail leak
> into text properties.
>
> What I don't really understand is whether I should handle overlays
> with after-string/before-string properties. Overlays can contain
> strings propertized with fringe display specs as well. In fact, there
> is an example showing this trick in the manual
> (https://www.gnu.org/software/emacs/manual/html_node/elisp/Fringe-Bitmaps.html).
>
> Should I just go through all overlays touching the spec position and
> parse into the after-string/before-string as well?
>
> Pardon the many questions, display code has many moving parts.
>
> Thank you
>
> --
> Regards,
>
> Vladimir Kazanov



-- 
Regards,

Vladimir Kazanov

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

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

---
 doc/lispref/display.texi |   7 ++-
 etc/NEWS                 |   6 ++
 etc/TODO                 |   4 --
 src/dispextern.h         |  13 ++++
 src/xdisp.c              | 124 ++++++++++++++++++++++++++++++++++++++-
 5 files changed, 144 insertions(+), 10 deletions(-)

diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index b497967c445..31408e58679 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.
+for the details.  The optional @var{help-echo} string can be used to
+display tooltips or help text in the echo area.

 @item (space-width @var{factor})
 This display specification affects all the space characters within the
diff --git a/etc/NEWS b/etc/NEWS
index 73af6ab773e..b15cffe73f9 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1741,6 +1741,12 @@ the handler code without unwinding the stack, such that we can record
 the backtrace and other dynamic state at the point of the error.  See
 the Info node "(elisp) Handling Errors".

++++
+** 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 52c77ccc28d..21b504ad18b 100644
--- a/etc/TODO
+++ b/etc/TODO
@@ -172,10 +172,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/dispextern.h b/src/dispextern.h
index 1c3232fae3d..8e41e03f680 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -22,6 +22,7 @@ Copyright (C) 1985, 1993-1994, 1997-2024 Free Software Foundation, Inc.
 #ifndef DISPEXTERN_H_INCLUDED
 #define DISPEXTERN_H_INCLUDED

+#include <stddef.h>
 #include "character.h"

 #ifdef HAVE_X_WINDOWS
@@ -975,6 +976,12 @@ #define CHECK_MATRIX(MATRIX) ((void) 0)
      it specifies actual fringe bitmap number.  */
   int overlay_arrow_bitmap;

+  /* Left fringe help echo position within a buffer  */
+  ptrdiff_t left_user_fringe_help_echo_pos;
+
+  /* Right fringe help echo position within a buffer*/
+  ptrdiff_t right_user_fringe_help_echo_pos;
+
   /* Left fringe bitmap number (enum fringe_bitmap_type).  */
   unsigned left_user_fringe_bitmap : FRINGE_ID_BITS;

@@ -2811,6 +2818,12 @@ #define OVERLAY_STRING_CHUNK_SIZE 16
      is in effect, and only in hscrolled windows.  */
   int stretch_adjust;

+  /* Left fringe help echo position within a buffer  */
+  ptrdiff_t left_user_fringe_help_echo_pos;
+
+  /* Right fringe help echo position within a buffer*/
+  ptrdiff_t right_user_fringe_help_echo_pos;
+
   /* Left fringe bitmap number (enum fringe_bitmap_type).  */
   unsigned left_user_fringe_bitmap : FRINGE_ID_BITS;

diff --git a/src/xdisp.c b/src/xdisp.c
index 140d71129f3..6d81a3041e1 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -6248,11 +6248,13 @@ handle_single_display_spec (struct it *it, Lisp_Object spec, Lisp_Object object,
 	    {
 	      it->left_user_fringe_bitmap = fringe_bitmap;
 	      it->left_user_fringe_face_id = face_id;
+              it->left_user_fringe_help_echo_pos = bufpos;
 	    }
 	  else
 	    {
 	      it->right_user_fringe_bitmap = fringe_bitmap;
 	      it->right_user_fringe_face_id = face_id;
+              it->right_user_fringe_help_echo_pos = bufpos;
 	    }
 	}
 #endif /* HAVE_WINDOW_SYSTEM */
@@ -26019,13 +26021,20 @@ #define RECORD_MAX_MIN_POS(IT)					\
        && it->ellipsis_p);

   /* Save fringe bitmaps in this row.  */
+  row->left_user_fringe_help_echo_pos = it->left_user_fringe_help_echo_pos;
   row->left_user_fringe_bitmap = it->left_user_fringe_bitmap;
   row->left_user_fringe_face_id = it->left_user_fringe_face_id;
+
+  row->right_user_fringe_help_echo_pos = it->right_user_fringe_help_echo_pos;
   row->right_user_fringe_bitmap = it->right_user_fringe_bitmap;
   row->right_user_fringe_face_id = it->right_user_fringe_face_id;

+  /* Reset fringe info.  */
+  it->left_user_fringe_help_echo_pos = 0;
   it->left_user_fringe_bitmap = 0;
   it->left_user_fringe_face_id = 0;
+
+  it->right_user_fringe_help_echo_pos = 0;
   it->right_user_fringe_bitmap = 0;
   it->right_user_fringe_face_id = 0;

@@ -35730,6 +35739,109 @@ 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 the help-echo mechanism whenever
+     the mouse hovers over a fringe indicator.  Both text properties and
+     overlays have to be checked.  */
+
+  /* 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 glyph row based on the vertical position of the
+     fringe.  */
+  struct glyph_row *row = MATRIX_ROW (w->current_matrix, vpos);
+
+  /* The glyph row contains info on fringe indicators found.  */
+  ptrdiff_t bufpos;
+  Lisp_Object spec_sym;
+  if (part == ON_LEFT_FRINGE && row->left_user_fringe_bitmap != 0)
+    {
+      bufpos = row->left_user_fringe_help_echo_pos;
+      spec_sym = Qleft_fringe;
+    }
+  else if (part == ON_RIGHT_FRINGE && row->right_user_fringe_bitmap != 0)
+    {
+      bufpos = row->right_user_fringe_help_echo_pos;
+      spec_sym = Qright_fringe;
+    }
+  else
+    return;
+
+  /* Lookup display properties in text properties and overlays at buffer
+     position.  */
+
+  Lisp_Object disp_prop = get_char_property_and_overlay (make_fixnum (bufpos),
+							 Qdisplay, w->contents, NULL);
+  /* TODO: the property where the fringe indicator comes from is
+     probably in hidden overlay strings  */
+  if (NILP(disp_prop))
+      return;
+
+  /* TODO: refactor this into a function, or extend find_display_prop */
+
+  /* Look for fringe specs while handle display property formats: single
+     spec, list of specs, vector of specs.  */
+
+  Lisp_Object fringe_spec = Qnil;
+  if (!NILP(disp_prop) && CONSP(disp_prop))
+    {
+
+      /* The spec is either a list of display specs or a single
+         spec.  */
+      if (EQ (XCAR (disp_prop), spec_sym))
+        {
+          fringe_spec = disp_prop;
+        }
+      else
+        {
+          for (; CONSP (disp_prop); disp_prop = XCDR (disp_prop))
+            {
+              Lisp_Object spec = XCAR(disp_prop);
+              if (CONSP (spec) && EQ (XCAR (spec), spec_sym))
+                {
+                  fringe_spec = spec;
+                }
+            }
+        }
+    }
+    /* The spec is a vector, which the code has to look for fringe
+       specs in the elements of the vector.   */
+  else if (!NILP(disp_prop) && VECTORP (disp_prop))
+    {
+      ptrdiff_t i;
+      for (i = 0; i < ASIZE (disp_prop); ++i)
+        {
+          Lisp_Object spec = AREF (disp_prop, i);
+          if (!NILP(spec) && EQ (XCAR (spec), spec_sym))
+            {
+              fringe_spec = spec;
+            }
+        }
+    }
+
+  /* TODO: nothing found, the fringe is probably in
+     after-string/before-string  */
+  if (NILP(fringe_spec))
+      return;
+
+  /* TODO: (when CONDITION . SPEC) */
+  Lisp_Object fringe_help_echo = CAR_SAFE (CDR_SAFE (CDR_SAFE (CDR_SAFE (fringe_spec))));
+  if (STRINGP (fringe_help_echo))
+    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
@@ -35957,10 +36069,15 @@ 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
+  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;
+      cursor = FRAME_OUTPUT_DATA (f)->nontext_cursor;
   else
     cursor = FRAME_OUTPUT_DATA (f)->text_cursor;
 #endif
@@ -35982,6 +36099,7 @@ note_mouse_highlight (struct frame *f, int x, int y)
       bool same_region;

       /* Find the glyph under X/Y.  */
+      /* TODO: move up? And pass the data into note_frindge highlight */
       glyph = x_y_to_hpos_vpos (w, x, y, &hpos, &vpos, &dx, &dy, &area);

 #ifdef HAVE_WINDOW_SYSTEM
--
2.34.1