GNU bug report logs - #58406
29.0.50; Bars refactoring?

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: 58406 <at> debbugs.gnu.org
Subject: bug#58406: 29.0.50; Bars refactoring?
Date: Mon, 10 Oct 2022 11:45:31 +0300

> From: Manuel Giraud <manuel <at> ledu-giraud.fr>
> Date: Mon, 10 Oct 2022 09:37:43 +0200
> 
> I'm trying to have some kind of highlight feature on the no-toolkit menu
> bar.  As this feature is already present in tool/tab bar, I'd like to
> mimic those.
> 
> While trying this, I think that I found that some "bar" functions could
> be factorized (up to a certain point I guess).  Here is a patch that
> shows my direction.
> 
> Do you think that this approach could make it into master or would it be
> frown upon?  And if so, what would be a better way?

The approach is OK, and welcome.  But let's please wait with this
refactoring until after the emacs-29 branch is cut.  OK?

Some minor comments below:

> +  /* Get the text property `menu-item' at pos. The value of that
> +     property is the start index of this item's properties in
> +     F->tool_bar_items.  */

The "F->tool_bar_items" part of the comment is outdated.

Also, our style is to leave two spaces between sentences in comments
and documentation.

> +  /* Get the start of this tool-bar item's properties in
> +     f->tool_bar_items.  */

Likewise here: outdated reference to tool_bar_items.

> +  /* Is mouse on the highlighted item?  */
> +  if (bar == TAB_BAR)
> +    return *prop_idx == f->last_tab_bar_item ? 0 : 1;
> +  else if (EQ (window, hlinfo->mouse_face_window)
> +      && *vpos >= hlinfo->mouse_face_beg_row
> +      && *vpos <= hlinfo->mouse_face_end_row
> +      && (*vpos > hlinfo->mouse_face_beg_row
> +	  || *hpos >= hlinfo->mouse_face_beg_col)
> +      && (*vpos < hlinfo->mouse_face_end_row
> +	  || *hpos < hlinfo->mouse_face_end_col
> +	  || hlinfo->mouse_face_past_end))
> +    return 0;

The "else if" clause should only be used for the tool bar, not for the
menu bar, AFAIU.

> @@ -15485,6 +15451,7 @@ handle_tool_bar_click_with_device (struct frame *f, int x, int y, bool down_p,
>    Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (f);
>    struct window *w = XWINDOW (f->tool_bar_window);
>    int hpos, vpos, prop_idx;
> +  bool close_p;

The value of this is ignored here, so it is better to call this
variable 'ignored' or 'dummy' or something to that effect.

Previous Next

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