Subject: Re: bug#58406: 29.0.50; Bars refactoring?

From: Manuel Giraud <[email-masked]>
Date: Mon, 10 Oct 2022 13:58:40 +0200

Eli Zaretskii <[email-masked]> writes:

[...]

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

Yes.  Po Lu made that point clear too.

[...]

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

[...]

> Likewise here: outdated reference to tool_bar_items.

Yes.  Sorry to have wasted your time: this patch is far from clean (and
it shows in comment and the like).  It was really just to show my
direction.

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

I don't know yet 😅

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

Yes it is a good idea.  I'll try to remember that next time.
-- 
Manuel Giraud