On 2025-02-06 01:34, Eli Zaretskii wrote: >> Date: Wed, 05 Feb 2025 21:47:38 -0800 >> From: Jared Finder >> Cc: martin rudalics , Eli Zaretskii , >> 75844@debbugs.gnu.org >> >> > I have no more feedback. >> >> Attached is a patch with the issue around dragging the header line not >> mentioned. I'll be filing a bug immediately after this to track fixing >> header line dragging (it will take a bit longer to address). > > Thanks, a few minor comments below. Almost all comments addressed. Some additional follow-up... >> +@vindex window-tool-bar-style >> +@cindex Window Tool Bar style > > Index entries should preferably be all-lowercase, to make sure their > sorting does not depend on the locale where the manual is generated. 0002.patch also does this for tool-bar-mode docs. >> + :type '(choice (const :tag "Images" :value image) >> + (const :tag "Text" :value text) >> + ;; This option would require multiple tool bar >> lines. >> + ;;(const :tag "Both" :value both) >> + (const :tag "Both-horiz" :value both-horiz) >> + (const :tag "Text-image-horiz" :value >> text-image-horiz) >> + (const :tag "Inherit tool-bar-style" :value >> tool-bar-style) >> + (const :tag "System default" :value nil)) > > Many of these tags have cryptic text. Can we make this text more > user-friendly? It is there to explain the meaning of each value to > the users when they customize the option. This too. >> +(defun window-tool-bar--style () >> + "Return the effective style based on `window-tool-bar-style'. >> + >> +This also takes into account frame capabilities. If the current >> +frame cannot display images (see `display-images-p'), then this >> +will always return text." >> + (if (not (display-images-p)) >> + 'text > > Should we perhaps test for support of specific image types? I don't think type checks are needed. A tool bar keymap entries' :image property gets built by tool-bar--image-expression which uses find-image to get the best supported image format for an icon. -- MJF