GNU bug report logs - #64440
30.0.50; [PATCH] Highlight on non toolkit menu bar items

Previous Next

Full log

View this message in rfc822 format

From: Eli Zaretskii <eliz <at> gnu.org>
To: Manuel Giraud <manuel <at> ledu-giraud.fr>
Cc: luangruo <at> yahoo.com, stefankangas <at> gmail.com, 64440 <at> debbugs.gnu.org
Subject: bug#64440: 30.0.50; [PATCH] Highlight on non toolkit menu bar items
Date: Sun, 10 Sep 2023 10:31:22 +0300

> From: Manuel Giraud <manuel <at> ledu-giraud.fr>
> Cc: Po Lu <luangruo <at> yahoo.com>,  stefankangas <at> gmail.com,  64440 <at> debbugs.gnu.org
> Date: Tue, 05 Sep 2023 11:53:38 +0200
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > I'd appreciate a walkthrough of the patches with explanations for the
> > significant hunks.  It's a non-trivial change, so I think the
> > rationale and the main ideas of the implementation should be described
> > and discussed.
> 
> Hi,
> 
> For this walkthrough, I have inserted the 4 patches, made some elisions
> and comment inline.

Thanks.

> +  /* Convert to pixels bounds.  */
> +  row = MATRIX_ROW (w->current_matrix, *vpos);
> +  *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; i < *h_end; ++i)
> +    *x_end += row->glyphs[TEXT_AREA][i].pixel_width;
> 
> Here, I convert those limits from chars to pixels.

What does this glyph_row look like?  Does it include several strings
one after the other or something?

In general, we always test the index of a glyph in a glyph row against
glyphs[TEXT_AREA].used, and I'm worried that these tests are absent
from the code above.  Which is why I'm asking for more details about
the arrangement of the menu text in these glyph rows.

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

Is this function under a suitable #if condition?  AFAIU, it is only
appropriate in a build with X but without any toolkit, so it shouldn't
be compiled in other configurations.

> --- a/src/xdisp.c
> +++ b/src/xdisp.c
> @@ -33878,6 +33878,9 @@ show_mouse_face (Mouse_HLInfo *hlinfo, enum draw_glyphs_face draw)
>    if (FRAME_WINDOW_P (f) && NILP (track_mouse))
>      {
>        if (draw == DRAW_NORMAL_TEXT
> +#ifndef HAVE_EXT_MENU_BAR
> +	  && !EQ (hlinfo->mouse_face_window, f->menu_bar_window)
> +#endif

Won't this cpp conditional be true in a build --without-x?  Should it?

Thanks for working on this.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.