unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [PATCH] User-defined fringe tooltips (a request for review)
@ 2023-12-19 19:38 Vladimir Kazanov
  2023-12-20 12:31 ` Eli Zaretskii
  0 siblings, 1 reply; 28+ messages in thread
From: Vladimir Kazanov @ 2023-12-19 19:38 UTC (permalink / raw)
  To: emacs-devel

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

Hi all,

Recently I've been looking into implementing a suggestion from
etc/TODO related to fringe indicator tooltips:

> ** Allow fringe indicators to display a tooltip
> Provide a help-echo property?

Attached is a quick and dirty proof-of-concept. The patch extends the
display specification with an optional string that would be displayed
through either tooltips or the echo area:

(overlay-put (make-overlay (point) (point))
   'before-string (propertize "x" 'display `(left-fringe right-arrow
nil "left fringe test tooltip")))

Moving a mouse pointer to the fringe would then display the string in
a tooltip on a relevant line.

To make this work I've extended "struct it" and "struct glyphs_row"
with pointers to strings, and added some mouse hover handling in the
"note_mouse_highlight function". Obviously, the code needs more checks
but I want to confirm the approach with emacs-devel@ first.

Happy to hear comments and suggestions about the path taken!

Thank you

-- 
Regards,

Vladimir Kazanov

[-- Attachment #2: 0001-first-draft-version-of-user-fringe-tooltips.patch --]
[-- Type: text/x-patch, Size: 4764 bytes --]

From 09551c3a191201ebe45e1345e5c515b59ad3d728 Mon Sep 17 00:00:00 2001
From: Vladimir Kazanov <vekazanov@gmail.com>
Date: Tue, 19 Dec 2023 19:11:26 +0000
Subject: [PATCH] first draft version of user fringe tooltips

---
 src/dispextern.h | 12 ++++++++++++
 src/xdisp.c      | 39 +++++++++++++++++++++++++++++++++++++--
 2 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/src/dispextern.h b/src/dispextern.h
index ece128949f5..53dee99ab05 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -940,6 +940,12 @@ #define CHECK_MATRIX(MATRIX) ((void) 0)
      it specifies actual fringe bitmap number.  */
   int overlay_arrow_bitmap;

+  /* Left fringe caption (Qnil or a stringp)*/
+  Lisp_Object left_user_fringe_caption;
+
+  /* Right fringe caption (Qnil or a stringp)*/
+  Lisp_Object right_user_fringe_caption;
+
   /* Left fringe bitmap number (enum fringe_bitmap_type).  */
   unsigned left_user_fringe_bitmap : FRINGE_ID_BITS;

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

+  /* Left fringe caption (Qnil or a stringp) */
+  Lisp_Object left_user_fringe_caption;
+
+  /* Right fringe caption  (Qnil or a stringp)*/
+  Lisp_Object right_user_fringe_caption;
+
   /* 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 fdb4acd71bf..0e8a18da358 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -3252,6 +3252,10 @@ init_iterator (struct it *it, struct window *w,
   /* Clear IT, and set it->object and other IT's Lisp objects to Qnil.
      Other parts of redisplay rely on that.  */
   memclear (it, sizeof *it);
+
+  it->left_user_fringe_caption = Qnil;
+  it->right_user_fringe_caption = Qnil;
+
   it->current.overlay_string_index = -1;
   it->current.dpvec_index = -1;
   it->base_face_id = remapped_base_face_id;
@@ -6082,6 +6086,13 @@ handle_single_display_spec (struct it *it, Lisp_Object spec, Lisp_Object object,
 		face_id = face_id2;
 	    }

+	  Lisp_Object caption = Qnil;
+	  if (CONSP (XCDR (XCDR (XCDR (spec)))))
+	    {
+	      caption = XCAR (XCDR (XCDR (XCDR (spec))));
+	    }
+
+
 	  /* Save current settings of IT so that we can restore them
 	     when we are finished with the glyph property value.  */
 	  push_it (it, position);
@@ -6105,11 +6116,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_caption = caption;
 	    }
 	  else
 	    {
 	      it->right_user_fringe_bitmap = fringe_bitmap;
 	      it->right_user_fringe_face_id = face_id;
+	      it->right_user_fringe_caption = caption;
 	    }
 	}
 #endif /* HAVE_WINDOW_SYSTEM */
@@ -25593,13 +25606,19 @@ #define RECORD_MAX_MIN_POS(IT)					\
   /* Save fringe bitmaps in this row.  */
   row->left_user_fringe_bitmap = it->left_user_fringe_bitmap;
   row->left_user_fringe_face_id = it->left_user_fringe_face_id;
+  row->left_user_fringe_caption = it->left_user_fringe_caption;
+
   row->right_user_fringe_bitmap = it->right_user_fringe_bitmap;
   row->right_user_fringe_face_id = it->right_user_fringe_face_id;
+  row->right_user_fringe_caption = it->right_user_fringe_caption;

   it->left_user_fringe_bitmap = 0;
   it->left_user_fringe_face_id = 0;
+  it->left_user_fringe_caption = Qnil;
+
   it->right_user_fringe_bitmap = 0;
   it->right_user_fringe_face_id = 0;
+  it->right_user_fringe_caption = Qnil;

   /* When they turn off tooltip-mode on a GUI frame, we call 'message'
      with message-truncate-lines bound to non-nil, which produces
@@ -35273,8 +35292,24 @@ 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;
+
+	  if (NILP (help_echo_string)) {
+	      int hpos, vpos, area;
+	      x_y_to_hpos_vpos (w, x, y, &hpos, &vpos, 0, 0, &area);
+	      struct glyph_row *r = MATRIX_ROW (w->current_matrix, vpos);
+	      if (part == ON_LEFT_FRINGE
+		  && !NILP(r->left_user_fringe_caption))
+		  help_echo_string = r->left_user_fringe_caption;
+	      else if (part == ON_RIGHT_FRINGE
+		       && !NILP(r->right_user_fringe_caption))
+	          help_echo_string = r->right_user_fringe_caption;
+	  }
+
+      }
+  else if (part == ON_VERTICAL_SCROLL_BAR
 	   || part == ON_HORIZONTAL_SCROLL_BAR)
     cursor = FRAME_OUTPUT_DATA (f)->nontext_cursor;
   else
--
2.34.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH] User-defined fringe tooltips (a request for review)
  2023-12-19 19:38 [PATCH] User-defined fringe tooltips (a request for review) Vladimir Kazanov
@ 2023-12-20 12:31 ` Eli Zaretskii
  2023-12-21 16:51   ` Vladimir Kazanov
  0 siblings, 1 reply; 28+ messages in thread
From: Eli Zaretskii @ 2023-12-20 12:31 UTC (permalink / raw)
  To: Vladimir Kazanov; +Cc: emacs-devel

> From: Vladimir Kazanov <vekazanov@gmail.com>
> Date: Tue, 19 Dec 2023 19:38:09 +0000
> 
> Recently I've been looking into implementing a suggestion from
> etc/TODO related to fringe indicator tooltips:
> 
> > ** Allow fringe indicators to display a tooltip
> > Provide a help-echo property?
> 
> Attached is a quick and dirty proof-of-concept. The patch extends the
> display specification with an optional string that would be displayed
> through either tooltips or the echo area:
> 
> (overlay-put (make-overlay (point) (point))
>    'before-string (propertize "x" 'display `(left-fringe right-arrow
> nil "left fringe test tooltip")))
> 
> Moving a mouse pointer to the fringe would then display the string in
> a tooltip on a relevant line.

Thanks.

> To make this work I've extended "struct it" and "struct glyphs_row"
> with pointers to strings, and added some mouse hover handling in the
> "note_mouse_highlight function". Obviously, the code needs more checks
> but I want to confirm the approach with emacs-devel@ first.
> 
> Happy to hear comments and suggestions about the path taken!

I'm not sure I like to have Lisp_Object members in 'struct glyph_row'.
I also don't think I understand why you needed to copy the hel-echo
string to the glyph row.  The code in note_mouse_highlight examines
the buffer text and overlays for text properties whose meaning is to
display a help-echo string, so why cannot you do the same to look for
a 'display' property of this specific structure, and extract the
help-echo directly from there at mouse-highlight time instead of at
glyph-generation time?



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] User-defined fringe tooltips (a request for review)
  2023-12-20 12:31 ` Eli Zaretskii
@ 2023-12-21 16:51   ` Vladimir Kazanov
  2023-12-21 17:37     ` Eli Zaretskii
  0 siblings, 1 reply; 28+ messages in thread
From: Vladimir Kazanov @ 2023-12-21 16:51 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

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

Thanks for looking into this!

> The code in note_mouse_highlight examines
> the buffer text and overlays for text properties whose meaning is to
> display a help-echo string, so why cannot you do the same to look for
> a 'display' property of this specific structure, and extract the
> help-echo directly from there at mouse-highlight time instead of at
> glyph-generation time?

Yes, having Lispy stuff in the glyph_row struct doesn't make sense,
and this is why I was in doubt.

Anyway, a new patch is attached with the approach you suggested (the
way I understood it). I use "struct it" to iterate over the line for
which the fringe indicator is defined, in note_mouse_highlight. The
iterator then pulls out the display spec and saves the last left/right
fringe captions it walked over.

Does that feel right?

Thank you,
Vlad

On Wed, 20 Dec 2023 at 12:32, Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Vladimir Kazanov <vekazanov@gmail.com>
> > Date: Tue, 19 Dec 2023 19:38:09 +0000
> >
> > Recently I've been looking into implementing a suggestion from
> > etc/TODO related to fringe indicator tooltips:
> >
> > > ** Allow fringe indicators to display a tooltip
> > > Provide a help-echo property?
> >
> > Attached is a quick and dirty proof-of-concept. The patch extends the
> > display specification with an optional string that would be displayed
> > through either tooltips or the echo area:
> >
> > (overlay-put (make-overlay (point) (point))
> >    'before-string (propertize "x" 'display `(left-fringe right-arrow
> > nil "left fringe test tooltip")))
> >
> > Moving a mouse pointer to the fringe would then display the string in
> > a tooltip on a relevant line.
>
> Thanks.
>
> > To make this work I've extended "struct it" and "struct glyphs_row"
> > with pointers to strings, and added some mouse hover handling in the
> > "note_mouse_highlight function". Obviously, the code needs more checks
> > but I want to confirm the approach with emacs-devel@ first.
> >
> > Happy to hear comments and suggestions about the path taken!
>
> I'm not sure I like to have Lisp_Object members in 'struct glyph_row'.
> I also don't think I understand why you needed to copy the hel-echo
> string to the glyph row.  The code in note_mouse_highlight examines
> the buffer text and overlays for text properties whose meaning is to
> display a help-echo string, so why cannot you do the same to look for
> a 'display' property of this specific structure, and extract the
> help-echo directly from there at mouse-highlight time instead of at
> glyph-generation time?



-- 
Regards,

Vladimir Kazanov

[-- Attachment #2: add-user-fringe-indicator-tooltips.patch --]
[-- Type: text/x-patch, Size: 4078 bytes --]

diff --git a/src/dispextern.h b/src/dispextern.h
index 3a4d6095f73..e8f984e1a71 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -2801,6 +2801,12 @@ #define OVERLAY_STRING_CHUNK_SIZE 16
      is in effect, and only in hscrolled windows.  */
   int stretch_adjust;
 
+  /* Left fringe caption (Qnil or a stringp) */
+  Lisp_Object left_user_fringe_caption;
+
+  /* Right fringe caption  (Qnil or a stringp)*/
+  Lisp_Object right_user_fringe_caption;
+
   /* 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 75d769600c4..5c5d39acde1 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -3265,6 +3265,10 @@ init_iterator (struct it *it, struct window *w,
   /* Clear IT, and set it->object and other IT's Lisp objects to Qnil.
      Other parts of redisplay rely on that.  */
   memclear (it, sizeof *it);
+
+  it->left_user_fringe_caption = Qnil;
+  it->right_user_fringe_caption = Qnil;
+
   it->current.overlay_string_index = -1;
   it->current.dpvec_index = -1;
   it->base_face_id = remapped_base_face_id;
@@ -6127,6 +6131,13 @@ handle_single_display_spec (struct it *it, Lisp_Object spec, Lisp_Object object,
 		face_id = face_id2;
 	    }
 
+	  Lisp_Object caption = Qnil;
+          if (CONSP (XCDR (XCDR (XCDR (spec)))))
+            {
+              caption = XCAR (XCDR (XCDR (XCDR (spec))));
+            }
+
+
 	  /* Save current settings of IT so that we can restore them
 	     when we are finished with the glyph property value.  */
 	  push_it (it, position);
@@ -6150,11 +6161,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_caption = caption;
 	    }
 	  else
 	    {
 	      it->right_user_fringe_bitmap = fringe_bitmap;
 	      it->right_user_fringe_face_id = face_id;
+              it->right_user_fringe_caption = caption;
 	    }
 	}
 #endif /* HAVE_WINDOW_SYSTEM */
@@ -35720,10 +35733,50 @@ 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;
+
+      if (NILP (help_echo_string)) {
+	  /* Translate windows coordinates into vertical window
+             position. */
+	  int hpos, vpos, area;
+	  x_y_to_hpos_vpos (w, x, y, &hpos, &vpos, 0, 0, &area);
+
+	  /* Figure out where the window starts - will need to get to
+             the fringe line */
+	  struct text_pos start_pos;
+	  SET_TEXT_POS_FROM_MARKER (start_pos, w->start);
+
+	  /* Need an iterator to walk through all the properties on
+             the line. Find the line first, reset text from previous
+             lines and then collect the properties on it. */
+	  struct it it;
+	  init_iterator (&it, w, CHARPOS(start_pos), BYTEPOS (start_pos),
+			 NULL, DEFAULT_FACE_ID);
+
+	  /* Walk to the fringe line and reset captions found on the
+             way*/
+	  move_it_by_lines(&it, vpos);
+	  it.left_user_fringe_caption = Qnil;
+	  it.right_user_fringe_caption = Qnil;
+
+	  /* Reset and go through the line. */
+	  move_it_by_lines (&it, 1);
+
+	  /* Output through whatever the user prefers, most likely a
+             tooltip */
+	  if (part == ON_LEFT_FRINGE
+	      && !NILP(it.left_user_fringe_caption))
+	      help_echo_string = it.left_user_fringe_caption;
+	  else if (part == ON_RIGHT_FRINGE
+		   && !NILP(it.right_user_fringe_caption))
+              help_echo_string = it.right_user_fringe_caption;
+      }
+
+  }
+  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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH] User-defined fringe tooltips (a request for review)
  2023-12-21 16:51   ` Vladimir Kazanov
@ 2023-12-21 17:37     ` Eli Zaretskii
  2023-12-23 13:28       ` Vladimir Kazanov
  0 siblings, 1 reply; 28+ messages in thread
From: Eli Zaretskii @ 2023-12-21 17:37 UTC (permalink / raw)
  To: Vladimir Kazanov; +Cc: emacs-devel

> From: Vladimir Kazanov <vekazanov@gmail.com>
> Date: Thu, 21 Dec 2023 16:51:15 +0000
> Cc: emacs-devel@gnu.org
> 
> > The code in note_mouse_highlight examines
> > the buffer text and overlays for text properties whose meaning is to
> > display a help-echo string, so why cannot you do the same to look for
> > a 'display' property of this specific structure, and extract the
> > help-echo directly from there at mouse-highlight time instead of at
> > glyph-generation time?
> 
> Yes, having Lispy stuff in the glyph_row struct doesn't make sense,
> and this is why I was in doubt.
> 
> Anyway, a new patch is attached with the approach you suggested (the
> way I understood it). I use "struct it" to iterate over the line for
> which the fringe indicator is defined, in note_mouse_highlight. The
> iterator then pulls out the display spec and saves the last left/right
> fringe captions it walked over.
> 
> Does that feel right?

No, there's no need to use the iterator.  Just look at the positions
recorded in the glyphs of the relevant glyph_row, and then look up
text properties at those positions using Fget_char_property etc., like
we already do to look for the 'help-echo' property in the same
function.  Here's a typical fragment:

	    Lisp_Object obj = glyph->object;
	    ptrdiff_t charpos = glyph->charpos;

	    /* Try text properties.  */
	    if (STRINGP (obj)
		&& charpos >= 0
		&& charpos < SCHARS (obj))
	      {
		help = Fget_text_property (make_fixnum (charpos),
					   Qhelp_echo, obj);

Since the 'display' property that shows a fringe bitmap can be on any
character of a screen line, you will need to loop over all the glyphs
of the glyph_row where the mouse pointer is, testing the 'display'
property as above.  Something like this:

  for (glyph = row->glyphs[TEXT_AREA];
       glyph < row->glyphs[TEXT_AREA] + row->used[TEXT_AREA];
       glyph++)

where 'row' is

   row = MATRIX_ROW (w->current_matrix, vpos);

and 'vpos' is returned by this call which we already have in
note_mouse_highlight:

      /* Find the glyph under X/Y.  */
      glyph = x_y_to_hpos_vpos (w, x, y, &hpos, &vpos, &dx, &dy, &area);

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'.

Then you won't need to use 'struct it', and won't need any new caption
members in 'struct it'.

Thanks.



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] User-defined fringe tooltips (a request for review)
  2023-12-21 17:37     ` Eli Zaretskii
@ 2023-12-23 13:28       ` Vladimir Kazanov
  2023-12-23 13:40         ` Eli Zaretskii
  0 siblings, 1 reply; 28+ messages in thread
From: Vladimir Kazanov @ 2023-12-23 13:28 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

So I've spent some time playing with various buffer text arrangements
and looking into iterator code. Quite a read, I must say!

>
>   for (glyph = row->glyphs[TEXT_AREA];
>        glyph < row->glyphs[TEXT_AREA] + row->used[TEXT_AREA];
>        glyph++)
>
>

Just iterating through glyphs doesn't work as glyphs do not reflect
charpos'es within invisible parts of the buffer. If a
left-fringe/right-fringe display property is specified then the text
is invisible so there's no glyph having that charpos. The iterator
handles all of that, of course, this is why I didn't have to take it
into account the first time.

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... */
   }

> 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?
This might not work in all cases as some major modes remove top-level
help-echo properties for some reason - I didn't look into it
thoroughly.

Either way, I'd need to test this thoroughly, I suspect there might be
interesting corner cases.

Thanks

--
Regards,

Vladimir Kazanov



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] User-defined fringe tooltips (a request for review)
  2023-12-23 13:28       ` Vladimir Kazanov
@ 2023-12-23 13:40         ` Eli Zaretskii
  2023-12-24 11:31           ` Vladimir Kazanov
  0 siblings, 1 reply; 28+ messages in thread
From: Eli Zaretskii @ 2023-12-23 13:40 UTC (permalink / raw)
  To: Vladimir Kazanov; +Cc: emacs-devel

> 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.



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] User-defined fringe tooltips (a request for review)
  2023-12-23 13:40         ` Eli Zaretskii
@ 2023-12-24 11:31           ` Vladimir Kazanov
  2023-12-24 16:54             ` Eli Zaretskii
  0 siblings, 1 reply; 28+ messages in thread
From: Vladimir Kazanov @ 2023-12-24 11:31 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

[-- 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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH] User-defined fringe tooltips (a request for review)
  2023-12-24 11:31           ` Vladimir Kazanov
@ 2023-12-24 16:54             ` Eli Zaretskii
  2024-03-25 15:55               ` Vladimir Kazanov
  0 siblings, 1 reply; 28+ messages in thread
From: Eli Zaretskii @ 2023-12-24 16:54 UTC (permalink / raw)
  To: Vladimir Kazanov; +Cc: emacs-devel

> From: Vladimir Kazanov <vekazanov@gmail.com>
> Date: Sun, 24 Dec 2023 11:31:28 +0000
> Cc: emacs-devel@gnu.org
> 
> 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.

There's no catastrophe if we only support some sources of this display
property; we already do that in other similar cases.  We can later add
more sources if there's demand.

I have some comments on the code and the documentation:

> --- 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.

The @xref sentence should be after the sentence which ends with
"bitmap display", since it describes more details about that, and has
nothing to do with the new help-echo string feature.

> ++++
> +** Tooltips for user fringe indicators.
> +User fringe indicators defined in text display specifications now
> +support user-defined tooltips. See the "Other Display Specifications"
                                ^^
Our conventions are to leave two spaces between sentences.

> +  /* 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);

This was already done by the caller, so no need to do it again;
instead, pass the info as arguments to this function.

> +  /* 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  */

Our style for comments in C is to have them as complete sentences
ending in a period, and leave two spaces between the last sentence and
the closing "*/" comment delimiter.

> +  /* 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);

To avoid punishing display of text that has no such properties, I
would first see if any Qdisplay properties are within this glyph row,
using Fnext_char_property_change, and only enter the loop if that call
returns a position in-range.

> +      /* 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))

Please place each comment before the line of code on which it
comments.

> +	continue;

We prefer not to over-use 'continue' in loops; instead, reverse the
condition in 'if' and do everything inside the 'if' block.

> +      /* 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;

Here, you only support the simplest form of the display property's
value, i.e.

  (left-fringe BITMAP [FACE] HELP-STRING)

But the property's value can have a more complex form:

  . it could be a vector or a list of several display specifications
  . a single specification could have the form (when CONDITION . SPEC)

So I think you need something more complex here; see the loop in
handle_display_prop for the details.

> +  else if (part == ON_LEFT_FRINGE || part == ON_RIGHT_FRINGE) {

This is not our style of placing braces.  We use this style:

   else if (CONDITION)
     {
       ...
     }

Thanks.



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] User-defined fringe tooltips (a request for review)
  2023-12-24 16:54             ` Eli Zaretskii
@ 2024-03-25 15:55               ` Vladimir Kazanov
  2024-03-25 17:11                 ` Eli Zaretskii
  0 siblings, 1 reply; 28+ messages in thread
From: Vladimir Kazanov @ 2024-03-25 15:55 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Hi Eli,

Thank you for looking into this and sorry for the delay - life
happened. Anyways, I have some time now to finish the patch.

I've integrated all of the suggestions but the last one, related to
many forms of display specs.

The code in the (note_fringe_highlight) function works outside of the
main (handle_single_display_spec) call. So to get to the :help-echo
string it has to parse into the spec, or a list of specs, or a vector
of specs, effectively duplicating parsing logic in
handle_single_display_spec. This is not nice but doable.

But every display spec can be wrapped in a (when CONDITION . SPEC)
form. And because (handle_single_display_spec) happens at a time
different from the (note_fringe_highlight) call time we can never be
sure that the condition still holds even if the code reevaluates
CONDITION.

One approach would be to just go into SPEC of the condition without
any reevaluations and get the :help-echo string anyway, hoping that
there are no discrepancies. This would work, hopefully, most of the
time but not all the time.

What do you think?

Thank you

On Sun, 24 Dec 2023 at 16:54, Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Vladimir Kazanov <vekazanov@gmail.com>
> > Date: Sun, 24 Dec 2023 11:31:28 +0000
> > Cc: emacs-devel@gnu.org
> >
> > 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.
>
> There's no catastrophe if we only support some sources of this display
> property; we already do that in other similar cases.  We can later add
> more sources if there's demand.
>
> I have some comments on the code and the documentation:
>
> > --- 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.
>
> The @xref sentence should be after the sentence which ends with
> "bitmap display", since it describes more details about that, and has
> nothing to do with the new help-echo string feature.
>
> > ++++
> > +** Tooltips for user fringe indicators.
> > +User fringe indicators defined in text display specifications now
> > +support user-defined tooltips. See the "Other Display Specifications"
>                                 ^^
> Our conventions are to leave two spaces between sentences.
>
> > +  /* 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);
>
> This was already done by the caller, so no need to do it again;
> instead, pass the info as arguments to this function.
>
> > +  /* 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  */
>
> Our style for comments in C is to have them as complete sentences
> ending in a period, and leave two spaces between the last sentence and
> the closing "*/" comment delimiter.
>
> > +  /* 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);
>
> To avoid punishing display of text that has no such properties, I
> would first see if any Qdisplay properties are within this glyph row,
> using Fnext_char_property_change, and only enter the loop if that call
> returns a position in-range.
>
> > +      /* 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))
>
> Please place each comment before the line of code on which it
> comments.
>
> > +     continue;
>
> We prefer not to over-use 'continue' in loops; instead, reverse the
> condition in 'if' and do everything inside the 'if' block.
>
> > +      /* 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;
>
> Here, you only support the simplest form of the display property's
> value, i.e.
>
>   (left-fringe BITMAP [FACE] HELP-STRING)
>
> But the property's value can have a more complex form:
>
>   . it could be a vector or a list of several display specifications
>   . a single specification could have the form (when CONDITION . SPEC)
>
> So I think you need something more complex here; see the loop in
> handle_display_prop for the details.
>
> > +  else if (part == ON_LEFT_FRINGE || part == ON_RIGHT_FRINGE) {
>
> This is not our style of placing braces.  We use this style:
>
>    else if (CONDITION)
>      {
>        ...
>      }
>
> Thanks.



-- 
Regards,

Vladimir Kazanov



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] User-defined fringe tooltips (a request for review)
  2024-03-25 15:55               ` Vladimir Kazanov
@ 2024-03-25 17:11                 ` Eli Zaretskii
  2024-03-26 22:16                   ` Vladimir Kazanov
  0 siblings, 1 reply; 28+ messages in thread
From: Eli Zaretskii @ 2024-03-25 17:11 UTC (permalink / raw)
  To: Vladimir Kazanov; +Cc: emacs-devel

> From: Vladimir Kazanov <vekazanov@gmail.com>
> Date: Mon, 25 Mar 2024 15:55:16 +0000
> Cc: emacs-devel@gnu.org
> 
> The code in the (note_fringe_highlight) function works outside of the
> main (handle_single_display_spec) call. So to get to the :help-echo
> string it has to parse into the spec, or a list of specs, or a vector
> of specs, effectively duplicating parsing logic in
> handle_single_display_spec. This is not nice but doable.
> 
> But every display spec can be wrapped in a (when CONDITION . SPEC)
> form. And because (handle_single_display_spec) happens at a time
> different from the (note_fringe_highlight) call time we can never be
> sure that the condition still holds even if the code reevaluates
> CONDITION.

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.



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] User-defined fringe tooltips (a request for review)
  2024-03-25 17:11                 ` Eli Zaretskii
@ 2024-03-26 22:16                   ` Vladimir Kazanov
  2024-03-27 10:59                     ` Vladimir Kazanov
  2024-03-27 12:14                     ` Eli Zaretskii
  0 siblings, 2 replies; 28+ messages in thread
From: Vladimir Kazanov @ 2024-03-26 22:16 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

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



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] User-defined fringe tooltips (a request for review)
  2024-03-26 22:16                   ` Vladimir Kazanov
@ 2024-03-27 10:59                     ` Vladimir Kazanov
  2024-03-27 11:25                       ` Po Lu
                                         ` (2 more replies)
  2024-03-27 12:14                     ` Eli Zaretskii
  1 sibling, 3 replies; 28+ messages in thread
From: Vladimir Kazanov @ 2024-03-27 10:59 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

[-- 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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH] User-defined fringe tooltips (a request for review)
  2024-03-27 10:59                     ` Vladimir Kazanov
@ 2024-03-27 11:25                       ` Po Lu
  2024-03-27 12:48                         ` Vladimir Kazanov
  2024-03-27 11:25                       ` Po Lu
  2024-03-31  8:36                       ` Eli Zaretskii
  2 siblings, 1 reply; 28+ messages in thread
From: Po Lu @ 2024-03-27 11:25 UTC (permalink / raw)
  To: Vladimir Kazanov; +Cc: Eli Zaretskii, emacs-devel

There are a number of general stylistic issues with this code, viz. the
use of spaces for indentation, to columns beyond the tab width, the
presence of redundant braces in if/else statements, and that of lines
exceeding 80 columns.  Please see that they are addressed before it is
installed in Emacs, and thanks in advance.



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] User-defined fringe tooltips (a request for review)
  2024-03-27 10:59                     ` Vladimir Kazanov
  2024-03-27 11:25                       ` Po Lu
@ 2024-03-27 11:25                       ` Po Lu
  2024-03-31  8:36                       ` Eli Zaretskii
  2 siblings, 0 replies; 28+ messages in thread
From: Po Lu @ 2024-03-27 11:25 UTC (permalink / raw)
  To: Vladimir Kazanov; +Cc: Eli Zaretskii, emacs-devel

There are a number of general stylistic issues with this code, viz. the
use of spaces for indentation, to columns beyond the tab width, the
presence of redundant braces in if/else statements, and that of lines
exceeding 80 columns.  Please see that they are addressed before it is
installed in Emacs, and thanks in advance.



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] User-defined fringe tooltips (a request for review)
  2024-03-26 22:16                   ` Vladimir Kazanov
  2024-03-27 10:59                     ` Vladimir Kazanov
@ 2024-03-27 12:14                     ` Eli Zaretskii
  2024-03-27 12:48                       ` Vladimir Kazanov
  1 sibling, 1 reply; 28+ messages in thread
From: Eli Zaretskii @ 2024-03-27 12:14 UTC (permalink / raw)
  To: Vladimir Kazanov; +Cc: emacs-devel

> From: Vladimir Kazanov <vekazanov@gmail.com>
> Date: Tue, 26 Mar 2024 22:16:02 +0000
> Cc: emacs-devel@gnu.org
> 
> > 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.

When you say "move it", what is "it" in this case?

> 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.

Why do you need to retrieve the display spec in note_fringe_highlight?

> What I don't really understand is whether I should handle overlays
> with after-string/before-string properties.

Ideally, yes.  Is there some complication there that would make those
overlays special?

> Should I just go through all overlays touching the spec position and
> parse into the after-string/before-string as well?

Not sure I understand the question, but maybe if you answer the ones
above, I will understand, or this question will answer itself.

> Pardon the many questions, display code has many moving parts.

No need to apologize.  Thanks for working on this.



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] User-defined fringe tooltips (a request for review)
  2024-03-27 12:14                     ` Eli Zaretskii
@ 2024-03-27 12:48                       ` Vladimir Kazanov
  0 siblings, 0 replies; 28+ messages in thread
From: Vladimir Kazanov @ 2024-03-27 12:48 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Hi Eli,

> > 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.
>
> When you say "move it", what is "it" in this case?

I record the position of the :help-echo message in the buffer in the
iterator (struct it). This is ptrdiff_t
left_user_fringe_help_echo_pos. Then I copy this buffer position into
(struct glyph_row), next to where left_user_fringe_bitmap sits.

> > 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.
>
> Why do you need to retrieve the display spec in note_fringe_highlight?

Once I have the position of the help-echo string (saved in the
glyph_row), I still have to retrieve the :help-echo string. That's
what the code calling (get_char_property_and_overlay) does in the
(note_fringe_highlight). This gives me a list/vector/etc of display
specs. Somewhere within the specs there should be a fringe spec as
this is what was recorded by the iterator (struct it).

And the fringe spec contains the help-echo message I need to show in
the tooltip.

> > What I don't really understand is whether I should handle overlays
> > with after-string/before-string properties.
>
> Ideally, yes.  Is there some complication there that would make those
> overlays special?

I am not sure I understand everything about overlays but for some
reason I can't get Qafter_string/Qbefore_string overlay properties
from the position saved in the glyph_row when the overlay is not
represented by any visual element in the row.

> > Should I just go through all overlays touching the spec position and
> > parse into the after-string/before-string as well?
>
> Not sure I understand the question, but maybe if you answer the ones
> above, I will understand, or this question will answer itself.

Hope it all makes sense now. I'll try to get things working for all
variants of fringe definition.

-- 
Regards,

Vladimir Kazanov



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] User-defined fringe tooltips (a request for review)
  2024-03-27 11:25                       ` Po Lu
@ 2024-03-27 12:48                         ` Vladimir Kazanov
  0 siblings, 0 replies; 28+ messages in thread
From: Vladimir Kazanov @ 2024-03-27 12:48 UTC (permalink / raw)
  To: Po Lu; +Cc: Eli Zaretskii, emacs-devel

Po,

Thanks for looking at the patch!

Sure, I will fix things in the final version of the patch, the code is
in flux right now.


On Wed, 27 Mar 2024 at 11:25, Po Lu <luangruo@yahoo.com> wrote:
>
> There are a number of general stylistic issues with this code, viz. the
> use of spaces for indentation, to columns beyond the tab width, the
> presence of redundant braces in if/else statements, and that of lines
> exceeding 80 columns.  Please see that they are addressed before it is
> installed in Emacs, and thanks in advance.



--
Regards,

Vladimir Kazanov



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] User-defined fringe tooltips (a request for review)
  2024-03-27 10:59                     ` Vladimir Kazanov
  2024-03-27 11:25                       ` Po Lu
  2024-03-27 11:25                       ` Po Lu
@ 2024-03-31  8:36                       ` Eli Zaretskii
  2024-04-07 11:14                         ` Vladimir Kazanov
  2 siblings, 1 reply; 28+ messages in thread
From: Eli Zaretskii @ 2024-03-31  8:36 UTC (permalink / raw)
  To: Vladimir Kazanov; +Cc: emacs-devel

> From: Vladimir Kazanov <vekazanov@gmail.com>
> Date: Wed, 27 Mar 2024 10:59:50 +0000
> Cc: emacs-devel@gnu.org
> 
> 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")]))

Like I said earlier: I think it would be better to have the tooltip
text in a separate property, not as part of the 'display' property
spec.

Also, I don't see a need for recording the positions in the glyph_row
structure, because finding the glyph that corresponds to buffer/string
text with that special property is easy enough.



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] User-defined fringe tooltips (a request for review)
  2024-03-31  8:36                       ` Eli Zaretskii
@ 2024-04-07 11:14                         ` Vladimir Kazanov
  2024-04-07 12:44                           ` Eli Zaretskii
  0 siblings, 1 reply; 28+ messages in thread
From: Vladimir Kazanov @ 2024-04-07 11:14 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

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

Hi Eli,

Here's a 3rd version of the patch. Please, ignore documentation, which
I'll update later, and take a look at the code itself.

> Like I said earlier: I think it would be better to have the tooltip
> text in a separate property, not as part of the 'display' property
> spec.
>
> Also, I don't see a need for recording the positions in the glyph_row
> structure, because finding the glyph that corresponds to buffer/string
> text with that special property is easy enough.

Done.

The code in the patch goes through glyphs of the row under a mouse
pointer, finds an object that generated the glyph (glyph->object,
which can be a buffer or a string) and checks if
left-fringe-help/right-fringe-help text properties are defined.
Strings from the properties are then displayed as tooltips.

This only works for properties of visible text that has glyphs
generated for it. Properties coming from invisible text and
zero-length overlays are not represented as glyphs as much I can see
from the code.  Here are some code examples:

;;; tooltips will be shown in the examples below:

  (insert (propertize "foo" 'left-fringe-help "left tooltip"))
  (insert (propertize "foo" 'right-fringe-help "right tooltip"))

  (overlay-put (make-overlay (point) (1+ (point)))
               'right-fringe-help "right tooltip")

  (overlay-put
   (make-overlay (point) (1+ (point)))
   'after-string
   (propertize
    "lll"
    'left-fringe-help "left tooltip"))

  (overlay-put
   (make-overlay (point) (1+ (point)))
   'after-string
   (propertize
    "rrr"
    'right-fringe-help "right tooltip"))

;;; Tooltips will NOT be shown
  (insert (propertize "foo" 'display '(left-fringe left-arrow warning)
                      'left-fringe-help "left tooltip"))
  (insert (propertize "foo" 'display '(right-fringe left-arrow warning)
                      'right-fringe-help "right tooltip"))
  (insert (propertize "foo" 'left-fringe-help "left tooltip"))

  (insert (propertize "foo" 'right-fringe-help "right tooltip"))

(overlay-put
   (make-overlay 1 2)
   'before-string (propertize
                   "x"
                   'left-fringe-help "left tooltip"
                   'display '(left-fringe right-arrow warning)))

So this is a partial solution. If you're fine with the general
approach, we can do this in step-by-step fashion. I'll fix the
documentation and provide a final version of this implementation, and
then think about what to do about invisible text.

Thanks!

-- 
Regards,

Vladimir Kazanov

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

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

---
 doc/lispref/display.texi |  7 +++--
 etc/NEWS                 |  6 ++++
 etc/TODO                 |  4 ---
 src/frame.c              |  2 ++
 src/xdisp.c              | 61 ++++++++++++++++++++++++++++++++++++++--
 5 files changed, 71 insertions(+), 9 deletions(-)

diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index f82c2fad14d..b3b81bd342c 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 32cec82f970..b1f72cb9bf4 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1815,6 +1815,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/frame.c b/src/frame.c
index abd6ef00901..ff99b0353af 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -6383,6 +6383,7 @@ syms_of_frame (void)
   DEFSYM (Qchild_frame_border_width, "child-frame-border-width");
   DEFSYM (Qinternal_border_width, "internal-border-width");
   DEFSYM (Qleft_fringe, "left-fringe");
+  DEFSYM (Qleft_fringe_help, "left-fringe-help");
   DEFSYM (Qline_spacing, "line-spacing");
   DEFSYM (Qmenu_bar_lines, "menu-bar-lines");
   DEFSYM (Qtab_bar_lines, "tab-bar-lines");
@@ -6390,6 +6391,7 @@ syms_of_frame (void)
   DEFSYM (Qname, "name");
   DEFSYM (Qright_divider_width, "right-divider-width");
   DEFSYM (Qright_fringe, "right-fringe");
+  DEFSYM (Qright_fringe_help, "right-fringe-help");
   DEFSYM (Qscreen_gamma, "screen-gamma");
   DEFSYM (Qscroll_bar_background, "scroll-bar-background");
   DEFSYM (Qscroll_bar_foreground, "scroll-bar-foreground");
diff --git a/src/xdisp.c b/src/xdisp.c
index 140d71129f3..b4d57b5b6f2 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -35730,6 +35730,59 @@ 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.  */
+
+  /* Check the text property symbol to use.  */
+  Lisp_Object sym;
+  if (part == ON_LEFT_FRINGE)
+    sym = Qleft_fringe_help;
+  else
+    sym = Qright_fringe_help;
+
+  /* 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 first glyph of a text row based on the vertical position
+     of the fringe.  */
+  struct glyph *glyph = MATRIX_ROW_GLYPH_START(w->current_matrix, vpos);
+  int glyph_num = MATRIX_ROW_USED(w->current_matrix, vpos);
+
+  /* Check all glyphs while looking for fringe tooltips.  */
+
+  /* NOTE: iterating over glyphs can only find text properties coming
+     from visible text.  This means that zero-length overlays and
+     invisibile text are NOT inspected.  */
+  for (;glyph_num; glyph_num--, glyph++)
+    {
+      Lisp_Object pos = make_fixnum(glyph->charpos);
+      Lisp_Object help_echo = Qnil;
+
+      if (STRINGP(glyph->object) || BUFFERP(glyph->object))
+	help_echo = get_char_property_and_overlay (pos, sym, glyph->object, NULL);
+
+      if (STRINGP (help_echo))
+	{
+	  help_echo_string = help_echo;
+	  break;
+	}
+    }
+}
+
 /* 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,8 +36010,12 @@ 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;
   else
--
2.34.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH] User-defined fringe tooltips (a request for review)
  2024-04-07 11:14                         ` Vladimir Kazanov
@ 2024-04-07 12:44                           ` Eli Zaretskii
  2024-04-07 17:07                             ` Vladimir Kazanov
  0 siblings, 1 reply; 28+ messages in thread
From: Eli Zaretskii @ 2024-04-07 12:44 UTC (permalink / raw)
  To: Vladimir Kazanov; +Cc: emacs-devel

> From: Vladimir Kazanov <vekazanov@gmail.com>
> Date: Sun, 7 Apr 2024 12:14:54 +0100
> Cc: emacs-devel@gnu.org
> 
> > Like I said earlier: I think it would be better to have the tooltip
> > text in a separate property, not as part of the 'display' property
> > spec.
> >
> > Also, I don't see a need for recording the positions in the glyph_row
> > structure, because finding the glyph that corresponds to buffer/string
> > text with that special property is easy enough.
> 
> Done.
> 
> The code in the patch goes through glyphs of the row under a mouse
> pointer, finds an object that generated the glyph (glyph->object,
> which can be a buffer or a string) and checks if
> left-fringe-help/right-fringe-help text properties are defined.
> Strings from the properties are then displayed as tooltips.
> 
> This only works for properties of visible text that has glyphs
> generated for it. Properties coming from invisible text and
> zero-length overlays are not represented as glyphs as much I can see
> from the code.

Since there can be only one fringe bitmap displayed on each screen
line, Lisp programs don't need the left/right-fringe-help to be put on
the same positions as the display property that generated the fringe
bitmaps.  They can place the left/right-fringe-help property on _any_
position within the same screen line.  (Of course, to make sure these
properties end up on the same screen line as the fringe bitmap, they
should be on positions as close as possible to the ones which cause
the bitmap to be displayed on the fringe, but that shouldn't be too
hard.)  So I think the danger of not showing the tooltip due to
invisible text or text that didn't produce any glyphs is not a real
one, provided that we lift the restriction of having the
left/right-fringe-help property on the same positions as the display
property which produces the fringe bitmap display.  Do you agree?

> ;;; Tooltips will NOT be shown
>   (insert (propertize "foo" 'display '(left-fringe left-arrow warning)
>                       'left-fringe-help "left tooltip"))
>   (insert (propertize "foo" 'display '(right-fringe left-arrow warning)
>                       'right-fringe-help "right tooltip"))
>   (insert (propertize "foo" 'left-fringe-help "left tooltip"))
> 
>   (insert (propertize "foo" 'right-fringe-help "right tooltip"))
> 
> (overlay-put
>    (make-overlay 1 2)
>    'before-string (propertize
>                    "x"
>                    'left-fringe-help "left tooltip"
>                    'display '(left-fringe right-arrow warning)))

It should be enough to put the left/right-fringe-help property on the
character immediately following the display property, to get the
tooltip to display in those cases.



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] User-defined fringe tooltips (a request for review)
  2024-04-07 12:44                           ` Eli Zaretskii
@ 2024-04-07 17:07                             ` Vladimir Kazanov
  2024-04-07 18:40                               ` Eli Zaretskii
  0 siblings, 1 reply; 28+ messages in thread
From: Vladimir Kazanov @ 2024-04-07 17:07 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

> So I think the danger of not showing the tooltip due to
> invisible text or text that didn't produce any glyphs is not a real
> one, provided that we lift the restriction of having the
> left/right-fringe-help property on the same positions as the display
> property which produces the fringe bitmap display.  Do you agree?

With the implementation provided in the latest patch it doesn't matter
where bitmap was specified, or if it was specified at all. This means,
for example, that the first left-fringe-help property on the line will
be used as a tooltip on the left fringe row for the same line with or
*without* a bitmap.

> It should be enough to put the left/right-fringe-help property on the
> character immediately following the display property, to get the
> tooltip to display in those cases.

It is possible, yes, if you are fine with this approach. Here's a
review of options considered:

1. Use the iterator it used in the display_line function to extract
tooltips from fringe-related display specs (patch v1). This needs
caching Lisp_Objects in glyph_row structures for future retrieval.
This is good because it looks in all specs everywhere, doesn't matter
if the text is visible or not. Bad because it needs storing additional
non-text values in glyph_row.
2. Iterating over buffer text positions of the row (patch v2). Hard to
take into account all variants of text coming from buffers and
strings. This implementation only finds fringe-help in visible text of
the buffer.
3. Iterating of glyphs of the row (patch v3). All visible text, both
from the buffer and strings, is visited by the implementation.

I'd say that option 1 is best for user code as specs are retrieved uniformly.

Option 3 is the cleanest implementation-wise but we'd have to document
limitations and the suggested solution.

So if you're fine with this approach, I'll clean up the documentation
and provide a final patch based on option 3.

-- 
Regards,

Vladimir Kazanov



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] User-defined fringe tooltips (a request for review)
  2024-04-07 17:07                             ` Vladimir Kazanov
@ 2024-04-07 18:40                               ` Eli Zaretskii
  2024-04-08 14:41                                 ` Vladimir Kazanov
  0 siblings, 1 reply; 28+ messages in thread
From: Eli Zaretskii @ 2024-04-07 18:40 UTC (permalink / raw)
  To: Vladimir Kazanov; +Cc: emacs-devel

> From: Vladimir Kazanov <vekazanov@gmail.com>
> Date: Sun, 7 Apr 2024 18:07:31 +0100
> Cc: emacs-devel@gnu.org
> 
> 1. Use the iterator it used in the display_line function to extract
> tooltips from fringe-related display specs (patch v1). This needs
> caching Lisp_Objects in glyph_row structures for future retrieval.
> This is good because it looks in all specs everywhere, doesn't matter
> if the text is visible or not. Bad because it needs storing additional
> non-text values in glyph_row.
> 2. Iterating over buffer text positions of the row (patch v2). Hard to
> take into account all variants of text coming from buffers and
> strings. This implementation only finds fringe-help in visible text of
> the buffer.
> 3. Iterating of glyphs of the row (patch v3). All visible text, both
> from the buffer and strings, is visited by the implementation.
> 
> I'd say that option 1 is best for user code as specs are retrieved uniformly.

But it has a disadvantage of storing Lisp objects in the glyph row
structure (which means basically to store them in the glyph matrices).
I'd like to avoid that.

> Option 3 is the cleanest implementation-wise but we'd have to document
> limitations and the suggested solution.
> 
> So if you're fine with this approach, I'll clean up the documentation
> and provide a final patch based on option 3.

Yes, please.



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] User-defined fringe tooltips (a request for review)
  2024-04-07 18:40                               ` Eli Zaretskii
@ 2024-04-08 14:41                                 ` Vladimir Kazanov
  2024-04-13  9:14                                   ` Eli Zaretskii
  0 siblings, 1 reply; 28+ messages in thread
From: Vladimir Kazanov @ 2024-04-08 14:41 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

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

Hi Eli,

Here is the v4 patch. As we discussed, this is option 3 i.e., looking
up properties in visible text only. In addition to the implementation,
I've fixed the docs, NEWS and TODO files.

Thanks

-- 
Regards,

Vladimir Kazanov

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

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

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

diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index f82c2fad14d..fd083083fd2 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -5501,6 +5501,10 @@ Other Display Specs
 colors are to be used for the bitmap display.  @xref{Fringe Bitmaps},
 for the details.

+It also possible to add context help for fringe bitmaps through the
+@code{show-help-function} mechanism by using @code{left-fringe-help} or
+@code{right-fringe-help} text properties (@pxref{Special Properties}).
+
 @item (space-width @var{factor})
 This display specification affects all the space characters within the
 text that has the specification.  It displays all of these spaces
diff --git a/doc/lispref/text.texi b/doc/lispref/text.texi
index 18f0ee88fe5..3db82df49b3 100644
--- a/doc/lispref/text.texi
+++ b/doc/lispref/text.texi
@@ -3663,6 +3663,15 @@ Special Properties
 is displayed as-is by @code{show-help-function}, without being passed
 through @code{substitute-command-keys}.

+@item left-fringe-help
+@itemx right-fringe-help
+@cindex help-echo text on fringes
+If any visible text of a buffer line has @code{left-fringe-help} or
+@code{right-fringe-help} string text property defined on it, then the
+string will be displayed for a corresponding line's fringe through
+@code{show-help-function} (@pxref{Help display}).  This is useful when
+used together with fringe cursors and bitmaps (@pxref{Fringes}).
+
 @item keymap
 @cindex keymap of character
 @kindex keymap @r{(text property)}
diff --git a/etc/NEWS b/etc/NEWS
index 32cec82f970..38fb9819714 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1815,6 +1815,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 on fringes.
+It is now possible to provide tooltips on fringes by adding special text
+properties.  See the "Special Properties" Info 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/frame.c b/src/frame.c
index abd6ef00901..ff99b0353af 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -6383,6 +6383,7 @@ syms_of_frame (void)
   DEFSYM (Qchild_frame_border_width, "child-frame-border-width");
   DEFSYM (Qinternal_border_width, "internal-border-width");
   DEFSYM (Qleft_fringe, "left-fringe");
+  DEFSYM (Qleft_fringe_help, "left-fringe-help");
   DEFSYM (Qline_spacing, "line-spacing");
   DEFSYM (Qmenu_bar_lines, "menu-bar-lines");
   DEFSYM (Qtab_bar_lines, "tab-bar-lines");
@@ -6390,6 +6391,7 @@ syms_of_frame (void)
   DEFSYM (Qname, "name");
   DEFSYM (Qright_divider_width, "right-divider-width");
   DEFSYM (Qright_fringe, "right-fringe");
+  DEFSYM (Qright_fringe_help, "right-fringe-help");
   DEFSYM (Qscreen_gamma, "screen-gamma");
   DEFSYM (Qscroll_bar_background, "scroll-bar-background");
   DEFSYM (Qscroll_bar_foreground, "scroll-bar-foreground");
diff --git a/src/xdisp.c b/src/xdisp.c
index 140d71129f3..b4d57b5b6f2 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -35730,6 +35730,59 @@ 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.  */
+
+  /* Check the text property symbol to use.  */
+  Lisp_Object sym;
+  if (part == ON_LEFT_FRINGE)
+    sym = Qleft_fringe_help;
+  else
+    sym = Qright_fringe_help;
+
+  /* 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 first glyph of a text row based on the vertical position
+     of the fringe.  */
+  struct glyph *glyph = MATRIX_ROW_GLYPH_START(w->current_matrix, vpos);
+  int glyph_num = MATRIX_ROW_USED(w->current_matrix, vpos);
+
+  /* Check all glyphs while looking for fringe tooltips.  */
+
+  /* NOTE: iterating over glyphs can only find text properties coming
+     from visible text.  This means that zero-length overlays and
+     invisibile text are NOT inspected.  */
+  for (;glyph_num; glyph_num--, glyph++)
+    {
+      Lisp_Object pos = make_fixnum(glyph->charpos);
+      Lisp_Object help_echo = Qnil;
+
+      if (STRINGP(glyph->object) || BUFFERP(glyph->object))
+	help_echo = get_char_property_and_overlay (pos, sym, glyph->object, NULL);
+
+      if (STRINGP (help_echo))
+	{
+	  help_echo_string = help_echo;
+	  break;
+	}
+    }
+}
+
 /* 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,8 +36010,12 @@ 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;
   else
--
2.34.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH] User-defined fringe tooltips (a request for review)
  2024-04-08 14:41                                 ` Vladimir Kazanov
@ 2024-04-13  9:14                                   ` Eli Zaretskii
  2024-04-13  9:32                                     ` Vladimir Kazanov
  0 siblings, 1 reply; 28+ messages in thread
From: Eli Zaretskii @ 2024-04-13  9:14 UTC (permalink / raw)
  To: Vladimir Kazanov; +Cc: emacs-devel

> From: Vladimir Kazanov <vekazanov@gmail.com>
> Date: Mon, 8 Apr 2024 15:41:38 +0100
> Cc: emacs-devel@gnu.org
> 
> Here is the v4 patch. As we discussed, this is option 3 i.e., looking
> up properties in visible text only. In addition to the implementation,
> I've fixed the docs, NEWS and TODO files.

Thanks.  Can you post some simple Lisp to test this feature after
applying the patch?  I'd like to run some tests before I install it.



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] User-defined fringe tooltips (a request for review)
  2024-04-13  9:14                                   ` Eli Zaretskii
@ 2024-04-13  9:32                                     ` Vladimir Kazanov
  2024-04-13 11:21                                       ` Eli Zaretskii
  0 siblings, 1 reply; 28+ messages in thread
From: Vladimir Kazanov @ 2024-04-13  9:32 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Hi Eli,

Sure, here's the code I used to test tooltips. Just eval the
corresponding forms:

  (insert (propertize "foo" 'left-fringe-help "left tooltip"))
  (insert (propertize "foo" 'right-fringe-help "right tooltip"))

  (overlay-put
   (make-overlay (point) (1+ (point)))
   'after-string
   (propertize
    "lll"
    'left-fringe-help "left tooltip"))

  (overlay-put
   (make-overlay (point) (1+ (point)))
   'after-string
   (propertize
    "rrr"
    'right-fringe-help "right tooltip"))

On Sat, 13 Apr 2024 at 10:14, Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Vladimir Kazanov <vekazanov@gmail.com>
> > Date: Mon, 8 Apr 2024 15:41:38 +0100
> > Cc: emacs-devel@gnu.org
> >
> > Here is the v4 patch. As we discussed, this is option 3 i.e., looking
> > up properties in visible text only. In addition to the implementation,
> > I've fixed the docs, NEWS and TODO files.
>
> Thanks.  Can you post some simple Lisp to test this feature after
> applying the patch?  I'd like to run some tests before I install it.



-- 
Regards,

Vladimir Kazanov



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] User-defined fringe tooltips (a request for review)
  2024-04-13  9:32                                     ` Vladimir Kazanov
@ 2024-04-13 11:21                                       ` Eli Zaretskii
  2024-04-13 14:53                                         ` Vladimir Kazanov
  0 siblings, 1 reply; 28+ messages in thread
From: Eli Zaretskii @ 2024-04-13 11:21 UTC (permalink / raw)
  To: Vladimir Kazanov; +Cc: emacs-devel

> From: Vladimir Kazanov <vekazanov@gmail.com>
> Date: Sat, 13 Apr 2024 10:32:01 +0100
> Cc: emacs-devel@gnu.org
> 
> Sure, here's the code I used to test tooltips. Just eval the
> corresponding forms:
> 
>   (insert (propertize "foo" 'left-fringe-help "left tooltip"))
>   (insert (propertize "foo" 'right-fringe-help "right tooltip"))
> 
>   (overlay-put
>    (make-overlay (point) (1+ (point)))
>    'after-string
>    (propertize
>     "lll"
>     'left-fringe-help "left tooltip"))
> 
>   (overlay-put
>    (make-overlay (point) (1+ (point)))
>    'after-string
>    (propertize
>     "rrr"
>     'right-fringe-help "right tooltip"))

Thanks, I've now installed the changes on the master branch.



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] User-defined fringe tooltips (a request for review)
  2024-04-13 11:21                                       ` Eli Zaretskii
@ 2024-04-13 14:53                                         ` Vladimir Kazanov
  2024-04-13 15:47                                           ` Eli Zaretskii
  0 siblings, 1 reply; 28+ messages in thread
From: Vladimir Kazanov @ 2024-04-13 14:53 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

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

Phew..! Took a while.

Thank you for your endless patience and guidance!

On Sat, 13 Apr 2024, 12:21 Eli Zaretskii, <eliz@gnu.org> wrote:

> > From: Vladimir Kazanov <vekazanov@gmail.com>
> > Date: Sat, 13 Apr 2024 10:32:01 +0100
> > Cc: emacs-devel@gnu.org
> >
> > Sure, here's the code I used to test tooltips. Just eval the
> > corresponding forms:
> >
> >   (insert (propertize "foo" 'left-fringe-help "left tooltip"))
> >   (insert (propertize "foo" 'right-fringe-help "right tooltip"))
> >
> >   (overlay-put
> >    (make-overlay (point) (1+ (point)))
> >    'after-string
> >    (propertize
> >     "lll"
> >     'left-fringe-help "left tooltip"))
> >
> >   (overlay-put
> >    (make-overlay (point) (1+ (point)))
> >    'after-string
> >    (propertize
> >     "rrr"
> >     'right-fringe-help "right tooltip"))
>
> Thanks, I've now installed the changes on the master branch.
>

[-- Attachment #2: Type: text/html, Size: 1662 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] User-defined fringe tooltips (a request for review)
  2024-04-13 14:53                                         ` Vladimir Kazanov
@ 2024-04-13 15:47                                           ` Eli Zaretskii
  0 siblings, 0 replies; 28+ messages in thread
From: Eli Zaretskii @ 2024-04-13 15:47 UTC (permalink / raw)
  To: Vladimir Kazanov; +Cc: emacs-devel

> From: Vladimir Kazanov <vekazanov@gmail.com>
> Date: Sat, 13 Apr 2024 15:53:35 +0100
> Cc: emacs-devel@gnu.org
> 
> Phew..! Took a while. 
> 
> Thank you for your endless patience and guidance! 

You are welcome.  And thanks for working on this in the first place.



^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2024-04-13 15:47 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-19 19:38 [PATCH] User-defined fringe tooltips (a request for review) Vladimir Kazanov
2023-12-20 12:31 ` Eli Zaretskii
2023-12-21 16:51   ` Vladimir Kazanov
2023-12-21 17:37     ` Eli Zaretskii
2023-12-23 13:28       ` Vladimir Kazanov
2023-12-23 13:40         ` Eli Zaretskii
2023-12-24 11:31           ` Vladimir Kazanov
2023-12-24 16:54             ` Eli Zaretskii
2024-03-25 15:55               ` Vladimir Kazanov
2024-03-25 17:11                 ` Eli Zaretskii
2024-03-26 22:16                   ` Vladimir Kazanov
2024-03-27 10:59                     ` Vladimir Kazanov
2024-03-27 11:25                       ` Po Lu
2024-03-27 12:48                         ` Vladimir Kazanov
2024-03-27 11:25                       ` Po Lu
2024-03-31  8:36                       ` Eli Zaretskii
2024-04-07 11:14                         ` Vladimir Kazanov
2024-04-07 12:44                           ` Eli Zaretskii
2024-04-07 17:07                             ` Vladimir Kazanov
2024-04-07 18:40                               ` Eli Zaretskii
2024-04-08 14:41                                 ` Vladimir Kazanov
2024-04-13  9:14                                   ` Eli Zaretskii
2024-04-13  9:32                                     ` Vladimir Kazanov
2024-04-13 11:21                                       ` Eli Zaretskii
2024-04-13 14:53                                         ` Vladimir Kazanov
2024-04-13 15:47                                           ` Eli Zaretskii
2024-03-27 12:14                     ` Eli Zaretskii
2024-03-27 12:48                       ` Vladimir Kazanov

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).