This should be a "clean" patch. The R2L code can be removed or left in place. I put a comment on the assertion to that effect. On Fri, Feb 28, 2025 at 1:06 PM Ship Mints wrote: > On Fri, Feb 28, 2025 at 12:42 PM Eli Zaretskii wrote: > >> > From: Ship Mints >> > 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 >