Subject: Re: bug#64440: 30.0.50; [PATCH] Highlight on non toolkit menu bar items

From: Po Lu <[email-masked]>
Date: Tue, 12 Sep 2023 19:26:20 +0800

Manuel Giraud <[email-masked]> writes:

> +#if defined HAVE_WINDOW_SYSTEM && !defined HAVE_EXT_MENU_BAR
> +
> +/* Get information about the menu-bar item at position X/Y on frame F.
> +   Return menu-bar's item char position in H_START/H_END and pixel
> +   position in X_START/X_END.  Value is
> +
> +   -1	if X/Y is not on a menu-bar item
> +   0	if X/Y is on the same item that was highlighted before.
> +   1	otherwise.  */
> +
> +static int
> +get_menu_bar_item (struct frame *f, int x, int y, int *h_start, int *h_end,
> +		   int *x_start, int *x_end, int *vpos)
> +{
> +  Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (f);
> +  struct window *w = XWINDOW (f->menu_bar_window);
> +  struct glyph_row *row;
> +  int dummy;
> +  Lisp_Object items;
> +  int i;
> +
> +  /* Find glyph's hpos and vpos under X/Y.  */
> +  if (x_y_to_hpos_vpos (w, x, y, h_start, vpos, NULL, NULL, &dummy) == NULL)
> +    return -1;
> +
> +  /* We do not support R2L menu bar.  */
> +  row = MATRIX_ROW (w->current_matrix, *vpos);
> +  if (row->reversed_p)
> +    return -1;
> +
> +  /* Compute h_start and h_end for this menu bar item.  */
> +  items = FRAME_MENU_BAR_ITEMS (f);
> +  for (i = 0; i < ASIZE (items); i += 4)
> +    {
> +      Lisp_Object pos, string;
> +      string = AREF (items, i + 1);
> +      pos = AREF (items, i + 3);
> +      if (NILP (string))
> +	return -1;
> +      if (*h_start >= XFIXNUM (pos)
> +	  && *h_start < XFIXNUM (pos) + SCHARS (string))
> +	{
> +	  *h_start = XFIXNUM (pos);
> +	  *h_end = *h_start + SCHARS (string);
> +	  break;
> +	}
> +    }
> +
> +  /* Convert to pixels bounds.  */
> +  *x_start = 0;
> +  for (i = 0; i < *h_start; ++i)
> +    *x_start += row->glyphs[TEXT_AREA][i].pixel_width;
> +
> +  *x_end = *x_start;
> +  for (i = *h_start;
> +       CHARPOS (row->glyphs[TEXT_AREA][i]) != -1
> +	 && i < row->used[TEXT_AREA];
> +       ++i)
> +    *x_end += row->glyphs[TEXT_AREA][i].pixel_width;
> +
> +  /* Is mouse on the highlighted item?  */
> +  if (EQ (f->menu_bar_window, hlinfo->mouse_face_window)
> +      && *vpos >= hlinfo->mouse_face_beg_row
> +      && *vpos <= hlinfo->mouse_face_end_row
> +      && (*vpos > hlinfo->mouse_face_beg_row
> +	  || *h_start >= hlinfo->mouse_face_beg_col)
> +      && (*vpos < hlinfo->mouse_face_end_row
> +	  || *h_end < hlinfo->mouse_face_end_col
> +	  || hlinfo->mouse_face_past_end))
> +    return 0;
> +
> +  return 1;
> +}
> +
> +/* Possibly highlight a menu-bar item on frame F when mouse moves to
> +   menu-bar window-relative coordinates X/Y.  Called from
> +   note_mouse_highlight.  */
> +
> +static void
> +note_menu_bar_highlight (struct frame *f, int x, int y)
> +{
> +  Lisp_Object window = f->menu_bar_window;
> +  Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (f);
> +  int h_start, h_end, vpos, x_start, x_end;
> +  int rc;
> +
> +  /* Function note_mouse_highlight is called with negative X/Y
> +     values when mouse moves outside of the frame.  */
> +  if (x <= 0 || y <= 0)
> +    {
> +      clear_mouse_face (hlinfo);
> +      return;
> +    }
> +
> +  h_start = h_end = 0;
> +  rc = get_menu_bar_item (f, x, y, &h_start, &h_end, &x_start, &x_end, &vpos);
> +  if (rc < 0)
> +    {
> +      /* Not on menu-bar item.  */
> +      clear_mouse_face (hlinfo);
> +      return;
> +    }
> +  else if (rc == 0)
> +    /* On same menu-bar item as before.  */
> +    return;
> +
> +  if (!NILP (Vmouse_highlight))
> +    {
> +      /* Record this as the current active region.  */
> +      hlinfo->mouse_face_beg_col = h_start;
> +      hlinfo->mouse_face_beg_row = vpos;
> +      hlinfo->mouse_face_beg_x = x_start;
> +      hlinfo->mouse_face_past_end = false;
> +
> +      hlinfo->mouse_face_end_col = h_end;
> +      hlinfo->mouse_face_end_row = vpos;
> +      hlinfo->mouse_face_end_x = x_end;
> +      hlinfo->mouse_face_window = window;
> +      hlinfo->mouse_face_face_id = MENU_FACE_ID;
> +
> +      /* Display it as active.  */
> +      show_mouse_face (hlinfo, DRAW_MOUSE_FACE);
> +    }
> +}
> +#endif
>  
>  
>  /***********************************************************************
> @@ -35538,6 +35663,16 @@ note_mouse_highlight (struct frame *f, int x, int y)
>    w = XWINDOW (window);
>    frame_to_window_pixel_xy (w, &x, &y);
>  
> +#if defined HAVE_WINDOW_SYSTEM && !defined HAVE_EXT_MENU_BAR
> +  /* Handle menu-bar window differently since it doesn't display a
> +     buffer.  */
> +  if (EQ (window, f->menu_bar_window))
> +    {
> +      note_menu_bar_highlight (f, x, y);
> +      return;
> +    }
> +#endif
> +

Thanks, but I'd prefer an option to disable this.  Additionally, could
you insert the preprocessor condition in a comment after each #endif?
Some of the preprocessor blocks are very long.