On Fri, Feb 28, 2025 at 12:42 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Ship Mints <shipmints@gmail.com>
> > Date: Fri, 28 Feb 2025 11:20:31 -0500
> > Cc: juri@linkov.net, 76394@debbugs.gnu.org
> >
> > I tried.  I hope the revised log is acceptable.
>
> Thanks, a minor comment about that below.
>
> >  > +       if ( EQ (window, hlinfo->mouse_face_window)
> >  > +            && (!row->reversed_p
> >
> >  Can a glyph row that corresponds to a tab bar be reversed?  IOW, can
> >  the tab bar be ever displayed right-to-left?  I don't think so, but if
> >  I'm wrong, can you describe a scenario where it can happen?
> >
> > There's a comment in display_tab_bar:
> >
> >   /* FIXME: This should be controlled by a user option.  See the
> >      comments in redisplay_tool_bar and display_mode_line about
> >      this.  */
> >   it.paragraph_embedding = L2R;
> >
> > I figured let's be defensive for when this is implemented.
>
> What we usually do in these cases is to put an assertion there, so
> that if the condition we assume to always be true ever isn't, we are
> informed about that in the most violent manner.
>

So we'll be super duper defensive, then.  Should I leave the "future" code
in place after the assertion or do you think it's easy enough to recreate
if/when needed?

> * src/xdisp.c (note_tab_bar_highlight): mouse-face properties.
>
> This is not a complete sentence.  I guess you meant something like
>
>   * src/xdisp.c (note_tab_bar_highlight): Handle mouse-face property.
>
> > * lisp/tab-bar.el: Add face tab-bar-tab-highlight.
>
> Our usual style is a bit different:
>
>   * lisp/tab-bar.el (tab-bar-tab-highlight): New face.
>

I'll update.

But I have a question: why add this face if there's no code that uses
> it?  Or what did I miss?
>
> > -  bool close_p;
> > -  enum draw_glyphs_face draw = DRAW_IMAGE_RAISED;
> > -  int rc;
> > +    Lisp_Object window = f->tab_bar_window;
> > +    struct window *w = XWINDOW (window);
> > +    Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (f);
>

The idea is that now that there's mouse-face support, people can customize
a dedicated face for tab-bar tabs.  This is in line with tab-line tabs and
its highlight face.

Why does indention of the new code use 4 spaces, not 2?
>

No idea.  The .dir-locals.el is in effect.  I checked several buffer locals
to ensure that was true.  What should I look at to help analyze this?  I
suspect others will have this issue as well when helping with C code.

-Stephane