Subject: Re: bug#75844: Update for window-tool-bar
From: Eli Zaretskii <[email-masked]>
To: Jared Finder <[email-masked]>
Cc: [email-masked], [email-masked], [email-masked]
Date: Sat, 08 Feb 2025 13:15:47 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

> Date: Thu, 06 Feb 2025 14:49:56 -0800
> From: Jared Finder <[email-masked]>
> Cc: [email-masked], [email-masked], [email-masked]
> 
> >> 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.

Thanks, installed on the master branch, and closing the bug.