GNU bug report logs - #68765
30.0.50; Adding window-tool-bar package.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 68765 in the body.
You can then email your comments to 68765 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jared Finder <jared <at> finder.org>
To: Bug-gnu Emacs <bug-gnu-emacs <at> gnu.org>
Subject: 30.0.50; Adding window-tool-bar package.
Date: Sat, 27 Jan 2024 15:36:56 -0800

[Message part 1 (text/plain, inline)]

This is a follow-up from bug#68334 where there was interest in adding 
support into Emacs core for tool bars in windows. 
(https://debbugs.gnu.org/cgi/bugreport.cgi?bug=68334#52)  I have 
attached three changes I authored to accomplish this, two small changes 
to tool-bar and tab-line, and then a code drop of my window-tool-bar.el 
package.  I have not yet put in documentation in NEWS or the manual 
about window-tool-bar and was hoping to get a code review done first.

I think it would be good to have window-tool-bar.el also be a package so 
that I can keep supporting Emacs 29 as well.  I am happy to have this 
put on GNU ELPA and would appreciate help in getting that completed.

I am open to renaming the package and public functions if that's 
desired.  I like the name "window-tool-bar-mode" as it clearly relates 
to tool-bar-mode.

After these patches are accepted, I'd also like to clean up the existing 
tool bar maps for built in minor modes.  This would allow me to address 
Eli's request to have a usable tool bar for M-x gdb 
(https://debbugs.gnu.org/cgi/bugreport.cgi?bug=68334#23).

For tracking purposes, I signed copyright paperwork back in 2020, so 
that shouldn't be an issue.  Thanks.

  -- MJF

[0001-Enable-multiple-modes-to-appear-in-tab-line.patch (text/x-diff, attachment)]

[0002-Add-user-option-to-only-display-default-tool-bar.patch (text/x-diff, attachment)]

[0003-Adding-window-tool-bar-package.patch (text/x-diff, attachment)]

Message #10 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jared Finder <jared <at> finder.org>, Juri Linkov <juri <at> linkov.net>,
 Philip Kaludercic <philipk <at> posteo.net>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 68765 <at> debbugs.gnu.org
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Sat, 10 Feb 2024 10:19:37 +0200

> Date: Sat, 27 Jan 2024 15:36:56 -0800
> From:  Jared Finder via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> This is a follow-up from bug#68334 where there was interest in adding 
> support into Emacs core for tool bars in windows. 
> (https://debbugs.gnu.org/cgi/bugreport.cgi?bug=68334#52)  I have 
> attached three changes I authored to accomplish this, two small changes 
> to tool-bar and tab-line, and then a code drop of my window-tool-bar.el 
> package.  I have not yet put in documentation in NEWS or the manual 
> about window-tool-bar and was hoping to get a code review done first.

Thanks, please find some review comments below.

> Subject: [PATCH 1/3] Enable multiple modes to appear in tab line
> 
> This adds space for window-tool-bar-mode, which will be added in an
> upcoming commit.
> 
> * lisp/tab-line.el (tab-line-format-template): Add separator space.
> (tab-line-display-order): New user variable to control display order.
> (tab-line--runtime-display-order, tab-line--cookie): New internal
> variables.
> (tab-line-set-display): New function for modes to call to enable only
> their content.
> (tab-line-mode): Call `tab-line-set-display'.

Juri, would you please review this part of the changeset?

> Subject: [PATCH 2/3] Add user option to only display default tool bar
> 
> This works well with `window-tool-bar-mode', to be added in upcoming
> commit.  Then the default tool bar is displayed frame-wide and
> mode-specific tool bars are displayed in the window that mode is
> active in.
> 
> * lisp/tool-bar.el (tool-bar-always-show-default): New user option.
> (tool-bar--cache-key, tool-bar-make-keymap-1): Return default tool bar
> when option is set.
> ---
>  lisp/tool-bar.el | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/lisp/tool-bar.el b/lisp/tool-bar.el
> index 96b61c7b229..685e4dfbb86 100644
> --- a/lisp/tool-bar.el
> +++ b/lisp/tool-bar.el
> @@ -100,7 +100,9 @@ secondary-tool-bar-map
>  (defconst tool-bar-keymap-cache (make-hash-table :test #'equal))
>  
>  (defsubst tool-bar--cache-key ()
> -  (cons (frame-terminal) (sxhash-eq tool-bar-map)))
> +  (cons (frame-terminal)
> +        (sxhash-eq (if tool-bar-always-show-default (default-value 'tool-bar-map)
> +                     tool-bar-map))))
>  
>  (defsubst tool-bar--secondary-cache-key ()
>    (cons (frame-terminal) (sxhash-eq secondary-tool-bar-map)))
> @@ -191,7 +193,9 @@ tool-bar-make-keymap-1
>  				      bind))
>  		  (plist-put plist :image image)))
>  	      bind))
> -	  (or map tool-bar-map)))
> +	  (or map
> +              (if tool-bar-always-show-default (default-value 'tool-bar-map)
> +                tool-bar-map))))
>  
>  ;;;###autoload
>  (defun tool-bar-add-item (icon def key &rest props)
> @@ -377,6 +381,15 @@ tool-bar-setup
>  	     (modify-all-frames-parameters
>  	      (list (cons 'tool-bar-position val))))))
>  
> +(defcustom tool-bar-always-show-default nil
> +  "If non-nil, do not show mode-specific tool bars.

Double negation alert!

> I think it would be good to have window-tool-bar.el also be a package so 
> that I can keep supporting Emacs 29 as well.  I am happy to have this 
> put on GNU ELPA and would appreciate help in getting that completed.
> [...]
> From 24ed752ec0bfdded24cba4892426c2c9710126cf Mon Sep 17 00:00:00 2001
> From: Jared Finder <jared <at> finder.org>
> Date: Fri, 26 Jan 2024 15:44:12 -0800
> 
> ---
>  lisp/window-tool-bar.el | 488 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 488 insertions(+)
>  create mode 100644 lisp/window-tool-bar.el

Philip and Stefan, would you please review this additional package,
which AFAIU is being proposed as a cofre package for ELPA?

> +This works well when using `global-window-tool-bar-mode' to
> +display the mode-specific tool bars attached to each window."

I don't think I understand what you mean by "mode-specific tool bars".
The doc string doesn't explain that, so it is not clear enough, IMO.

> +(defun window-tool-bar-show-memory-use ()
> +  "Pop up a window showing the memory use metrics."

I'm not sure I understand why this package needs to be interested in
memory usage.  It sounds like a separate feature.

Message #13 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Philip Kaludercic <philipk <at> posteo.net>, Jared Finder <jared <at> finder.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, 68765 <at> debbugs.gnu.org
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Sat, 10 Feb 2024 19:25:58 +0200

>> * lisp/tab-line.el (tab-line-format-template): Add separator space.
>> (tab-line-display-order): New user variable to control display order.
>> (tab-line--runtime-display-order, tab-line--cookie): New internal
>> variables.
>> (tab-line-set-display): New function for modes to call to enable only
>> their content.
>> (tab-line-mode): Call `tab-line-set-display'.
>
> Juri, would you please review this part of the changeset?

I'm not sure if this provides a clean interface.
Have you tried to show both window-tool-bar and tab-line tabs
on the same tab-line?  Do you think this combination is usable?
Or maybe better just repurpose the tab-line in window-tool-bar.el
exclusively for window-local tool-bar?

Message #16 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Jared Finder <jared <at> finder.org>
Cc: 68765 <at> debbugs.gnu.org
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Sun, 11 Feb 2024 20:51:44 +0000

Here are a few comments from a quick skim:

> diff --git a/lisp/window-tool-bar.el b/lisp/window-tool-bar.el
> new file mode 100644
> index 00000000000..3950fe12f1a
> --- /dev/null
> +++ b/lisp/window-tool-bar.el
> @@ -0,0 +1,488 @@
> +;;; window-tool-bar.el --- Add tool bars inside windows -*- lexical-binding: t -*-
> +
> +;; Copyright (C) 2023-2024 Free Software Foundation, Inc.

Leave an empty line here, looks more conventional.

> +;; Author: Jared Finder <jared <at> finder.org>
> +;; Created: Nov 21, 2023
> +;; Version: 0.2
> +;; Keywords: mouse
> +;; URL: http://github.com/chaosemer/window-tool-bar

If the idea is for this to be a core package, that is developed in the
core, then I don't know if you want to keep this URL.

> +;; Package-Requires: ((emacs "29.1"))
> +
> +;; This file is part of GNU Emacs.
> +
> +;; GNU Emacs is free software; you can redistribute it and/or modify
> +;; it under the terms of the GNU General Public License as published by
> +;; the Free Software Foundation, either version 3 of the License, or
> +;; (at your option) any later version.
> +
> +;; GNU Emacs is distributed in the hope that it will be useful,
> +;; but WITHOUT ANY WARRANTY; without even the implied warranty of
> +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +;; GNU General Public License for more details.
> +
> +;; You should have received a copy of the GNU General Public License
> +;; along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.
> +
> +;;; Commentary:
> +;;
> +;; This package puts a tool bar in each window.  This allows you to see
> +;; multiple tool bars simultaneously directly next to the buffer it
> +;; acts on which feels much more intuitive.  Emacs "browsing" modes
> +;; generally have sensible tool bars, for example: *info*, *help*, and
> +;; *eww* have them.
> +;;
> +;; It does this while being mindful of screen real estate.  Most modes
> +;; do not provide a custom tool bar, and this package does not show the
> +;; default tool bar.  This means that for most buffers there will be no
> +;; space taken up.  Furthermore, you can put this tool bar in the mode
> +;; line or tab line if you want to share it with existing content.
> +;;
> +;; To get the default behavior, run (global-window-tool-bar-mode 1) or
> +;; enable via M-x customize-group RET window-tool-bar RET.  This uses
> +;; the per-window tab line to show the tool bar.
> +;;
> +;; If you want to share space with an existing tab line, mode line, or
> +;; header line, add (:eval (window-tool-bar-string)) to
> +;; `tab-line-format', `mode-line-format', or `header-line-format'.
> +
> +;;; Known issues:
> +;;
> +;; On GNU Emacs 29.1, terminals dragging to resize windows will error
> +;; with message "<tab-line> <mouse-movement> is undefined".  This is a
> +;; bug in GNU Emacs,
> +;; <https://debbugs.gnu.org/cgi/bugreport.cgi?bug=67457>.
> +;;
> +;; On GNU Emacs 29, performance in terminals is lower than on
> +;; graphical frames.  This is due to a workaround, see "Workaround for
> +;; https://debbugs.gnu.org/cgi/bugreport.cgi?bug=68334", below.
> +
> +;;; Todo:
> +;;
> +;; Not all features planned are implemented yet.  Eventually I would
> +;; like to also generally make tool bars better.
> +;;
> +;; Targeting 0.3:
> +;; * Properly support reamining less frequently used tool bar item specs.  From
> +;;   `parse_tool_bar_item':
> +;;     * :visible
> +;;     * :filter
> +;;     * :button
> +;;     * :wrap
> +;; * Add display customization similar to `tool-bar-style'.
> +;;
> +;; Targeting 1.0:
> +;;
> +;; * Clean up Emacs tool bars
> +;;     * Default: Remove default tool-bar entirely
> +;;     * grep, vc: Remove default tool-bar inherited
> +;;     * info: Remove Next / Prev / Up, which is already in the header
> +;;     * smerge: Add tool bar for next/prev
> +;;
> +;; Post 1.0 work:
> +;;
> +;; * Show keyboard shortcut on help text.
> +;;
> +;; * Add a bit more documentation.
> +;; * Add customization option: ignore-default-tool-bar-map
> +;; * Make tab-line dragging resize the window
> +
> +;;; Code:
> +
> +(require 'mwheel)
> +(require 'tab-line)
> +(require 'tool-bar)
> +
> +;;; Benchmarking code
> +;;
> +;; Refreshing the tool bar is computationally simple, but generates a
> +;; lot of garbage.  So this benchmarking focuses on garbage
> +;; generation.  Since it has to run after most commands, generating
> +;; significantly more garbage will cause noticeable performance
> +;; degration.
> +;;
> +;; The refresh has two steps:
> +;;
> +;; Step 1: Look up the <tool-bar> map.
> +;; Step 2: Generate a Lisp string using text properties for the tool
> +;; bar string.
> +;;
> +;; Additionally, we keep track of the percentage of commands that
> +;; acutally created a refresh.
> +(defvar window-tool-bar--memory-use-delta-step1 (make-list 7 0)
> +  "Absolute delta of memory use counters during step 1.
> +This is a list in the same structure as `memory-use-counts'.")
> +(defvar window-tool-bar--memory-use-delta-step2 (make-list 7 0)
> +  "Absolute delta of memory use counters during step 2.
> +This is a list in the same structure as `memory-use-counts'.")
> +(defvar window-tool-bar--refresh-done-count 0
> +  "Number of tool bar string refreshes run.
> +The total number of requests is the sum of this and
> +`window-tool-bar--refresh-skipped-count'.")
> +(defvar window-tool-bar--refresh-skipped-count 0
> +  "Number of tool bar string refreshes that were skipped.
> +The total number of requests is the sum of this and
> +`window-tool-bar--refresh-done-count'.")
> +
> +(defun window-tool-bar--memory-use-avg-step1 ()
> +  "Return average memory use delta during step 1."
> +  (mapcar (lambda (elt) (/ elt window-tool-bar--refresh-done-count 1.0))

You can also use (float elt) to avoid the 1.0 at the end.

> +          window-tool-bar--memory-use-delta-step1))
> +
> +(defun window-tool-bar--memory-use-avg-step2 ()
> +  "Return average memory use delta during step 2."
> +  (mapcar (lambda (elt) (/ elt window-tool-bar--refresh-done-count 1.0))
> +          window-tool-bar--memory-use-delta-step2))
> +
> +(declare-function time-stamp-string "time-stamp")
> +
> +(defun window-tool-bar-show-memory-use ()
> +  "Pop up a window showing the memory use metrics."
> +  (interactive)
> +  (require 'time-stamp)
> +  (save-selected-window
> +    (pop-to-buffer "*WTB Memory Report*")

I think you should rewrite this as

(with-current-buffer (get-buffer "...")
  ;; ...
  (pop-to-buffer (current-buffer))

> +    (unless (eq major-mode 'special-mode)
> +      (special-mode))
> +
> +    (goto-char (point-max))
> +    (let ((inhibit-read-only t))
> +      (insert (propertize (concat "Function: window-tool-bar-string "
> +                                  (time-stamp-string))
> +                          'face 'underline 'font-lock-face 'underline)
> +              "\n\n")
> +      (window-tool-bar--insert-memory-use
> +       "Step 1" (window-tool-bar--memory-use-avg-step1))
> +      (window-tool-bar--insert-memory-use
> +       "Step 2" (window-tool-bar--memory-use-avg-step2))
> +      (insert (format "Refresh count  %d\n" window-tool-bar--refresh-done-count)
> +              (format "Refresh executed percent %.2f\n"
> +                      (/ window-tool-bar--refresh-done-count
> +                         (+ window-tool-bar--refresh-done-count
> +                            window-tool-bar--refresh-skipped-count)
> +                         1.0))
> +              "\n"))))
> +
> +(defun window-tool-bar--insert-memory-use (label avg-memory-use)
> +  "Insert memory use into current buffer.
> +
> +LABEL: A prefix string to be in front of the data.
> +AVG-MEMORY-USE: A list of averages, with the same meaning as
> +  `memory-use-counts'."
> +  (let* ((label-len (length label))
> +         (padding (make-string label-len ?\s)))
> +    (insert (format "%s  %8.2f Conses\n" label (elt avg-memory-use 0)))
> +    (insert (format "%s  %8.2f Floats\n" padding (elt avg-memory-use 1)))
> +    (insert (format "%s  %8.2f Vector cells\n" padding (elt avg-memory-use 2)))
> +    (insert (format "%s  %8.2f Symbols\n" padding (elt avg-memory-use 3)))
> +    (insert (format "%s  %8.2f String chars\n" padding (elt avg-memory-use 4)))
> +    (insert (format "%s  %8.2f Intervals\n" padding (elt avg-memory-use 5)))
> +    (insert (format "%s  %8.2f Strings\n" padding (elt avg-memory-use 6)))))

You should be able to make this slightly more readable by looping over a
list like '("Conses" "Floats" ...), e.g. using cl-loop

(cl-loop for section = '("Conses" "Floats")
         for usage = avg-memory-use
         do (insert (format "%s  %8.2f %s\n" padding usage section)))
                            
> +
> +(defgroup window-tool-bar nil
> +  "Tool bars per-window."
> +  :group 'convenience
> +  :prefix "window-tool-bar-")
> +
> +(defvar-keymap window-tool-bar--button-keymap
> +  :doc "Keymap used by `window-tool-bar--keymap-entry-to-string'."
> +  "<follow-link>" 'mouse-face
> +  ;; Follow link on all clicks of mouse-1 and mouse-2 since the tool
> +  ;; bar is not a place the point can travel to.
> +  "<tab-line> <mouse-1>" #'window-tool-bar--call-button
> +  "<tab-line> <double-mouse-1>" #'window-tool-bar--call-button
> +  "<tab-line> <triple-mouse-1>" #'window-tool-bar--call-button
> +  "<tab-line> <mouse-2>" #'window-tool-bar--call-button
> +  "<tab-line> <double-mouse-2>" #'window-tool-bar--call-button
> +  "<tab-line> <triple-mouse-2>" #'window-tool-bar--call-button
> +
> +  ;; Mouse down events do nothing.  A binding is needed so isearch
> +  ;; does not exit when the tab bar is clicked.
> +  "<tab-line> <down-mouse-1>" #'window-tool-bar--ignore
> +  "<tab-line> <double-down-mouse-1>" #'window-tool-bar--ignore
> +  "<tab-line> <triple-down-mouse-1>" #'window-tool-bar--ignore
> +  "<tab-line> <down-mouse-2>" #'window-tool-bar--ignore
> +  "<tab-line> <double-down-mouse-2>" #'window-tool-bar--ignore
> +  "<tab-line> <triple-down-mouse-2>" #'window-tool-bar--ignore)
> +(fset 'window-tool-bar--button-keymap window-tool-bar--button-keymap) ; So it can be a keymap property
> +
> +;; Register bindings that stay in isearch.  Technically, these
> +;; commands don't pop up a menu but they act very similar in that they
> +;; end up calling an actual command via `call-interactively'.
> +(push 'window-tool-bar--call-button isearch-menu-bar-commands)
> +(push 'window-tool-bar--ignore isearch-menu-bar-commands)
> +
> +(defvar-local window-tool-bar-string--cache nil
> +  "Cache for previous result of `window-tool-bar-string'.")
> +
> +;;;###autoload
> +(defun window-tool-bar-string ()
> +  "Return a (propertized) string for the tool bar.
> +
> +This is for when you want more customizations than
> +`window-tool-bar-mode' provides.  Commonly added to the variable
> +`tab-line-format', `header-line-format', or `mode-line-format'"
> +  (if (or (null window-tool-bar-string--cache)
> +          (window-tool-bar--last-command-triggers-refresh-p))
> +      (let* ((mem0 (memory-use-counts))
> +             (toolbar-menu (window-tool-bar--get-keymap))
> +             (mem1 (memory-use-counts))
> +             (result (mapconcat #'window-tool-bar--keymap-entry-to-string
> +                                (cdr toolbar-menu) ;Skip 'keymap
> +                                ;; Without spaces between the text, hovering
> +                                ;; highlights all adjacent buttons.
> +                                (if (window-tool-bar--use-images)
> +                                    (propertize " " 'invisible t)
> +                                  " ")))
> +             (mem2 (memory-use-counts)))
> +        (cl-mapl (lambda (l-init l0 l1)
> +                   (cl-incf (car l-init) (- (car l1) (car l0))))
> +                 window-tool-bar--memory-use-delta-step1 mem0 mem1)
> +        (cl-mapl (lambda (l-init l1 l2)
> +                   (cl-incf (car l-init) (- (car l2) (car l1))))
> +                 window-tool-bar--memory-use-delta-step2 mem1 mem2)
> +
> +        (setf window-tool-bar-string--cache
> +              (concat
> +               ;; The tool bar face by default puts boxes around the
> +               ;; buttons.  However, this box is not displayed if the
> +               ;; box starts at the leftmost pixel of the tab-line.
> +               ;; Add a single space in this case so the box displays
> +               ;; correctly.
> +               (when (display-supports-face-attributes-p
> +                      '(:box (line-width 1)))
> +                 (propertize " " 'display '(space :width (1))))
> +               result))
> +        (cl-incf window-tool-bar--refresh-done-count))
> +    (cl-incf window-tool-bar--refresh-skipped-count))
> +
> +  window-tool-bar-string--cache)
> +
> +(defconst window-tool-bar--graphical-separator
> +  (let ((str (make-string 3 ?\s)))
> +    (set-text-properties 0 1 '(display (space :width (4))) str)
> +    (set-text-properties 1 2
> +                         '(display (space :width (1))
> +                           face (:inverse-video t))
> +                         str)
> +    (set-text-properties 2 3 '(display (space :width (4))) str)
> +    str))
> +
> +(defun window-tool-bar--keymap-entry-to-string (menu-item)
> +  "Convert MENU-ITEM into a (propertized) string representation.
> +
> +MENU-ITEM: Menu item to convert.  See info node (elisp)Tool Bar."
                                                   ^
                                                   quote this in `...'
                                                   for the hyperlink to work.
> +  (pcase menu-item
> +    ;; Separators
> +    ((or `(,_ "--")
> +         `(,_ menu-item ,(and (pred stringp)
> +                              (pred (string-prefix-p "--")))))
> +     (if (window-tool-bar--use-images)
> +         window-tool-bar--graphical-separator
> +       "|"))
> +
> +    ;; Menu item, turn into propertized string button
> +    (`(,key menu-item ,name-expr ,binding . ,plist)
> +     (when binding      ; If no binding exists, then button is hidden.
> +       (let* ((name (eval name-expr))
> +              (str (upcase-initials (or (plist-get plist :label)
> +                                        (string-trim-right name "\\.+"))))
> +              (len (length str))
> +              (enable-form (plist-get plist :enable))
> +              (enabled (or (not enable-form)
> +                           (eval enable-form))))
> +         (if enabled
> +             (add-text-properties 0 len
> +                                  '(mouse-face window-tool-bar-button-hover
> +                                    keymap window-tool-bar--button-keymap
> +				    face window-tool-bar-button)
> +                                  str)
> +           (put-text-property 0 len
> +                              'face
> +                              'window-tool-bar-button-disabled
> +                              str))
> +         (when-let ((spec (and (window-tool-bar--use-images)
> +                               (plist-get menu-item :image))))
> +           (put-text-property 0 len
> +                              'display
> +                              (append spec
> +                                      (if enabled '(:margin 2 :ascent center)
> +                                        '(:margin 2 :ascent center
> +                                          :conversion disabled)))
> +                              str))
> +         (put-text-property 0 len
> +                            'help-echo
> +                            (or (plist-get plist :help) name)
> +                            str)
> +         (put-text-property 0 len 'tool-bar-key key str)
> +         str)))))
> +
> +(defun window-tool-bar--call-button ()
> +  "Call the button that was clicked on in the tab line."
> +  (interactive)
> +  (when (mouse-event-p last-command-event)
> +    (let ((posn (event-start last-command-event)))
> +      ;; Commands need to execute with the right buffer and window
> +      ;; selected.  The selection needs to be permanent for isearch.
> +      (select-window (posn-window posn))
> +      (let* ((str (posn-string posn))
> +             (key (get-text-property (cdr str) 'tool-bar-key (car str)))
> +             (cmd (lookup-key (window-tool-bar--get-keymap) (vector key))))
> +        (call-interactively cmd)))))
> +
> +(defun window-tool-bar--ignore ()
> +  "Do nothing.  This command exists for isearch."
> +  (interactive)
> +  nil)
> +
> +(defvar window-tool-bar--ignored-event-types
> +  (let ((list (list 'mouse-movement 'pinch
> +                    'wheel-down 'wheel-up 'wheel-left 'wheel-right
> +                    mouse-wheel-down-event mouse-wheel-up-event
> +                    mouse-wheel-left-event mouse-wheel-right-event
> +                    (bound-and-true-p mouse-wheel-down-alternate-event)
> +                    (bound-and-true-p mouse-wheel-up-alternate-event)
> +                    (bound-and-true-p mouse-wheel-left-alternate-event)
> +                    (bound-and-true-p mouse-wheel-right-alternate-event))))
> +    (delete-dups (delete nil list)))
> +  "Cache for `window-tool-bar--last-command-triggers-refresh-p'.")
> +
> +(defun window-tool-bar--last-command-triggers-refresh-p ()
> +  "Test if the recent command or event should trigger a tool bar refresh."
> +  (let ((type (event-basic-type last-command-event)))
> +    (and
> +     ;; Assume that key presses and button presses are the only user
> +     ;; interactions that can alter the tool bar.  Specifically, this
> +     ;; excludes mouse movement, mouse wheel scroll, and pinch.
> +     (not (member type window-tool-bar--ignored-event-types))
> +     ;; Assume that any command that triggers shift select can't alter
> +     ;; the tool bar.  This excludes pure navigation commands.
> +     (not (window-tool-bar--command-triggers-shift-select-p last-command))
> +     ;; Assume that self-insert-command won't alter the tool bar.
> +     ;; This is the most commonly executed command.
> +     (not (eq last-command 'self-insert-command)))))
> +
> +(defun window-tool-bar--command-triggers-shift-select-p (command)
> +  "Test if COMMAND would trigger shift select."
> +  (let* ((form (interactive-form command))
> +         (spec (car-safe (cdr-safe form))))
> +    (and (eq (car-safe form) 'interactive)
> +         (stringp spec)
> +         (seq-position spec ?^))))
> +
> +;;;###autoload
> +(define-minor-mode window-tool-bar-mode
> +  "Toggle display of the tool bar in the tab line of the current buffer."
> +  :lighter nil
> +  (let ((should-display (and window-tool-bar-mode
> +                             (not (eq tool-bar-map
> +                                      (default-value 'tool-bar-map))))))
> +    (if (fboundp 'tab-line-set-display)
> +        ;; Newly added function for Emacs 30.
> +        (tab-line-set-display 'window-tool-bar-mode
> +			      (and should-display
> +				   '(:eval (window-tool-bar-string))))
> +      ;; Legacy path for Emacs 29.
> +      (setq tab-line-format
> +            (and should-display
> +                 '(:eval (window-tool-bar-string)))))))
> +
> +;;;###autoload
> +(define-globalized-minor-mode global-window-tool-bar-mode
> +  window-tool-bar-mode window-tool-bar--turn-on
> +  :group 'window-tool-bar
> +  (add-hook 'isearch-mode-hook #'window-tool-bar--turn-on)
> +  (add-hook 'isearch-mode-end-hook #'window-tool-bar--turn-on))
> +
> +(defvar window-tool-bar--allow-images t
> +  "Internal debug flag to force text mode.")
> +
> +(defun window-tool-bar--use-images ()
> +  "Internal function.
> +Respects `window-tool-bar--allow-images' as well as frame
> +capabilities."
> +  (and window-tool-bar--allow-images
> +       (display-images-p)))
> +
> +;;; Display styling:
> +(defface window-tool-bar-button
> +  '((default
> +     :inherit tab-line)
> +    (((class color) (min-colors 88) (supports :box t))
> +     :box (:line-width -1 :style released-button)
> +     :background "grey85")
> +    ;; If the box is not supported, dim the button background a bit.
> +    (((class color) (min-colors 88))
> +     :background "grey70")
> +    (t
> +     :inverse-video t))
> +  "Face used for buttons when the mouse is not hovering over the button."
> +  :group 'window-tool-bar)
> +
> +(defface window-tool-bar-button-hover
> +  '((default
> +     :inherit tab-line)
> +    (((class color) (min-colors 88))
> +     :box (:line-width -1 :style released-button)
> +     :background "grey95")
> +    (t
> +     :inverse-video t))
> +  "Face used for buttons when the mouse is hovering over the button."
> +  :group 'window-tool-bar)
> +
> +(defface window-tool-bar-button-disabled
> +  '((default
> +     :inherit tab-line)
> +    (((class color) (min-colors 88))
> +     :box (:line-width -1 :style released-button)
> +     :background "grey50"
> +     :foreground "grey70")
> +    (t
> +     :inverse-video t
> +     :background "brightblack"))
> +  "Face used for buttons when the button is disabled."
> +  :group 'window-tool-bar)
> +
> +;;; Workaround for https://debbugs.gnu.org/cgi/bugreport.cgi?bug=68334.
> +(defun window-tool-bar--get-keymap ()
> +  "Return the tool bar keymap."
> +  (let ((tool-bar-always-show-default nil))
> +    (if (and (version< emacs-version "30")
> +             (not (window-tool-bar--use-images)))
> +        ;; This code path is a less efficient workaround.
> +        (window-tool-bar--make-keymap-1)
> +      (keymap-global-lookup "<tool-bar>"))))
> +
> +(declare-function image-mask-p "image.c" (spec &optional frame))
> +
> +(defun window-tool-bar--make-keymap-1 ()
> +  "Patched copy of `tool-bar-make-keymap-1'."
> +  (mapcar (lambda (bind)
> +            (let (image-exp plist)
> +              (when (and (eq (car-safe (cdr-safe bind)) 'menu-item)
> +			 ;; For the format of menu-items, see node
> +			 ;; `Extended Menu Items' in the Elisp manual.
> +			 (setq plist (nthcdr (if (consp (nth 4 bind)) 5 4)
> +					     bind))
> +			 (setq image-exp (plist-get plist :image))
> +			 (consp image-exp)
> +			 (not (eq (car image-exp) 'image))
> +			 (fboundp (car image-exp)))
> +		(let ((image (and (display-images-p)
> +                                  (eval image-exp))))
> +		  (unless (and image (image-mask-p image))
> +		    (setq image (append image '(:mask heuristic))))
> +		  (setq bind (copy-sequence bind)
> +			plist (nthcdr (if (consp (nth 4 bind)) 5 4)
> +				      bind))
> +		  (plist-put plist :image image)))
> +	      bind))
> +          tool-bar-map))
> +
> +(defun window-tool-bar--turn-on ()
> +  "Internal function called by `global-window-tool-bar-mode'."
> +  (when global-window-tool-bar-mode
> +    (window-tool-bar-mode 1)))
> +
> +(provide 'window-tool-bar)
> +
> +;;; window-tool-bar.el ends here

Message #19 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Jared Finder <jared <at> finder.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68765 <at> debbugs.gnu.org,
 Philip Kaludercic <philipk <at> posteo.net>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Mon, 26 Feb 2024 14:38:39 -0800

On 2024-02-10 09:25, Juri Linkov wrote:
>>> * lisp/tab-line.el (tab-line-format-template): Add separator space.
>>> (tab-line-display-order): New user variable to control display order.
>>> (tab-line--runtime-display-order, tab-line--cookie): New internal
>>> variables.
>>> (tab-line-set-display): New function for modes to call to enable only
>>> their content.
>>> (tab-line-mode): Call `tab-line-set-display'.
>> 
>> Juri, would you please review this part of the changeset?
> 
> I'm not sure if this provides a clean interface.
> Have you tried to show both window-tool-bar and tab-line tabs
> on the same tab-line?  Do you think this combination is usable?
> Or maybe better just repurpose the tab-line in window-tool-bar.el
> exclusively for window-local tool-bar?

The implementation I provided works fine for me with both 
window-tool-bar-mode and tab-line-mode enabled as long as there are not 
too many tabs.  I could see further adding a way to specify the max 
width tabs are able to take up if you think that's necessary.

  -- MJF

Message #22 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Jared Finder <jared <at> finder.org>
To: Philip Kaludercic <philipk <at> posteo.net>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 68765 <at> debbugs.gnu.org
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Mon, 26 Feb 2024 19:02:06 -0800

[Message part 1 (text/plain, inline)]

On 2024-02-11 12:51, Philip Kaludercic wrote:
> Here are a few comments from a quick skim:

Comments addressed.  New patches for 0002 and 0003 added.  I also 
addressed Eli's comments from 
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=68765#10 as well.

The following comment was not addressed:

>> +(defun window-tool-bar-show-memory-use ()
>> +  "Pop up a window showing the memory use metrics."
>> +  (interactive)
>> +  (require 'time-stamp)
>> +  (save-selected-window
>> +    (pop-to-buffer "*WTB Memory Report*")
> 
> I think you should rewrite this as
> 
> (with-current-buffer (get-buffer "...")
>   ;; ...
>   (pop-to-buffer (current-buffer))

I couldn't make this change and keep the current behavior that is 
important to me:

1. The window with focus should not change.
2. The buffer should get scrolled to the bottom to displayed the newly 
inserted text.

  -- MJF

[0002-Add-user-option-to-only-display-default-tool-bar.patch (text/x-diff, attachment)]

[0003-Adding-window-tool-bar-package.patch (text/x-diff, attachment)]

Message #25 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Jared Finder <jared <at> finder.org>
To: Philip Kaludercic <philipk <at> posteo.net>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 68765 <at> debbugs.gnu.org
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Tue, 26 Mar 2024 08:33:22 -0700

It's been four weeks and I've seen no reply to these updated patches.  
Are you able to review?

On 2024-02-26 19:02, Jared Finder wrote:
> On 2024-02-11 12:51, Philip Kaludercic wrote:
>> Here are a few comments from a quick skim:
> 
> Comments addressed.  New patches for 0002 and 0003 added.  I also 
> addressed Eli's comments from 
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=68765#10 as well.
> 
> The following comment was not addressed:
> 
>>> +(defun window-tool-bar-show-memory-use ()
>>> +  "Pop up a window showing the memory use metrics."
>>> +  (interactive)
>>> +  (require 'time-stamp)
>>> +  (save-selected-window
>>> +    (pop-to-buffer "*WTB Memory Report*")
>> 
>> I think you should rewrite this as
>> 
>> (with-current-buffer (get-buffer "...")
>>   ;; ...
>>   (pop-to-buffer (current-buffer))
> 
> I couldn't make this change and keep the current behavior that is 
> important to me:
> 
> 1. The window with focus should not change.
> 2. The buffer should get scrolled to the bottom to displayed the newly 
> inserted text.
> 
>   -- MJF

Message #28 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Jared Finder <jared <at> finder.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68765 <at> debbugs.gnu.org,
 Philip Kaludercic <philipk <at> posteo.net>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Tue, 26 Mar 2024 08:34:11 -0700

It's been four weeks and I've seen no reply to this comment.  Are there 
any other concerns?

  -- MJF

On 2024-02-26 14:38, Jared Finder wrote:
> On 2024-02-10 09:25, Juri Linkov wrote:
>>>> * lisp/tab-line.el (tab-line-format-template): Add separator space.
>>>> (tab-line-display-order): New user variable to control display 
>>>> order.
>>>> (tab-line--runtime-display-order, tab-line--cookie): New internal
>>>> variables.
>>>> (tab-line-set-display): New function for modes to call to enable 
>>>> only
>>>> their content.
>>>> (tab-line-mode): Call `tab-line-set-display'.
>>> 
>>> Juri, would you please review this part of the changeset?
>> 
>> I'm not sure if this provides a clean interface.
>> Have you tried to show both window-tool-bar and tab-line tabs
>> on the same tab-line?  Do you think this combination is usable?
>> Or maybe better just repurpose the tab-line in window-tool-bar.el
>> exclusively for window-local tool-bar?
> 
> The implementation I provided works fine for me with both 
> window-tool-bar-mode and tab-line-mode enabled as long as there are not 
> too many tabs.  I could see further adding a way to specify the max 
> width tabs are able to take up if you think that's necessary.
> 
>   -- MJF

Message #31 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Jared Finder <jared <at> finder.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68765 <at> debbugs.gnu.org,
 Philip Kaludercic <philipk <at> posteo.net>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Wed, 27 Mar 2024 09:04:30 +0200

>>> I'm not sure if this provides a clean interface.
>>> Have you tried to show both window-tool-bar and tab-line tabs
>>> on the same tab-line?  Do you think this combination is usable?
>>> Or maybe better just repurpose the tab-line in window-tool-bar.el
>>> exclusively for window-local tool-bar?
>> The implementation I provided works fine for me with both
>> window-tool-bar-mode and tab-line-mode enabled as long as there are not
>> too many tabs.  I could see further adding a way to specify the max width
>> tabs are able to take up if you think that's necessary.
>
> It's been four weeks and I've seen no reply to this comment.  Are there any
> other concerns?

Sorry, I didn't know that you expected that I should review your patch -
I was silent because your last patch has no changes in tab-line.el,
so I have no more objections in this regard.

As for combining of tabs and the tool-bar on the tab-line,
I don't think this is feasible to achieve a consistent solution.
So users have to choose only one feature at a time:
whether buffer tabs in the tab-line or the window-local tool-bar.

Message #34 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jared Finder <jared <at> finder.org>
Cc: philipk <at> posteo.net, 68765 <at> debbugs.gnu.org
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Sat, 13 Apr 2024 10:43:41 +0300

Ping!  How should we make progress with this issue?

> Date: Tue, 26 Mar 2024 08:33:22 -0700
> From: Jared Finder <jared <at> finder.org>
> Cc: 68765 <at> debbugs.gnu.org
> 
> It's been four weeks and I've seen no reply to these updated patches.  
> Are you able to review?
> 
> On 2024-02-26 19:02, Jared Finder wrote:
> > On 2024-02-11 12:51, Philip Kaludercic wrote:
> >> Here are a few comments from a quick skim:
> > 
> > Comments addressed.  New patches for 0002 and 0003 added.  I also 
> > addressed Eli's comments from 
> > https://debbugs.gnu.org/cgi/bugreport.cgi?bug=68765#10 as well.
> > 
> > The following comment was not addressed:
> > 
> >>> +(defun window-tool-bar-show-memory-use ()
> >>> +  "Pop up a window showing the memory use metrics."
> >>> +  (interactive)
> >>> +  (require 'time-stamp)
> >>> +  (save-selected-window
> >>> +    (pop-to-buffer "*WTB Memory Report*")
> >> 
> >> I think you should rewrite this as
> >> 
> >> (with-current-buffer (get-buffer "...")
> >>   ;; ...
> >>   (pop-to-buffer (current-buffer))
> > 
> > I couldn't make this change and keep the current behavior that is 
> > important to me:
> > 
> > 1. The window with focus should not change.
> > 2. The buffer should get scrolled to the bottom to displayed the newly 
> > inserted text.
> > 
> >   -- MJF
> 

Message #37 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: philipk <at> posteo.net, jared <at> finder.org, 68765 <at> debbugs.gnu.org
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Sat, 27 Apr 2024 11:25:59 +0300

Ping! Ping! Philip and Jared, are there any issues left to resolve
here, or should this be installed?

> Cc: philipk <at> posteo.net, 68765 <at> debbugs.gnu.org
> Date: Sat, 13 Apr 2024 10:43:41 +0300
> From: Eli Zaretskii <eliz <at> gnu.org>
> 
> Ping!  How should we make progress with this issue?
> 
> > Date: Tue, 26 Mar 2024 08:33:22 -0700
> > From: Jared Finder <jared <at> finder.org>
> > Cc: 68765 <at> debbugs.gnu.org
> > 
> > It's been four weeks and I've seen no reply to these updated patches.  
> > Are you able to review?
> > 
> > On 2024-02-26 19:02, Jared Finder wrote:
> > > On 2024-02-11 12:51, Philip Kaludercic wrote:
> > >> Here are a few comments from a quick skim:
> > > 
> > > Comments addressed.  New patches for 0002 and 0003 added.  I also 
> > > addressed Eli's comments from 
> > > https://debbugs.gnu.org/cgi/bugreport.cgi?bug=68765#10 as well.
> > > 
> > > The following comment was not addressed:
> > > 
> > >>> +(defun window-tool-bar-show-memory-use ()
> > >>> +  "Pop up a window showing the memory use metrics."
> > >>> +  (interactive)
> > >>> +  (require 'time-stamp)
> > >>> +  (save-selected-window
> > >>> +    (pop-to-buffer "*WTB Memory Report*")
> > >> 
> > >> I think you should rewrite this as
> > >> 
> > >> (with-current-buffer (get-buffer "...")
> > >>   ;; ...
> > >>   (pop-to-buffer (current-buffer))
> > > 
> > > I couldn't make this change and keep the current behavior that is 
> > > important to me:
> > > 
> > > 1. The window with focus should not change.
> > > 2. The buffer should get scrolled to the bottom to displayed the newly 
> > > inserted text.
> > > 
> > >   -- MJF
> > 
> 
> 
> 
> 

Message #40 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Jared Finder <jared <at> finder.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68765 <at> debbugs.gnu.org
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Sat, 27 Apr 2024 10:00:21 +0000

Eli Zaretskii <eliz <at> gnu.org> writes:

> Ping! Ping! Philip and Jared, are there any issues left to resolve
> here, or should this be installed?

Sorry for the delay, I'm going to comment below.

[...]


Jared Finder <jared <at> finder.org> writes:

> It's been four weeks and I've seen no reply to these updated patches.
> Are you able to review?

Likewise, my apologies, the messages got lost in my backlog that I am
currently trying to make up.

Jared Finder <jared <at> finder.org> writes:
> Comments addressed.  New patches for 0002 and 0003 added.  I also
> addressed Eli's comments from
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=68765#10 as well.
>
> The following comment was not addressed:
>
>>> +(defun window-tool-bar-show-memory-use ()
>>> +  "Pop up a window showing the memory use metrics."
>>> +  (interactive)
>>> +  (require 'time-stamp)
>>> +  (save-selected-window
>>> +    (pop-to-buffer "*WTB Memory Report*")
>> I think you should rewrite this as
>> (with-current-buffer (get-buffer "...")
>>   ;; ...
>>   (pop-to-buffer (current-buffer))
>
> I couldn't make this change and keep the current behavior that is
> important to me:
>
> 1. The window with focus should not change.
> 2. The buffer should get scrolled to the bottom to displayed the newly
> inserted text.

Ok.

>   -- MJF
>
> From 622c11c6f314355b0e742fcbcbcc8ae51661bca0 Mon Sep 17 00:00:00 2001
> From: Jared Finder <jared <at> finder.org>
> Date: Fri, 26 Jan 2024 10:08:30 -0800
> Subject: [PATCH 2/3] Add user option to only display default tool bar
>
> This works well with `window-tool-bar-mode', to be added in upcoming
> commit.  Then the default tool bar is displayed frame-wide and
> mode-specific tool bars are displayed in the window that mode is
> active in.
>
> * lisp/tool-bar.el (tool-bar-always-show-default): New user option.
> (tool-bar--cache-key, tool-bar-make-keymap-1): Return default tool bar
> when option is set.
> ---
>  lisp/tool-bar.el | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/lisp/tool-bar.el b/lisp/tool-bar.el
> index 96b61c7b229..52d60b32412 100644
> --- a/lisp/tool-bar.el
> +++ b/lisp/tool-bar.el
> @@ -100,7 +100,9 @@ secondary-tool-bar-map
>  (defconst tool-bar-keymap-cache (make-hash-table :test #'equal))
>  
>  (defsubst tool-bar--cache-key ()
> -  (cons (frame-terminal) (sxhash-eq tool-bar-map)))
> +  (cons (frame-terminal)
> +        (sxhash-eq (if tool-bar-always-show-default (default-value 'tool-bar-map)
> +                     tool-bar-map))))
>  
>  (defsubst tool-bar--secondary-cache-key ()
>    (cons (frame-terminal) (sxhash-eq secondary-tool-bar-map)))
> @@ -191,7 +193,9 @@ tool-bar-make-keymap-1
>  				      bind))
>  		  (plist-put plist :image image)))
>  	      bind))
> -	  (or map tool-bar-map)))
> +	  (or map
> +              (if tool-bar-always-show-default (default-value 'tool-bar-map)
> +                tool-bar-map))))
>  
>  ;;;###autoload
>  (defun tool-bar-add-item (icon def key &rest props)
> @@ -377,6 +381,15 @@ tool-bar-setup
>  	     (modify-all-frames-parameters
>  	      (list (cons 'tool-bar-position val))))))
>  
> +(defcustom tool-bar-always-show-default nil
> +  "If non-nil, `tool-bar-mode' only shows the default tool bar.
> +This works well when also using `global-window-tool-bar-mode' to
> +display buffer-specific tool bars."
> +  :type 'boolean
> +  :group 'frames
> +  :group 'mouse
> +  :version "30.1")
> +
>  

No comments from me here.

>  ;; Modifier bar mode.
> -- 
> 2.39.2
>
>
> From baf4c81df3e4e82576a8084ae029d56b45750553 Mon Sep 17 00:00:00 2001
> From: Jared Finder <jared <at> finder.org>
> Date: Fri, 26 Jan 2024 15:44:12 -0800
> Subject: [PATCH 3/3] Adding window-tool-bar package
>
> ---
>  lisp/window-tool-bar.el | 489 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 489 insertions(+)
>  create mode 100644 lisp/window-tool-bar.el
>
> diff --git a/lisp/window-tool-bar.el b/lisp/window-tool-bar.el
> new file mode 100644
> index 00000000000..eefd6109f7d
> --- /dev/null
> +++ b/lisp/window-tool-bar.el
> @@ -0,0 +1,489 @@
> +;;; window-tool-bar.el --- Add tool bars inside windows -*- lexical-binding: t -*-
> +
> +;; Copyright (C) 2023-2024 Free Software Foundation, Inc.
> +
> +;; Author: Jared Finder <jared <at> finder.org>
> +;; Created: Nov 21, 2023
> +;; Version: 0.2
> +;; Keywords: mouse
> +;; Package-Requires: ((emacs "29.1"))

If the plan is for this to be a core-package, then you should add a
comment like

;; This is a GNU ELPA :core package.  Avoid adding functionality
;; that is not available in the version of Emacs recorded above or any
;; of the package dependencies.

> +
> +;; This file is part of GNU Emacs.
> +
> +;; GNU Emacs is free software; you can redistribute it and/or modify
> +;; it under the terms of the GNU General Public License as published by
> +;; the Free Software Foundation, either version 3 of the License, or
> +;; (at your option) any later version.
> +
> +;; GNU Emacs is distributed in the hope that it will be useful,
> +;; but WITHOUT ANY WARRANTY; without even the implied warranty of
> +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +;; GNU General Public License for more details.
> +
> +;; You should have received a copy of the GNU General Public License
> +;; along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.
> +
> +;;; Commentary:
> +;;
> +;; This package puts a tool bar in each window.  This allows you to see
> +;; multiple tool bars simultaneously directly next to the buffer it
> +;; acts on which feels much more intuitive.  Emacs "browsing" modes
> +;; generally have sensible tool bars, for example: *info*, *help*, and
> +;; *eww* have them.
> +;;
> +;; It does this while being mindful of screen real estate.  Most modes
> +;; do not provide a custom tool bar, and this package does not show the
> +;; default tool bar.  This means that for most buffers there will be no
> +;; space taken up.  Furthermore, you can put this tool bar in the mode
> +;; line or tab line if you want to share it with existing content.
> +;;
> +;; To get the default behavior, run (global-window-tool-bar-mode 1) or
> +;; enable via M-x customize-group RET window-tool-bar RET.  This uses
> +;; the per-window tab line to show the tool bar.
> +;;
> +;; If you want to share space with an existing tab line, mode line, or
> +;; header line, add (:eval (window-tool-bar-string)) to
> +;; `tab-line-format', `mode-line-format', or `header-line-format'.
> +
> +;;; Known issues:
> +;;
> +;; On GNU Emacs 29.1, terminals dragging to resize windows will error
> +;; with message "<tab-line> <mouse-movement> is undefined".  This is a
> +;; bug in GNU Emacs,
> +;; <https://debbugs.gnu.org/cgi/bugreport.cgi?bug=67457>.
> +;;
> +;; On GNU Emacs 29, performance in terminals is lower than on
> +;; graphical frames.  This is due to a workaround, see "Workaround for
> +;; https://debbugs.gnu.org/cgi/bugreport.cgi?bug=68334", below.
> +
> +;;; Todo:
> +;;
> +;; Not all features planned are implemented yet.  Eventually I would
> +;; like to also generally make tool bars better.
> +;;
> +;; Targeting 0.3:
> +;; * Properly support reamining less frequently used tool bar item specs.  From
> +;;   `parse_tool_bar_item':
> +;;     * :visible
> +;;     * :filter
> +;;     * :button
> +;;     * :wrap
> +;; * Add display customization similar to `tool-bar-style'.
> +;;
> +;; Targeting 1.0:
> +;;
> +;; * Clean up Emacs tool bars
> +;;     * Default: Remove default tool-bar entirely
> +;;     * grep, vc: Remove default tool-bar inherited
> +;;     * info: Remove Next / Prev / Up, which is already in the header
> +;;     * smerge: Add tool bar for next/prev
> +;;
> +;; Post 1.0 work:
> +;;
> +;; * Show keyboard shortcut on help text.
> +;;
> +;; * Add a bit more documentation.
> +;; * Add customization option: ignore-default-tool-bar-map
> +;; * Make tab-line dragging resize the window
> +
> +;;; Code:
> +
> +(require 'mwheel)
> +(require 'tab-line)
> +(require 'tool-bar)
> +
> +;;; Benchmarking code
> +;;
> +;; Refreshing the tool bar is computationally simple, but generates a
> +;; lot of garbage.  So this benchmarking focuses on garbage
> +;; generation.  Since it has to run after most commands, generating
> +;; significantly more garbage will cause noticeable performance
> +;; degration.
> +;;
> +;; The refresh has two steps:
> +;;
> +;; Step 1: Look up the <tool-bar> map.
> +;; Step 2: Generate a Lisp string using text properties for the tool
> +;; bar string.
> +;;
> +;; Additionally, we keep track of the percentage of commands that
> +;; acutally created a refresh.
> +(defvar window-tool-bar--memory-use-delta-step1 (make-list 7 0)
> +  "Absolute delta of memory use counters during step 1.
> +This is a list in the same structure as `memory-use-counts'.")
> +(defvar window-tool-bar--memory-use-delta-step2 (make-list 7 0)
> +  "Absolute delta of memory use counters during step 2.
> +This is a list in the same structure as `memory-use-counts'.")
> +(defvar window-tool-bar--refresh-done-count 0
> +  "Number of tool bar string refreshes run.
> +The total number of requests is the sum of this and
> +`window-tool-bar--refresh-skipped-count'.")
> +(defvar window-tool-bar--refresh-skipped-count 0
> +  "Number of tool bar string refreshes that were skipped.
> +The total number of requests is the sum of this and
> +`window-tool-bar--refresh-done-count'.")
> +
> +(defun window-tool-bar--memory-use-avg-step1 ()
> +  "Return average memory use delta during step 1."
> +  (mapcar (lambda (elt) (/ elt window-tool-bar--refresh-done-count 1.0))
> +          window-tool-bar--memory-use-delta-step1))
> +
> +(defun window-tool-bar--memory-use-avg-step2 ()
> +  "Return average memory use delta during step 2."
> +  (mapcar (lambda (elt) (/ (float elt) window-tool-bar--refresh-done-count))
> +          window-tool-bar--memory-use-delta-step2))
> +
> +(declare-function time-stamp-string "time-stamp")
> +
> +(defun window-tool-bar-debug-show-memory-use ()
> +  "Development-only command to show memory used by `window-tool-bar-string'."
> +  (interactive)
> +  (require 'time-stamp)
> +  (save-selected-window
> +    (pop-to-buffer "*WTB Memory Report*")
> +    (unless (eq major-mode 'special-mode)

You should explain what is going on here, and why you are checking
major-mode instead of using derived-mode-p.

> +      (special-mode))
> +
> +    (goto-char (point-max))
> +    (let ((inhibit-read-only t))
> +      (insert (propertize (concat "Function: window-tool-bar-string "
> +                                  (time-stamp-string))
> +                          'face 'underline 'font-lock-face 'underline)
> +              "\n\n")
> +      (window-tool-bar--insert-memory-use
> +       "Step 1" (window-tool-bar--memory-use-avg-step1))
> +      (window-tool-bar--insert-memory-use
> +       "Step 2" (window-tool-bar--memory-use-avg-step2))
> +      (insert (format "Refresh count  %d\n" window-tool-bar--refresh-done-count)
> +              (format "Refresh executed percent %.2f\n"
> +                      (/ window-tool-bar--refresh-done-count
> +                         (+ window-tool-bar--refresh-done-count
> +                            window-tool-bar--refresh-skipped-count)
> +                         1.0))

I don't know if there is any significant difference between (/ a b 1.0)
and (/ a (float b)), but interesting they have the same number of
bytecode instructions and funcalls:

(disassemble (byte-compile (lambda (a b) (/ a b 1.0))))

byte code:
  doc:   ...
  args: (arg1 arg2)
0	constant  /
1	stack-ref 2
2	stack-ref 2
3	constant  1.0
4	call	  3
5	return	  



(disassemble (byte-compile (lambda (a b) (/ a (float b)))))

byte code:
  doc:   ...
  args: (arg1 arg2)
0	stack-ref 1
1	constant  float
2	stack-ref 2
3	call	  1
4	quo	  
5	return	  

> +              "\n"))))
> +
> +(defun window-tool-bar--insert-memory-use (label avg-memory-use)
> +  "Insert memory use into current buffer.
> +
> +LABEL: A prefix string to be in front of the data.
> +AVG-MEMORY-USE: A list of averages, with the same meaning as
> +  `memory-use-counts'."

The formatting is somewhat unconventional and can easily be broken using M-q.

> +  (let* ((label-len (length label))
> +         (padding (make-string label-len ?\s)))
> +    (cl-loop for usage in avg-memory-use
> +             for usage-label in '("Conses" "Floats" "Vector cells" "Symbols"
> +                                  "String chars" "Intervals" "Strings")
> +             for idx from 0
> +             do (insert (format "%s  %8.2f %s\n"
> +                                (if (= idx 0) label padding)
> +                                usage
> +                                usage-label)))))
> +
> +(defgroup window-tool-bar nil
> +  "Tool bars per-window."
> +  :group 'convenience
> +  :prefix "window-tool-bar-")
> +
> +(defvar-keymap window-tool-bar--button-keymap
> +  :doc "Keymap used by `window-tool-bar--keymap-entry-to-string'."
> +  "<follow-link>" 'mouse-face
> +  ;; Follow link on all clicks of mouse-1 and mouse-2 since the tool
> +  ;; bar is not a place the point can travel to.
> +  "<tab-line> <mouse-1>" #'window-tool-bar--call-button
> +  "<tab-line> <double-mouse-1>" #'window-tool-bar--call-button
> +  "<tab-line> <triple-mouse-1>" #'window-tool-bar--call-button
> +  "<tab-line> <mouse-2>" #'window-tool-bar--call-button
> +  "<tab-line> <double-mouse-2>" #'window-tool-bar--call-button
> +  "<tab-line> <triple-mouse-2>" #'window-tool-bar--call-button
> +
> +  ;; Mouse down events do nothing.  A binding is needed so isearch
> +  ;; does not exit when the tab bar is clicked.
> +  "<tab-line> <down-mouse-1>" #'window-tool-bar--ignore
> +  "<tab-line> <double-down-mouse-1>" #'window-tool-bar--ignore
> +  "<tab-line> <triple-down-mouse-1>" #'window-tool-bar--ignore
> +  "<tab-line> <down-mouse-2>" #'window-tool-bar--ignore
> +  "<tab-line> <double-down-mouse-2>" #'window-tool-bar--ignore
> +  "<tab-line> <triple-down-mouse-2>" #'window-tool-bar--ignore)
> +(fset 'window-tool-bar--button-keymap window-tool-bar--button-keymap) ; So it can be a keymap property
> +
> +;; Register bindings that stay in isearch.  Technically, these
> +;; commands don't pop up a menu but they act very similar in that they
> +;; end up calling an actual command via `call-interactively'.
> +(push 'window-tool-bar--call-button isearch-menu-bar-commands)
> +(push 'window-tool-bar--ignore isearch-menu-bar-commands)
> +
> +(defvar-local window-tool-bar-string--cache nil
> +  "Cache for previous result of `window-tool-bar-string'.")
> +
> +;;;###autoload
> +(defun window-tool-bar-string ()
> +  "Return a (propertized) string for the tool bar.
> +
> +This is for when you want more customizations than
> +`window-tool-bar-mode' provides.  Commonly added to the variable
> +`tab-line-format', `header-line-format', or `mode-line-format'"
> +  (if (or (null window-tool-bar-string--cache)
> +          (window-tool-bar--last-command-triggers-refresh-p))
> +      (let* ((mem0 (memory-use-counts))
> +             (toolbar-menu (window-tool-bar--get-keymap))
> +             (mem1 (memory-use-counts))
> +             (result (mapconcat #'window-tool-bar--keymap-entry-to-string
> +                                (cdr toolbar-menu) ;Skip 'keymap
> +                                ;; Without spaces between the text, hovering
> +                                ;; highlights all adjacent buttons.
> +                                (if (window-tool-bar--use-images)
> +                                    (propertize " " 'invisible t)
> +                                  " ")))
> +             (mem2 (memory-use-counts)))
> +        (cl-mapl (lambda (l-init l0 l1)
> +                   (cl-incf (car l-init) (- (car l1) (car l0))))
> +                 window-tool-bar--memory-use-delta-step1 mem0 mem1)
> +        (cl-mapl (lambda (l-init l1 l2)
> +                   (cl-incf (car l-init) (- (car l2) (car l1))))
> +                 window-tool-bar--memory-use-delta-step2 mem1 mem2)
> +
> +        (setf window-tool-bar-string--cache
> +              (concat
> +               ;; The tool bar face by default puts boxes around the
> +               ;; buttons.  However, this box is not displayed if the
> +               ;; box starts at the leftmost pixel of the tab-line.
> +               ;; Add a single space in this case so the box displays
> +               ;; correctly.
> +               (when (display-supports-face-attributes-p

I'd use `and' here, instead of `when', since the evaluation result is of
interest.

> +                      '(:box (line-width 1)))
> +                 (propertize " " 'display '(space :width (1))))
> +               result))
> +        (cl-incf window-tool-bar--refresh-done-count))
> +    (cl-incf window-tool-bar--refresh-skipped-count))
> +
> +  window-tool-bar-string--cache)
> +
> +(defconst window-tool-bar--graphical-separator
> +  (let ((str (make-string 3 ?\s)))
> +    (set-text-properties 0 1 '(display (space :width (4))) str)
> +    (set-text-properties 1 2
> +                         '(display (space :width (1))
> +                           face (:inverse-video t))
> +                         str)
> +    (set-text-properties 2 3 '(display (space :width (4))) str)
> +    str))

This should be equivalent to

(concat
 (propertize " " 'display '(space :width (4)))
 (propertize " " 'display '(space :width (1)) 'face '(:inverse-video t))
 (propertize " " 'display '(space :width (4))))

right?

> +
> +(defun window-tool-bar--keymap-entry-to-string (menu-item)
> +  "Convert MENU-ITEM into a (propertized) string representation.
> +
> +MENU-ITEM: Menu item to convert.  See info node (elisp)Tool Bar."
> +  (pcase menu-item

pcase or pcase-exhaustive?

> +    ;; Separators
> +    ((or `(,_ "--")
> +         `(,_ menu-item ,(and (pred stringp)
> +                              (pred (string-prefix-p "--")))))
> +     (if (window-tool-bar--use-images)
> +         window-tool-bar--graphical-separator
> +       "|"))
> +
> +    ;; Menu item, turn into propertized string button
> +    (`(,key menu-item ,name-expr ,binding . ,plist)
> +     (when binding      ; If no binding exists, then button is hidden.
> +       (let* ((name (eval name-expr))
> +              (str (upcase-initials (or (plist-get plist :label)
> +                                        (string-trim-right name "\\.+"))))
> +              (len (length str))
> +              (enable-form (plist-get plist :enable))
> +              (enabled (or (not enable-form)
> +                           (eval enable-form))))
> +         (if enabled
> +             (add-text-properties 0 len
> +                                  '(mouse-face window-tool-bar-button-hover
> +                                    keymap window-tool-bar--button-keymap
> +                                    face window-tool-bar-button)
> +                                  str)
> +           (put-text-property 0 len
> +                              'face
> +                              'window-tool-bar-button-disabled
> +                              str))
> +         (when-let ((spec (and (window-tool-bar--use-images)
> +                               (plist-get menu-item :image))))
> +           (put-text-property 0 len
> +                              'display
> +                              (append spec
> +                                      (if enabled '(:margin 2 :ascent center)
> +                                        '(:margin 2 :ascent center
> +                                          :conversion disabled)))
> +                              str))
> +         (put-text-property 0 len
> +                            'help-echo
> +                            (or (plist-get plist :help) name)
> +                            str)
> +         (put-text-property 0 len 'tool-bar-key key str)
> +         str)))))
> +
> +(defun window-tool-bar--call-button ()
> +  "Call the button that was clicked on in the tab line."
> +  (interactive)
> +  (when (mouse-event-p last-command-event)
> +    (let ((posn (event-start last-command-event)))
> +      ;; Commands need to execute with the right buffer and window
> +      ;; selected.  The selection needs to be permanent for isearch.
> +      (select-window (posn-window posn))
> +      (let* ((str (posn-string posn))
> +             (key (get-text-property (cdr str) 'tool-bar-key (car str)))
> +             (cmd (lookup-key (window-tool-bar--get-keymap) (vector key))))
> +        (call-interactively cmd)))))
> +
> +(defun window-tool-bar--ignore ()
> +  "Do nothing.  This command exists for isearch."

Can you elaborate?  Why not just use the existing ignore?  Or
defaliasing it?

> +  (interactive)
> +  nil)
> +
> +(defvar window-tool-bar--ignored-event-types
> +  (let ((list (list 'mouse-movement 'pinch
> +                    'wheel-down 'wheel-up 'wheel-left 'wheel-right
> +                    mouse-wheel-down-event mouse-wheel-up-event
> +                    mouse-wheel-left-event mouse-wheel-right-event
> +                    (bound-and-true-p mouse-wheel-down-alternate-event)
> +                    (bound-and-true-p mouse-wheel-up-alternate-event)
> +                    (bound-and-true-p mouse-wheel-left-alternate-event)
> +                    (bound-and-true-p mouse-wheel-right-alternate-event))))
> +    (delete-dups (delete nil list)))
> +  "Cache for `window-tool-bar--last-command-triggers-refresh-p'.")
> +
> +(defun window-tool-bar--last-command-triggers-refresh-p ()
> +  "Test if the recent command or event should trigger a tool bar refresh."
> +  (let ((type (event-basic-type last-command-event)))
> +    (and
> +     ;; Assume that key presses and button presses are the only user
> +     ;; interactions that can alter the tool bar.  Specifically, this
> +     ;; excludes mouse movement, mouse wheel scroll, and pinch.
> +     (not (member type window-tool-bar--ignored-event-types))
> +     ;; Assume that any command that triggers shift select can't alter
> +     ;; the tool bar.  This excludes pure navigation commands.
> +     (not (window-tool-bar--command-triggers-shift-select-p last-command))
> +     ;; Assume that self-insert-command won't alter the tool bar.
> +     ;; This is the most commonly executed command.
> +     (not (eq last-command 'self-insert-command)))))
> +
> +(defun window-tool-bar--command-triggers-shift-select-p (command)
> +  "Test if COMMAND would trigger shift select."
> +  (let* ((form (interactive-form command))
> +         (spec (car-safe (cdr-safe form))))
> +    (and (eq (car-safe form) 'interactive)
> +         (stringp spec)
> +         (seq-position spec ?^))))
> +
> +;;;###autoload
> +(define-minor-mode window-tool-bar-mode
> +  "Toggle display of the tool bar in the tab line of the current buffer."
> +  :lighter nil

There is no lighter by default, I prefer writing :global nil, to make it
explicit to the reader that this is a local minor mode.

> +  (let ((should-display (and window-tool-bar-mode
> +                             (not (eq tool-bar-map
> +                                      (default-value 'tool-bar-map))))))
> +    (if (fboundp 'tab-line-set-display)
> +        ;; Newly added function for Emacs 30.
> +        (tab-line-set-display 'window-tool-bar-mode
> +                              (and should-display
> +                                   '(:eval (window-tool-bar-string))))
> +      ;; Legacy path for Emacs 29.
> +      (setq tab-line-format
> +            (and should-display
> +                 '(:eval (window-tool-bar-string)))))))
> +
> +;;;###autoload
> +(define-globalized-minor-mode global-window-tool-bar-mode
> +  window-tool-bar-mode window-tool-bar--turn-on
> +  :group 'window-tool-bar
> +  (add-hook 'isearch-mode-hook #'window-tool-bar--turn-on)
> +  (add-hook 'isearch-mode-end-hook #'window-tool-bar--turn-on))
> +
> +(defvar window-tool-bar--allow-images t
> +  "Internal debug flag to force text mode.")
> +
> +(defun window-tool-bar--use-images ()
> +  "Internal function.
> +Respects `window-tool-bar--allow-images' as well as frame
> +capabilities."
> +  (and window-tool-bar--allow-images
> +       (display-images-p)))
> +
> +;;; Display styling:
> +(defface window-tool-bar-button
> +  '((default
> +     :inherit tab-line)
> +    (((class color) (min-colors 88) (supports :box t))
> +     :box (:line-width -1 :style released-button)
> +     :background "grey85")
> +    ;; If the box is not supported, dim the button background a bit.
> +    (((class color) (min-colors 88))
> +     :background "grey70")
> +    (t
> +     :inverse-video t))
> +  "Face used for buttons when the mouse is not hovering over the button."
> +  :group 'window-tool-bar)
> +
> +(defface window-tool-bar-button-hover
> +  '((default
> +     :inherit tab-line)
> +    (((class color) (min-colors 88))
> +     :box (:line-width -1 :style released-button)
> +     :background "grey95")
> +    (t
> +     :inverse-video t))
> +  "Face used for buttons when the mouse is hovering over the button."
> +  :group 'window-tool-bar)
> +
> +(defface window-tool-bar-button-disabled
> +  '((default
> +     :inherit tab-line)
> +    (((class color) (min-colors 88))
> +     :box (:line-width -1 :style released-button)
> +     :background "grey50"
> +     :foreground "grey70")
> +    (t
> +     :inverse-video t
> +     :background "brightblack"))
> +  "Face used for buttons when the button is disabled."
> +  :group 'window-tool-bar)
> +
> +;;; Workaround for https://debbugs.gnu.org/cgi/bugreport.cgi?bug=68334.
> +(defun window-tool-bar--get-keymap ()
> +  "Return the tool bar keymap."
> +  (let ((tool-bar-always-show-default nil))
> +    (if (and (version< emacs-version "30")
> +             (not (window-tool-bar--use-images)))
> +        ;; This code path is a less efficient workaround.
> +        (window-tool-bar--make-keymap-1)
> +      (keymap-global-lookup "<tool-bar>"))))
> +
> +(declare-function image-mask-p "image.c" (spec &optional frame))
> +
> +(defun window-tool-bar--make-keymap-1 ()
> +  "Patched copy of `tool-bar-make-keymap-1'."
> +  (mapcar (lambda (bind)
> +            (let (image-exp plist)
> +              (when (and (eq (car-safe (cdr-safe bind)) 'menu-item)
> +                         ;; For the format of menu-items, see node
> +                         ;; `Extended Menu Items' in the Elisp manual.
> +                         (setq plist (nthcdr (if (consp (nth 4 bind)) 5 4)
> +                                             bind))
> +                         (setq image-exp (plist-get plist :image))
> +                         (consp image-exp)
> +                         (not (eq (car image-exp) 'image))
> +                         (fboundp (car image-exp)))
> +                (let ((image (and (display-images-p)
> +                                  (eval image-exp))))
> +                  (unless (and image (image-mask-p image))
> +                    (setq image (append image '(:mask heuristic))))
> +                  (setq bind (copy-sequence bind)
> +                        plist (nthcdr (if (consp (nth 4 bind)) 5 4)
> +                                      bind))
> +                  (plist-put plist :image image)))
> +              bind))
> +          tool-bar-map))
> +
> +(defun window-tool-bar--turn-on ()
> +  "Internal function called by `global-window-tool-bar-mode'."
> +  (when global-window-tool-bar-mode
> +    (window-tool-bar-mode 1)))
> +
> +(provide 'window-tool-bar)
> +
> +;;; window-tool-bar.el ends here

Hope this was of use.

-- 
	Philip Kaludercic on peregrine

Message #43 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Jared Finder <jared <at> finder.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68765 <at> debbugs.gnu.org
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Sat, 27 Apr 2024 21:44:32 -0700

[Message part 1 (text/plain, inline)]

On 2024-04-27 03:00, Philip Kaludercic wrote:
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
>> Ping! Ping! Philip and Jared, are there any issues left to resolve
>> here, or should this be installed?
> 
> Sorry for the delay, I'm going to comment below.

I addressed all feedback provided.  Updated patch for just 0003 attached 
since that's where all the feedback was.  Let me know if this looks 
good.

> 
... partial patch elided ...
>> +(defun window-tool-bar-debug-show-memory-use ()
>> +  "Development-only command to show memory used by 
>> `window-tool-bar-string'."
>> +  (interactive)
>> +  (require 'time-stamp)
>> +  (save-selected-window
>> +    (pop-to-buffer "*WTB Memory Report*")
>> +    (unless (eq major-mode 'special-mode)
> 
> You should explain what is going on here, and why you are checking
> major-mode instead of using derived-mode-p.

I changed this to call derived-mode-p.  The intent is to put the buffer 
into special-mode and also avoid unnecessarily calling special-mode.

>> +      (special-mode))
>> +
... partial patch elided ...
>> +      (insert (format "Refresh count  %d\n" 
>> window-tool-bar--refresh-done-count)
>> +              (format "Refresh executed percent %.2f\n"
>> +                      (/ window-tool-bar--refresh-done-count
>> +                         (+ window-tool-bar--refresh-done-count
>> +                            window-tool-bar--refresh-skipped-count)
>> +                         1.0))
> 
> I don't know if there is any significant difference between (/ a b 1.0)
> and (/ a (float b)), but interesting they have the same number of
> bytecode instructions and funcalls:

I changed this and other places where I was dividing by 1.0 to force 
floating point division to instead do (float a) on the first parameter.  
It sounds like that's more idiomatic.

>> +              "\n"))))
>> +
>> +(defun window-tool-bar--insert-memory-use (label avg-memory-use)
>> +  "Insert memory use into current buffer.
>> +
>> +LABEL: A prefix string to be in front of the data.
>> +AVG-MEMORY-USE: A list of averages, with the same meaning as
>> +  `memory-use-counts'."
> 
> The formatting is somewhat unconventional and can easily be broken 
> using M-q.

Addressed here and other places I used this convention.

>> +(defun window-tool-bar--ignore ()
>> +  "Do nothing.  This command exists for isearch."
> 
> Can you elaborate?  Why not just use the existing ignore?  Or
> defaliasing it?

There's a lot of detailed comments around usage of this command, see 
comments around window-tool-bar--button-keymap.  I also added a bit more 
context to the docstring here.  In brief, this command exists so that 
isearch does not exit when you click on isearch tool bar buttons.  This 
is needed for window tool bar buttons since they are called by Emacs' 
usual mouse event bindings, unlike toolkit tool bars.

I can't use ignore because it does not have the special registration 
with isearch, specifically ignore is not a member of 
isearch-menu-bar-commands.  I did not feel safe changing all existing 
uses of ignore.

>> +  (let ((type (event-basic-type last-command-event)))
... partial patch elided ...
>> +;;;###autoload
>> +(define-minor-mode window-tool-bar-mode
>> +  "Toggle display of the tool bar in the tab line of the current 
>> buffer."
>> +  :lighter nil
> 
> There is no lighter by default, I prefer writing :global nil, to make 
> it
> explicit to the reader that this is a local minor mode.

Thanks, changed.

Can you update define-minor-mode's docstring to guide others in the 
right direction?  I only passed :lighter nil because that was the 
example given by define-minor-mode's docstring, "If you provide BODY, 
then you must provide at least one keyword argument (e.g. `:lighter 
nil')".

>> +  (let ((should-display (and window-tool-bar-mode
... partial patch elided ...
>> +;;; window-tool-bar.el ends here
> 
> Hope this was of use.

Thank you for the thorough review!

  -- MJF

[0003-Adding-window-tool-bar-package.patch (text/x-diff, attachment)]

Message #46 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Jared Finder <jared <at> finder.org>
Cc: Philip Kaludercic <philipk <at> posteo.net>, Eli Zaretskii <eliz <at> gnu.org>,
 68765 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Thu, 02 May 2024 09:03:20 +0300

>> It's been four weeks and I've seen no reply to this comment.  Are there any
>> other concerns?
>
> Sorry, I didn't know that you expected that I should review your patch -
> I was silent because your last patch has no changes in tab-line.el,
> so I have no more objections in this regard.

The reason why no changes are required in tab-line.el is
because this feature is not exclusively for the tab line,
so window-tool-bar.el should also handle the case
when the tool bar is combined with the header line.
For example, in Info mode the users might prefer to show
the Info tool bar icons and Next/Prev/Up
on the same header line.

Message #49 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: philipk <at> posteo.net, jared <at> finder.org, 68765 <at> debbugs.gnu.org,
 monnier <at> iro.umontreal.ca
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Thu, 09 May 2024 11:00:20 +0300

> From: Juri Linkov <juri <at> linkov.net>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  68765 <at> debbugs.gnu.org,  Philip Kaludercic
>  <philipk <at> posteo.net>,  Stefan Monnier <monnier <at> iro.umontreal.ca>
> Date: Thu, 02 May 2024 09:03:20 +0300
> 
> >> It's been four weeks and I've seen no reply to this comment.  Are there any
> >> other concerns?
> >
> > Sorry, I didn't know that you expected that I should review your patch -
> > I was silent because your last patch has no changes in tab-line.el,
> > so I have no more objections in this regard.
> 
> The reason why no changes are required in tab-line.el is
> because this feature is not exclusively for the tab line,
> so window-tool-bar.el should also handle the case
> when the tool bar is combined with the header line.
> For example, in Info mode the users might prefer to show
> the Info tool bar icons and Next/Prev/Up
> on the same header line.

If there's an agreement about this issue, could you, Jared, please
post the final patch reflecting the agreements?

If there's no agreement, what are the issues that prevent it?

Thanks.

Message #52 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Jared Finder <jared <at> finder.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: philipk <at> posteo.net, 68765 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Thu, 09 May 2024 21:24:02 -0700

On 2024-05-09 01:00, Eli Zaretskii wrote:
>> From: Juri Linkov <juri <at> linkov.net>
>> Cc: Eli Zaretskii <eliz <at> gnu.org>,  68765 <at> debbugs.gnu.org,  Philip 
>> Kaludercic
>>  <philipk <at> posteo.net>,  Stefan Monnier <monnier <at> iro.umontreal.ca>
>> Date: Thu, 02 May 2024 09:03:20 +0300
>> 
>> >> It's been four weeks and I've seen no reply to this comment.  Are there any
>> >> other concerns?
>> >
>> > Sorry, I didn't know that you expected that I should review your patch -
>> > I was silent because your last patch has no changes in tab-line.el,
>> > so I have no more objections in this regard.
>> 
>> The reason why no changes are required in tab-line.el is
>> because this feature is not exclusively for the tab line,
>> so window-tool-bar.el should also handle the case
>> when the tool bar is combined with the header line.
>> For example, in Info mode the users might prefer to show
>> the Info tool bar icons and Next/Prev/Up
>> on the same header line.
> 
> If there's an agreement about this issue, could you, Jared, please
> post the final patch reflecting the agreements?
> 
> If there's no agreement, what are the issues that prevent it?

I'm unclear on what changes Juri would like to window-tool-bar.el.  
Sorry for the really long reply, I just want to make sure things are 
extra clear so I can proceed.

First, let me share some additional information.  window-tool-bar.el 
currently provides two ways for a user to put the tool bar buttons into 
tab line / header line / mode line:

The command window-tool-bar-mode for simple enablement.
The function window-tool-bar-string can be added as (:eval 
(window-tool-bar-string)) to tab-line-format, header-line-format, or 
mode-line-format programmatically for more advanced placement.

I think the above interface is the right thing to have from a user 
perspective.  Then the most straightforward thing would be to make M-x 
window-tool-bar-mode behave similar to M-x tab-line-mode: it would 
always put the tool bar on the tab line, and only make changes if 
nothing else changed tab-line-format.

Additionally, I would make a very small change then to tab-line.el to 
display a message to the user if M-x tab-line-mode does not actually 
changing anything to avoid user confusion as this now can happen by 
running commands (M-x window-tool-bar-mode RET M-x tab-line-mode RET, 
for example).  I would add similar messaging to window-tool-bar as well.

Does this sound like a good plan?

  -- MJF

Message #55 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Jared Finder <jared <at> finder.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: philipk <at> posteo.net, 68765 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Fri, 10 May 2024 21:33:46 -0700

My prior reply got blocked as spam. Sending again.

On 2024-05-09 01:00, Eli Zaretskii wrote:
>> From: Juri Linkov <juri <at> linkov.net>
>> Cc: Eli Zaretskii <eliz <at> gnu.org>,  68765 <at> debbugs.gnu.org,  Philip 
>> Kaludercic
>>  <philipk <at> posteo.net>,  Stefan Monnier <monnier <at> iro.umontreal.ca>
>> Date: Thu, 02 May 2024 09:03:20 +0300
>> 
>> >> It's been four weeks and I've seen no reply to this comment.  Are there any
>> >> other concerns?
>> >
>> > Sorry, I didn't know that you expected that I should review your patch -
>> > I was silent because your last patch has no changes in tab-line.el,
>> > so I have no more objections in this regard.
>> 
>> The reason why no changes are required in tab-line.el is
>> because this feature is not exclusively for the tab line,
>> so window-tool-bar.el should also handle the case
>> when the tool bar is combined with the header line.
>> For example, in Info mode the users might prefer to show
>> the Info tool bar icons and Next/Prev/Up
>> on the same header line.
> 
> If there's an agreement about this issue, could you, Jared, please
> post the final patch reflecting the agreements?
> 
> If there's no agreement, what are the issues that prevent it?

I'm unclear on what changes Juri would like to window-tool-bar.el.  
Sorry for the really long reply, I just want to make sure things are 
extra clear so I can proceed.

First, let me share some additional information.  window-tool-bar.el 
currently provides two ways for a user to put the tool bar buttons into 
tab line / header line / mode line:

The command window-tool-bar-mode for simple enablement.
The function window-tool-bar-string can be added as (:eval 
(window-tool-bar-string)) to tab-line-format, header-line-format, or 
mode-line-format programmatically for more advanced placement.

I think the above interface is the right thing to have from a user 
perspective.  Then the most straightforward thing would be to make M-x 
window-tool-bar-mode behave similar to M-x tab-line-mode: it would 
always put the tool bar on the tab line, and only make changes if 
nothing else changed tab-line-format.

Additionally, I would make a very small change then to tab-line.el to 
display a message to the user if M-x tab-line-mode does not actually 
changing anything to avoid user confusion as this now can happen by 
running commands (M-x window-tool-bar-mode RET M-x tab-line-mode RET, 
for example).  I would add similar messaging to window-tool-bar as well.

Does this sound like a good plan?

  -- MJF

Message #58 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Jared Finder <jared <at> finder.org>
Cc: philipk <at> posteo.net, Eli Zaretskii <eliz <at> gnu.org>, 68765 <at> debbugs.gnu.org,
 monnier <at> iro.umontreal.ca
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Sun, 12 May 2024 19:34:22 +0300

>>> >> It's been four weeks and I've seen no reply to this comment.  Are
>>> >> there any other concerns?
>>> >
>>> > Sorry, I didn't know that you expected that I should review your patch -
>>> > I was silent because your last patch has no changes in tab-line.el,
>>> > so I have no more objections in this regard.
>>> The reason why no changes are required in tab-line.el is
>>> because this feature is not exclusively for the tab line,
>>> so window-tool-bar.el should also handle the case
>>> when the tool bar is combined with the header line.
>>> For example, in Info mode the users might prefer to show
>>> the Info tool bar icons and Next/Prev/Up
>>> on the same header line.
>> If there's an agreement about this issue, could you, Jared, please
>> post the final patch reflecting the agreements?
>> If there's no agreement, what are the issues that prevent it?
>
> I'm unclear on what changes Juri would like to window-tool-bar.el.  Sorry
> for the really long reply, I just want to make sure things are extra clear
> so I can proceed.
>
> First, let me share some additional information.  window-tool-bar.el
> currently provides two ways for a user to put the tool bar buttons into tab
> line / header line / mode line:
>
> The command window-tool-bar-mode for simple enablement.
> The function window-tool-bar-string can be added as (:eval
> (window-tool-bar-string)) to tab-line-format, header-line-format, or
> mode-line-format programmatically for more advanced placement.
>
> I think the above interface is the right thing to have from a user
> perspective.  Then the most straightforward thing would be to make M-x
> window-tool-bar-mode behave similar to M-x tab-line-mode: it would always
> put the tool bar on the tab line, and only make changes if nothing else
> changed tab-line-format.
>
> Additionally, I would make a very small change then to tab-line.el to
> display a message to the user if M-x tab-line-mode does not actually
> changing anything to avoid user confusion as this now can happen by running
> commands (M-x window-tool-bar-mode RET M-x tab-line-mode RET, for example).
> I would add similar messaging to window-tool-bar as well.
>
> Does this sound like a good plan?

Thanks, looks like a good thing to do.

Message #61 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Jared Finder <jared <at> finder.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: philipk <at> posteo.net, Eli Zaretskii <eliz <at> gnu.org>, 68765 <at> debbugs.gnu.org,
 monnier <at> iro.umontreal.ca
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Mon, 13 May 2024 21:14:16 -0700

[Message part 1 (text/plain, inline)]

On 2024-05-12 09:34, Juri Linkov wrote:
>>> If there's an agreement about this issue, could you, Jared, please
>>> post the final patch reflecting the agreements?
>>> If there's no agreement, what are the issues that prevent it?
... details elided ...
>> Does this sound like a good plan?
> 
> Thanks, looks like a good thing to do.

Thank you!  Final version of all three commits attached.  These address 
all comments raised on this thread.

I think after these patches are applied, the remaining work to resolve 
this would be having the ELPA package added and me updating NEWS and the 
manual.  I can start on the updates.

  -- MJF

[0001-Inform-user-when-tab-line-mode-command-makes-no-chan.patch (text/x-diff, attachment)]

[0002-Add-user-option-to-only-display-default-tool-bar.patch (text/x-diff, attachment)]

[0003-Adding-window-tool-bar-package.patch (text/x-diff, attachment)]

Message #64 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Jared Finder <jared <at> finder.org>
Cc: philipk <at> posteo.net, Eli Zaretskii <eliz <at> gnu.org>, 68765 <at> debbugs.gnu.org,
 monnier <at> iro.umontreal.ca
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Tue, 14 May 2024 09:01:17 +0300

>>> Does this sound like a good plan?
>> Thanks, looks like a good thing to do.
>
> Thank you!  Final version of all three commits attached.  These address all
> comments raised on this thread.
>
> I think after these patches are applied, the remaining work to resolve this
> would be having the ELPA package added and me updating NEWS and the manual.
> I can start on the updates.

Thanks, the changes in tab-line.el look nice.

I hope your other patches for tool-bar.el and window-tool-bar.el
will get the green light as well.

Message #67 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jared Finder <jared <at> finder.org>
Cc: philipk <at> posteo.net, 68765 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 juri <at> linkov.net
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Sat, 18 May 2024 12:45:35 +0300

> Date: Mon, 13 May 2024 21:14:16 -0700
> From: Jared Finder <jared <at> finder.org>
> Cc: Eli Zaretskii <eliz <at> gnu.org>, 68765 <at> debbugs.gnu.org, philipk <at> posteo.net,
>  monnier <at> iro.umontreal.ca
> 
> > Thanks, looks like a good thing to do.
> 
> Thank you!  Final version of all three commits attached.  These address 
> all comments raised on this thread.
> 
> I think after these patches are applied, the remaining work to resolve 
> this would be having the ELPA package added and me updating NEWS and the 
> manual.  I can start on the updates.

Fine by me, please go ahead.

Message #70 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jared Finder <jared <at> finder.org>
Cc: philipk <at> posteo.net, 68765 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 juri <at> linkov.net
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Sat, 18 May 2024 12:48:14 +0300

> Date: Mon, 13 May 2024 21:14:16 -0700
> From: Jared Finder <jared <at> finder.org>
> Cc: Eli Zaretskii <eliz <at> gnu.org>, 68765 <at> debbugs.gnu.org, philipk <at> posteo.net,
>  monnier <at> iro.umontreal.ca
> 
> > Thanks, looks like a good thing to do.
> 
> Thank you!  Final version of all three commits attached.  These address 
> all comments raised on this thread.
> 
> I think after these patches are applied, the remaining work to resolve 
> this would be having the ELPA package added and me updating NEWS and the 
> manual.  I can start on the updates.

I installed the patches on the master branch.

Message #73 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: jared <at> finder.org, Juri Linkov <juri <at> linkov.net>
Cc: philipk <at> posteo.net, 68765 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Sat, 18 May 2024 12:50:07 +0300

> From: Juri Linkov <juri <at> linkov.net>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  68765 <at> debbugs.gnu.org,
>   philipk <at> posteo.net,  monnier <at> iro.umontreal.ca
> Date: Tue, 14 May 2024 09:01:17 +0300
> 
> >>> Does this sound like a good plan?
> >> Thanks, looks like a good thing to do.
> >
> > Thank you!  Final version of all three commits attached.  These address all
> > comments raised on this thread.
> >
> > I think after these patches are applied, the remaining work to resolve this
> > would be having the ELPA package added and me updating NEWS and the manual.
> > I can start on the updates.
> 
> Thanks, the changes in tab-line.el look nice.
> 
> I hope your other patches for tool-bar.el and window-tool-bar.el
> will get the green light as well.

Byte-compiling emits some warnings:

  In toplevel form:
  window-tool-bar.el:341:21: Warning: `mouse-wheel-down-event' is an obsolete variable (as of 30.1); use `mouse-wheel-buttons' instead.
  window-tool-bar.el:341:44: Warning: `mouse-wheel-up-event' is an obsolete variable (as of 30.1); use `mouse-wheel-buttons' instead.
  window-tool-bar.el:342:21: Warning: `mouse-wheel-left-event' is an obsolete variable (as of 30.1); use `mouse-wheel-buttons' instead.
  window-tool-bar.el:342:44: Warning: `mouse-wheel-right-event' is an obsolete variable (as of 30.1); use `mouse-wheel-buttons' instead.

Jared, could you please take care of these?

Thanks.

Message #76 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Jared Finder <jared <at> finder.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: philipk <at> posteo.net, 68765 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Sat, 18 May 2024 20:55:38 -0700

[Message part 1 (text/plain, inline)]

On 2024-05-18 02:50, Eli Zaretskii wrote:
> 
> Byte-compiling emits some warnings:
> 
>   In toplevel form:
>   window-tool-bar.el:341:21: Warning: `mouse-wheel-down-event' is an 
> obsolete variable (as of 30.1); use `mouse-wheel-buttons' instead.
>   window-tool-bar.el:341:44: Warning: `mouse-wheel-up-event' is an 
> obsolete variable (as of 30.1); use `mouse-wheel-buttons' instead.
>   window-tool-bar.el:342:21: Warning: `mouse-wheel-left-event' is an 
> obsolete variable (as of 30.1); use `mouse-wheel-buttons' instead.
>   window-tool-bar.el:342:44: Warning: `mouse-wheel-right-event' is an 
> obsolete variable (as of 30.1); use `mouse-wheel-buttons' instead.
> 
> Jared, could you please take care of these?
> 
> Thanks.

The attached patch fixes those warnings.

  -- MJF

[0001-Fix-byte-compiler-warnings.patch (text/x-diff, attachment)]

Message #79 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Jared Finder <jared <at> finder.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: philipk <at> posteo.net, 68765 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 juri <at> linkov.net
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Sat, 18 May 2024 20:58:05 -0700

On 2024-05-18 02:48, Eli Zaretskii wrote:
>> Date: Mon, 13 May 2024 21:14:16 -0700
>> From: Jared Finder <jared <at> finder.org>
>> Cc: Eli Zaretskii <eliz <at> gnu.org>, 68765 <at> debbugs.gnu.org, 
>> philipk <at> posteo.net,
>>  monnier <at> iro.umontreal.ca
>> 
>> > Thanks, looks like a good thing to do.
>> 
>> Thank you!  Final version of all three commits attached.  These 
>> address
>> all comments raised on this thread.
>> 
>> I think after these patches are applied, the remaining work to resolve
>> this would be having the ELPA package added and me updating NEWS and 
>> the
>> manual.  I can start on the updates.
> 
> I installed the patches on the master branch.

Thank you!

Is there anything else that needs to be done to get this package on 
ELPA?  I'd like to use this package on older Emacsen as well.

  -- MJF

Message #82 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jared Finder <jared <at> finder.org>
Cc: philipk <at> posteo.net, 68765 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 juri <at> linkov.net
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Sun, 19 May 2024 09:43:29 +0300

> Date: Sat, 18 May 2024 20:55:38 -0700
> From: Jared Finder <jared <at> finder.org>
> Cc: Juri Linkov <juri <at> linkov.net>, 68765 <at> debbugs.gnu.org,
>  philipk <at> posteo.net, monnier <at> iro.umontreal.ca
> 
> On 2024-05-18 02:50, Eli Zaretskii wrote:
> > 
> > Byte-compiling emits some warnings:
> > 
> >   In toplevel form:
> >   window-tool-bar.el:341:21: Warning: `mouse-wheel-down-event' is an 
> > obsolete variable (as of 30.1); use `mouse-wheel-buttons' instead.
> >   window-tool-bar.el:341:44: Warning: `mouse-wheel-up-event' is an 
> > obsolete variable (as of 30.1); use `mouse-wheel-buttons' instead.
> >   window-tool-bar.el:342:21: Warning: `mouse-wheel-left-event' is an 
> > obsolete variable (as of 30.1); use `mouse-wheel-buttons' instead.
> >   window-tool-bar.el:342:44: Warning: `mouse-wheel-right-event' is an 
> > obsolete variable (as of 30.1); use `mouse-wheel-buttons' instead.
> > 
> > Jared, could you please take care of these?
> > 
> > Thanks.
> 
> The attached patch fixes those warnings.

Thanks, installed.

Message #85 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jared Finder <jared <at> finder.org>
Cc: philipk <at> posteo.net, 68765 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 juri <at> linkov.net
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Sun, 19 May 2024 09:48:00 +0300

> Date: Sat, 18 May 2024 20:58:05 -0700
> From: Jared Finder <jared <at> finder.org>
> Cc: juri <at> linkov.net, 68765 <at> debbugs.gnu.org, philipk <at> posteo.net,
>  monnier <at> iro.umontreal.ca
> 
> On 2024-05-18 02:48, Eli Zaretskii wrote:
> >> I think after these patches are applied, the remaining work to resolve
> >> this would be having the ELPA package added and me updating NEWS and 
> >> the
> >> manual.  I can start on the updates.
> > 
> > I installed the patches on the master branch.
> 
> Thank you!
> 
> Is there anything else that needs to be done to get this package on 
> ELPA?  I'd like to use this package on older Emacsen as well.

Philip will tell, but what I meant was to ask you to work on updates
for NEWS and the manual.

Message #88 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Jared Finder <jared <at> finder.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68765 <at> debbugs.gnu.org,
 monnier <at> iro.umontreal.ca, juri <at> linkov.net
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Sun, 19 May 2024 14:41:03 +0000

[Message part 1 (text/plain, inline)]

Jared Finder <jared <at> finder.org> writes:

> On 2024-05-18 02:48, Eli Zaretskii wrote:
>>> Date: Mon, 13 May 2024 21:14:16 -0700
>>> From: Jared Finder <jared <at> finder.org>
>>> Cc: Eli Zaretskii <eliz <at> gnu.org>, 68765 <at> debbugs.gnu.org,
>>> philipk <at> posteo.net,
>>>  monnier <at> iro.umontreal.ca
>>>  > Thanks, looks like a good thing to do.
>>> Thank you!  Final version of all three commits attached.  These
>>> address
>>> all comments raised on this thread.
>>> I think after these patches are applied, the remaining work to
>>> resolve
>>> this would be having the ELPA package added and me updating NEWS
>>> and the
>>> manual.  I can start on the updates.
>> I installed the patches on the master branch.
>
> Thank you!
>
> Is there anything else that needs to be done to get this package on
> ELPA?  I'd like to use this package on older Emacsen as well.

It just has to be added to ELPA, but that is a one-line patch to
elpa.git.  Here is the tarball I generated locally:

[window-tool-bar.tar (application/x-tar, attachment)]

[Message part 3 (text/plain, inline)]

Can you confirm that it looks alright and works on older systems (use
M-x package-install-file)?

>   -- MJF
>

Eli Zaretskii <eliz <at> gnu.org> writes:

>> Date: Sat, 18 May 2024 20:58:05 -0700
>> From: Jared Finder <jared <at> finder.org>
>> Cc: juri <at> linkov.net, 68765 <at> debbugs.gnu.org, philipk <at> posteo.net,
>>  monnier <at> iro.umontreal.ca
>> 
>> On 2024-05-18 02:48, Eli Zaretskii wrote:
>> >> I think after these patches are applied, the remaining work to resolve
>> >> this would be having the ELPA package added and me updating NEWS and 
>> >> the
>> >> manual.  I can start on the updates.
>> > 
>> > I installed the patches on the master branch.
>> 
>> Thank you!
>> 
>> Is there anything else that needs to be done to get this package on 
>> ELPA?  I'd like to use this package on older Emacsen as well.
>
> Philip will tell, but what I meant was to ask you to work on updates
> for NEWS and the manual.

Where will the package be documented?  Should anything be added to the
ELPA package?

-- 
	Philip Kaludercic on icterid

Message #91 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Jared Finder <jared <at> finder.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68765 <at> debbugs.gnu.org,
 monnier <at> iro.umontreal.ca, juri <at> linkov.net
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Sun, 19 May 2024 20:25:01 -0700

On 2024-05-19 07:41, Philip Kaludercic wrote:
> Jared Finder <jared <at> finder.org> writes:
>> 
>> Is there anything else that needs to be done to get this package on
>> ELPA?  I'd like to use this package on older Emacsen as well.
> 
> It just has to be added to ELPA, but that is a one-line patch to
> elpa.git.  Here is the tarball I generated locally:
> 
> 
> 
> Can you confirm that it looks alright and works on older systems (use
> M-x package-install-file)?

Confirmed, just tested it! It worked fine on Emacs 29.1 the package's 
minimum supported version.

>>> 
>>> Is there anything else that needs to be done to get this package on
>>> ELPA?  I'd like to use this package on older Emacsen as well.
>> 
>> Philip will tell, but what I meant was to ask you to work on updates
>> for NEWS and the manual.
> 
> Where will the package be documented?  Should anything be added to the
> ELPA package?

I was hoping documentation could just stay in the core Emacs manual.  
window-tool-bar is a core package and I was hoping to document it next 
to existing documentation on the existing (frame) tool bar.  Does that 
seem reasonable?

  -- MJF

Message #94 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Jared Finder <jared <at> finder.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68765 <at> debbugs.gnu.org,
 monnier <at> iro.umontreal.ca, juri <at> linkov.net
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Mon, 20 May 2024 06:39:25 +0000

Jared Finder <jared <at> finder.org> writes:

> On 2024-05-19 07:41, Philip Kaludercic wrote:
>> Jared Finder <jared <at> finder.org> writes:
>>> Is there anything else that needs to be done to get this package on
>>> ELPA?  I'd like to use this package on older Emacsen as well.
>> It just has to be added to ELPA, but that is a one-line patch to
>> elpa.git.  Here is the tarball I generated locally:
>> Can you confirm that it looks alright and works on older systems
>> (use
>> M-x package-install-file)?
>
> Confirmed, just tested it! It worked fine on Emacs 29.1 the package's
> minimum supported version.

1+

>>>> Is there anything else that needs to be done to get this package
>>>> on
>>>> ELPA?  I'd like to use this package on older Emacsen as well.
>>> Philip will tell, but what I meant was to ask you to work on
>>> updates
>>> for NEWS and the manual.
>> Where will the package be documented?  Should anything be added to
>> the
>> ELPA package?
>
> I was hoping documentation could just stay in the core Emacs manual.
> window-tool-bar is a core package and I was hoping to document it next
> to existing documentation on the existing (frame) tool bar.  Does that
> seem reasonable?

Sure, in that case the core should just link to the specific Info node.
As soon as the manual is updated on the gnu.org website I'd also add a
HTTP link for people previewing the package to be able to see as well.

Otherwise, I'll add the package to ELPA now, and I hope it will appear
on the site in a few hours.

>   -- MJF
>

-- 
	Philip Kaludercic on peregrine

Message #97 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Jared Finder <jared <at> finder.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68765 <at> debbugs.gnu.org,
 monnier <at> iro.umontreal.ca, juri <at> linkov.net
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Mon, 20 May 2024 09:19:16 +0000

Jared Finder <jared <at> finder.org> writes:

> On 2024-05-19 07:41, Philip Kaludercic wrote:
>> Jared Finder <jared <at> finder.org> writes:
>>> Is there anything else that needs to be done to get this package on
>>> ELPA?  I'd like to use this package on older Emacsen as well.
>> It just has to be added to ELPA, but that is a one-line patch to
>> elpa.git.  Here is the tarball I generated locally:
>> Can you confirm that it looks alright and works on older systems
>> (use
>> M-x package-install-file)?
>
> Confirmed, just tested it! It worked fine on Emacs 29.1 the package's
> minimum supported version.

Regarding that point, I didn't follow the thread in detail, but why is
29.1 the minimum version?  package-lint indicates that it should be
compatible with 27.1, but it might be missing something.

-- 
	Philip Kaludercic on peregrine

Message #100 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Jared Finder <jared <at> finder.org>
Cc: philipk <at> posteo.net, Eli Zaretskii <eliz <at> gnu.org>, monnier <at> iro.umontreal.ca,
 68765 <at> debbugs.gnu.org
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Mon, 20 May 2024 20:14:47 +0300

>> I think after these patches are applied, the remaining work to resolve this
>> would be having the ELPA package added and me updating NEWS and the manual.
>> I can start on the updates.
>
> Thanks, the changes in tab-line.el look nice.

Oh, for some reason now a lot of messages are displayed while restoring
the desktop where tab-line-mode is saved for every buffer:

tab-line-format set outside of tab-line-mode, currently ‘(:eval (tab-line-format))’
...
tab-line-format set outside of tab-line-mode, currently ‘(:eval (tab-line-format))’ [2 times]
...
tab-line-format set outside of tab-line-mode, currently ‘(:eval (tab-line-format))’
...
tab-line-format set outside of tab-line-mode, currently ‘(:eval (tab-line-format))’ [8 times]
...
tab-line-format set outside of tab-line-mode, currently ‘(:eval (tab-line-format))’ [35 times]

Message #103 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Jared Finder <jared <at> finder.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: philipk <at> posteo.net, Eli Zaretskii <eliz <at> gnu.org>, monnier <at> iro.umontreal.ca,
 68765 <at> debbugs.gnu.org
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Mon, 20 May 2024 19:25:49 -0700

[Message part 1 (text/plain, inline)]

On 2024-05-20 10:14, Juri Linkov wrote:
>>> I think after these patches are applied, the remaining work to 
>>> resolve this
>>> would be having the ELPA package added and me updating NEWS and the 
>>> manual.
>>> I can start on the updates.
>> 
>> Thanks, the changes in tab-line.el look nice.
> 
> Oh, for some reason now a lot of messages are displayed while restoring
> the desktop where tab-line-mode is saved for every buffer:
> 
> tab-line-format set outside of tab-line-mode, currently ‘(:eval 
> (tab-line-format))’
> ...
> tab-line-format set outside of tab-line-mode, currently ‘(:eval 
> (tab-line-format))’ [2 times]
> ...
> tab-line-format set outside of tab-line-mode, currently ‘(:eval 
> (tab-line-format))’
> ...
> tab-line-format set outside of tab-line-mode, currently ‘(:eval 
> (tab-line-format))’ [8 times]
> ...
> tab-line-format set outside of tab-line-mode, currently ‘(:eval 
> (tab-line-format))’ [35 times]

Sorry about that.  Fix attached.

  -- MJF

[0001-Do-not-message-for-repeated-enable-disable-of-mode.patch (text/x-diff, attachment)]

Message #106 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Jared Finder <jared <at> finder.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68765 <at> debbugs.gnu.org,
 monnier <at> iro.umontreal.ca, juri <at> linkov.net
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Mon, 20 May 2024 21:18:12 -0700

On 2024-05-20 02:19, Philip Kaludercic wrote:
> Jared Finder <jared <at> finder.org> writes:
>> On 2024-05-19 07:41, Philip Kaludercic wrote:
>>> Jared Finder <jared <at> finder.org> writes:
>>>> Is there anything else that needs to be done to get this package on
>>>> ELPA?  I'd like to use this package on older Emacsen as well.
>>> It just has to be added to ELPA, but that is a one-line patch to
>>> elpa.git.  Here is the tarball I generated locally:
>>> Can you confirm that it looks alright and works on older systems
>>> (use
>>> M-x package-install-file)?
>> 
>> Confirmed, just tested it! It worked fine on Emacs 29.1 the package's
>> minimum supported version.
> 
> Regarding that point, I didn't follow the thread in detail, but why is
> 29.1 the minimum version?  package-lint indicates that it should be
> compatible with 27.1, but it might be missing something.

Only because that was the version I used regularly when I started 
writing this.  I plan to extend support to earlier Emacs versions in a 
future version of this package.  I'm just waiting on squaring away this 
first step before working on any updates. :)

On 2024-05-19 23:39, Philip Kaludercic wrote:
> Jared Finder <jared <at> finder.org> writes:
>> On 2024-05-19 07:41, Philip Kaludercic wrote:
>>> Where will the package be documented?  Should anything be added to
>>> the
>>> ELPA package?
>> 
>> I was hoping documentation could just stay in the core Emacs manual.
>> window-tool-bar is a core package and I was hoping to document it next
>> to existing documentation on the existing (frame) tool bar.  Does that
>> seem reasonable?
> 
> Sure, in that case the core should just link to the specific Info node.
> As soon as the manual is updated on the gnu.org website I'd also add a
> HTTP link for people previewing the package to be able to see as well.

I'm not quite sure what change you're looking for here.  Let's assume 
I've added an info node "(emacs)Window Tool Bar" that is also a function 
index for global-window-tool-bar-mode, where do you think I should 
mention this link?

> Otherwise, I'll add the package to ELPA now, and I hope it will appear
> on the site in a few hours.

Yup, I see it! Thanks!

  -- MJF

Message #109 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Jared Finder <jared <at> finder.org>
Cc: philipk <at> posteo.net, Eli Zaretskii <eliz <at> gnu.org>, monnier <at> iro.umontreal.ca,
 68765 <at> debbugs.gnu.org
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Tue, 21 May 2024 09:12:35 +0300

>> tab-line-format set outside of tab-line-mode, currently ‘(:eval
>> (tab-line-format))’ [35 times]
>
> Sorry about that.  Fix attached.

Thanks, I confirm that the problem is fixed completely,
so your patch is pushed now.

Message #112 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Jared Finder <jared <at> finder.org>
Cc: Philip Kaludercic <philipk <at> posteo.net>, 68765 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, juri <at> linkov.net
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Tue, 21 May 2024 09:40:01 -0400

>> Regarding that point, I didn't follow the thread in detail, but why is
>> 29.1 the minimum version?  package-lint indicates that it should be
>> compatible with 27.1, but it might be missing something.
> Only because that was the version I used regularly when I started writing
> this.  I plan to extend support to earlier Emacs versions in a future
> version of this package.  I'm just waiting on squaring away this first step
> before working on any updates. :)

I think `Package-Requires:` and other such version dependencies should
state "I know it won't work" or "I have no intention to fix bugs" for
versions that do not satisfy the dependencies.
Not "I currently don't know if it works or not".

You can separately state which version(s) you recommend and/or have tested.


        Stefan

Message #115 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Jared Finder <jared <at> finder.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68765 <at> debbugs.gnu.org,
 monnier <at> iro.umontreal.ca, juri <at> linkov.net
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Wed, 22 May 2024 16:16:44 +0000

Jared Finder <jared <at> finder.org> writes:

> On 2024-05-20 02:19, Philip Kaludercic wrote:
>> Jared Finder <jared <at> finder.org> writes:
>>> On 2024-05-19 07:41, Philip Kaludercic wrote:
>>>> Jared Finder <jared <at> finder.org> writes:
>>>>> Is there anything else that needs to be done to get this package on
>>>>> ELPA?  I'd like to use this package on older Emacsen as well.
>>>> It just has to be added to ELPA, but that is a one-line patch to
>>>> elpa.git.  Here is the tarball I generated locally:
>>>> Can you confirm that it looks alright and works on older systems
>>>> (use
>>>> M-x package-install-file)?
>>> Confirmed, just tested it! It worked fine on Emacs 29.1 the
>>> package's
>>> minimum supported version.
>> Regarding that point, I didn't follow the thread in detail, but why
>> is
>> 29.1 the minimum version?  package-lint indicates that it should be
>> compatible with 27.1, but it might be missing something.
>
> Only because that was the version I used regularly when I started
> writing this.  I plan to extend support to earlier Emacs versions in a
> future version of this package.  I'm just waiting on squaring away
> this first step before working on any updates. :)

If you need some special new functionality, you can make use of the
Compat package, which as of Emacs 30 has a bundled shim making it easier
to use it in core packages.

> On 2024-05-19 23:39, Philip Kaludercic wrote:
>> Jared Finder <jared <at> finder.org> writes:
>>> On 2024-05-19 07:41, Philip Kaludercic wrote:
>>>> Where will the package be documented?  Should anything be added to
>>>> the
>>>> ELPA package?
>>> I was hoping documentation could just stay in the core Emacs
>>> manual.
>>> window-tool-bar is a core package and I was hoping to document it next
>>> to existing documentation on the existing (frame) tool bar.  Does that
>>> seem reasonable?
>> Sure, in that case the core should just link to the specific Info
>> node.
>> As soon as the manual is updated on the gnu.org website I'd also add a
>> HTTP link for people previewing the package to be able to see as well.
>
> I'm not quite sure what change you're looking for here.  Let's assume
> I've added an info node "(emacs)Window Tool Bar" that is also a
> function index for global-window-tool-bar-mode, where do you think I
> should mention this link?

In the commentary section of the package.

>> Otherwise, I'll add the package to ELPA now, and I hope it will appear
>> on the site in a few hours.
>
> Yup, I see it! Thanks!
>
>   -- MJF
>

-- 
	Philip Kaludercic on peregrine

Message #118 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Jared Finder <jared <at> finder.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: philipk <at> posteo.net, 68765 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 juri <at> linkov.net
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Wed, 22 May 2024 21:19:05 -0700

[Message part 1 (text/plain, inline)]

On 2024-05-18 23:48, Eli Zaretskii wrote:
>> Date: Sat, 18 May 2024 20:58:05 -0700
>> From: Jared Finder <jared <at> finder.org>
>> Cc: juri <at> linkov.net, 68765 <at> debbugs.gnu.org, philipk <at> posteo.net,
>>  monnier <at> iro.umontreal.ca
>> 
>> On 2024-05-18 02:48, Eli Zaretskii wrote:
>> >> I think after these patches are applied, the remaining work to resolve
>> >> this would be having the ELPA package added and me updating NEWS and
>> >> the
>> >> manual.  I can start on the updates.
>> >
>> > I installed the patches on the master branch.
>> 
>> Thank you!
>> 
>> Is there anything else that needs to be done to get this package on
>> ELPA?  I'd like to use this package on older Emacsen as well.
> 
> Philip will tell, but what I meant was to ask you to work on updates
> for NEWS and the manual.

Patch attached.  I'm still not comfortable with all the texinfo 
conventions, so please review that I'm using things like @code 
correctly.

Also, I couldn't figure out how to get the Top node menus to auto 
update, so I just manually put in new nodes.

  -- MJF

[0001-Adding-documentation-for-window-tool-bar.patch (text/x-diff, attachment)]

Message #121 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jared Finder <jared <at> finder.org>
Cc: philipk <at> posteo.net, 68765 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 juri <at> linkov.net
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Thu, 23 May 2024 09:52:51 +0300

> Date: Wed, 22 May 2024 21:19:05 -0700
> From: Jared Finder <jared <at> finder.org>
> Cc: juri <at> linkov.net, 68765 <at> debbugs.gnu.org, philipk <at> posteo.net,
>  monnier <at> iro.umontreal.ca
> 
> +@node Window Tool Bar
> +@cindex mode, Window Tool Bar

Index entries should preferably use only lower-case letters, to avoid
problems with sorting index entries in different locales.

In addition, I would rephrase the above index entry to say just

  @cindex window tool bar

> +@findex global-window-tool-bar-mode
> +@vindex global-window-tool-bar-mode

This will create two identical index entries (since we produce a
single Index from all the index entries).  So leave just one of the
two.

> +  The command @code{global-window-tool-bar-mode} toggles the display of
> +a tool bar at the top of all windows.  (You can also customize the
> +variable @code{global-window-tool-bar-mode}.)  To conserve space, a
> +window tool bar is only shown if the tool bar for a buffer is different
> +from the global (default) tool bar.  The most common way a buffer has a
> +different tool bar is due to its major mode, so you can think of the
> +window tool bar as showing a major mode's tool bar if it exists.

This paragraph left me puzzled.  By default, the frame-global tool bar
follows the major mode of the selected buffer's window.  This is not
going to change even under global-window-tool-bar-mode, is it?  If so,
when will the window-specific tool bar be displayed -- only in
non-selected windows whose buffers have a major mode different from
that of the selected-window's buffer?  If so, this should be
explicitly explained in the manual, I think.

Also, does this mean the window-specific tool bar will appear and
disappear depending on which window is selected?  Wouldn't that cause
annoying "flickering" of display?

> +@findex window-tool-bar-mode
> +If you want to toggle the display of a tool bar in just a single window,
> +run the command @code{window-tool-bar-mode}.  This is also useful to put
> +in a hook.  For example if you want the window tool bar to appear for
> +all buffers that do not represent files and have a custom tool bar, you
> +could add the following code to your init file (@pxref{Init File}):

Are these window tool bars also limited to windows whose buffers'
major mode is different from the selected window?  In any case, this
aspect, whether identical to global-window-tool-bar-mode or different
from it, should be explicitly documented.

Last, but not least: please mention the bug number in the commit log
message.


Thanks.

Message #124 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Jared Finder <jared <at> finder.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: philipk <at> posteo.net, 68765 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 juri <at> linkov.net
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Sat, 25 May 2024 08:49:22 -0700

[Message part 1 (text/plain, inline)]

On 2024-05-22 23:52, Eli Zaretskii wrote:
>> Date: Wed, 22 May 2024 21:19:05 -0700
>> From: Jared Finder <jared <at> finder.org>
>> Cc: juri <at> linkov.net, 68765 <at> debbugs.gnu.org, philipk <at> posteo.net,
>>  monnier <at> iro.umontreal.ca
>> 
>> +@node Window Tool Bar
>> +@cindex mode, Window Tool Bar
> 
> Index entries should preferably use only lower-case letters, to avoid
> problems with sorting index entries in different locales.
> 
> In addition, I would rephrase the above index entry to say just
> 
>   @cindex window tool bar

I don't think that fits in with the existing convention.  The convention 
appears to be "use mode, Capital Case Name" for specific modes and 
"mode, lower case" for concepts.  There's 76 other examples of "@cindex 
mode," in the Emacs manual already, the majority of which follow that 
convention.

The only examples I could find not following the convention are: "mode, 
archive", "mode, tar", "mode, display-fill-column-indicator".

> 
>> +@findex global-window-tool-bar-mode
>> +@vindex global-window-tool-bar-mode
> 
> This will create two identical index entries (since we produce a
> single Index from all the index entries).  So leave just one of the
> two.

Makes sense.  Will do.

>> +  The command @code{global-window-tool-bar-mode} toggles the display 
>> of
>> +a tool bar at the top of all windows.  (You can also customize the
>> +variable @code{global-window-tool-bar-mode}.)  To conserve space, a
>> +window tool bar is only shown if the tool bar for a buffer is 
>> different
>> +from the global (default) tool bar.  The most common way a buffer has 
>> a
>> +different tool bar is due to its major mode, so you can think of the
>> +window tool bar as showing a major mode's tool bar if it exists.
> 
> This paragraph left me puzzled.  By default, the frame-global tool bar
> follows the major mode of the selected buffer's window.  This is not
> going to change even under global-window-tool-bar-mode, is it?  If so,
> when will the window-specific tool bar be displayed -- only in
> non-selected windows whose buffers have a major mode different from
> that of the selected-window's buffer?  If so, this should be
> explicitly explained in the manual, I think.
> 
> Also, does this mean the window-specific tool bar will appear and
> disappear depending on which window is selected?  Wouldn't that cause
> annoying "flickering" of display?

I will try to redescribe it.  I have a typo in the existing description. 
 Change "all windows" to "each window".  The idea is that each window 
gets its own tool bar *inside the window*, specifically on the window's 
tab line.  Ideally I think I would just show a picture (see attached 
image), but I see very few pictures in any of the Emacs manuals and none 
of them show up in the HTML output at 
https://www.gnu.org/software/emacs/manual/, only in the PDF.

>> +@findex window-tool-bar-mode
>> +If you want to toggle the display of a tool bar in just a single 
>> window,
>> +run the command @code{window-tool-bar-mode}.  This is also useful to 
>> put
>> +in a hook.  For example if you want the window tool bar to appear for
>> +all buffers that do not represent files and have a custom tool bar, 
>> you
>> +could add the following code to your init file (@pxref{Init File}):
> 
> Are these window tool bars also limited to windows whose buffers'
> major mode is different from the selected window?  In any case, this
> aspect, whether identical to global-window-tool-bar-mode or different
> from it, should be explicitly documented.
> 
> Last, but not least: please mention the bug number in the commit log
> message.

Will do.

  -- MJF

[screenshot.png (image/png, attachment)]

Message #127 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jared Finder <jared <at> finder.org>
Cc: philipk <at> posteo.net, 68765 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 juri <at> linkov.net
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Sat, 25 May 2024 20:03:41 +0300

> Date: Sat, 25 May 2024 08:49:22 -0700
> From: Jared Finder <jared <at> finder.org>
> Cc: juri <at> linkov.net, 68765 <at> debbugs.gnu.org, philipk <at> posteo.net,
>  monnier <at> iro.umontreal.ca
> 
> >> +@node Window Tool Bar
> >> +@cindex mode, Window Tool Bar
> > 
> > Index entries should preferably use only lower-case letters, to avoid
> > problems with sorting index entries in different locales.
> > 
> > In addition, I would rephrase the above index entry to say just
> > 
> >   @cindex window tool bar
> 
> I don't think that fits in with the existing convention.  The convention 
> appears to be "use mode, Capital Case Name" for specific modes and 
> "mode, lower case" for concepts.  There's 76 other examples of "@cindex 
> mode," in the Emacs manual already, the majority of which follow that 
> convention.
> 
> The only examples I could find not following the convention are: "mode, 
> archive", "mode, tar", "mode, display-fill-column-indicator".

Nevertheless, I stand by what I wrote.  The examples which you quote,
whether they are a few or not, are mistakes: letter-case affects
sorting of index entries in locale-dependent way, so index entries
that us capital letters will appear in a different order in different
locales, which is not a good thing.

> I will try to redescribe it.  I have a typo in the existing description. 
>   Change "all windows" to "each window".  The idea is that each window 
> gets its own tool bar *inside the window*, specifically on the window's 
> tab line.  Ideally I think I would just show a picture (see attached 
> image), but I see very few pictures in any of the Emacs manuals and none 
> of them show up in the HTML output at 
> https://www.gnu.org/software/emacs/manual/, only in the PDF.

I don't think there were images in our manuals in previous versions,
which is why they don't appear in HTML.

Pictures have a disadvantage in that they cannot be displayed on TTY
frames, so a picture that cannot be replaced by some "ASCII art"
generally means the reader on a TTY frame will be at a disadvantage.

I will wait for the next version to see if my questions are now
answered or we need to discuss them further.

Thanks.

Message #130 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Jared Finder <jared <at> finder.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: philipk <at> posteo.net, 68765 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 juri <at> linkov.net
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Sat, 25 May 2024 12:54:30 -0700

[Message part 1 (text/plain, inline)]

On 2024-05-25 10:03, Eli Zaretskii wrote:
>> Date: Sat, 25 May 2024 08:49:22 -0700
>> From: Jared Finder <jared <at> finder.org>
>> Cc: juri <at> linkov.net, 68765 <at> debbugs.gnu.org, philipk <at> posteo.net,
>>  monnier <at> iro.umontreal.ca
>> 
>> >> +@node Window Tool Bar
>> >> +@cindex mode, Window Tool Bar
>> >
>> > Index entries should preferably use only lower-case letters, to avoid
>> > problems with sorting index entries in different locales.
>> >
>> > In addition, I would rephrase the above index entry to say just
>> >
>> >   @cindex window tool bar
>> 
>> I don't think that fits in with the existing convention.  The 
>> convention
>> appears to be "use mode, Capital Case Name" for specific modes and
>> "mode, lower case" for concepts.  There's 76 other examples of 
>> "@cindex
>> mode," in the Emacs manual already, the majority of which follow that
>> convention.
>> 
>> The only examples I could find not following the convention are: 
>> "mode,
>> archive", "mode, tar", "mode, display-fill-column-indicator".
> 
> Nevertheless, I stand by what I wrote.  The examples which you quote,
> whether they are a few or not, are mistakes: letter-case affects
> sorting of index entries in locale-dependent way, so index entries
> that us capital letters will appear in a different order in different
> locales, which is not a good thing.

Ok, I tried to fit into the existing convention but not use upper case 
letters with "@cindex mode, window tool bar".  Let me know if that's not 
appropriate.

>> I will try to redescribe it.  I have a typo in the existing 
>> description.
>>   Change "all windows" to "each window".  The idea is that each window
>> gets its own tool bar *inside the window*, specifically on the 
>> window's
>> tab line.  Ideally I think I would just show a picture (see attached
>> image), but I see very few pictures in any of the Emacs manuals and 
>> none
>> of them show up in the HTML output at
>> https://www.gnu.org/software/emacs/manual/, only in the PDF.
> 
> I don't think there were images in our manuals in previous versions,
> which is why they don't appear in HTML.
> 
> Pictures have a disadvantage in that they cannot be displayed on TTY
> frames, so a picture that cannot be replaced by some "ASCII art"
> generally means the reader on a TTY frame will be at a disadvantage.

That's what I was concerned about.  Please let me know if this updated 
text is clear enough.

> I will wait for the next version to see if my questions are now
> answered or we need to discuss them further.

New patch attached.  Feedback welcome.

  -- MJF

[0001-Adding-documentation-for-window-tool-bar-bug-68765.patch (text/x-diff, attachment)]

Message #133 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jared Finder <jared <at> finder.org>
Cc: philipk <at> posteo.net, 68765 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 juri <at> linkov.net
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Sun, 26 May 2024 12:44:31 +0300

> Date: Sat, 25 May 2024 12:54:30 -0700
> From: Jared Finder <jared <at> finder.org>
> Cc: juri <at> linkov.net, 68765 <at> debbugs.gnu.org, philipk <at> posteo.net,
>  monnier <at> iro.umontreal.ca
> 
> Ok, I tried to fit into the existing convention but not use upper case 
> letters with "@cindex mode, window tool bar".  Let me know if that's not 
> appropriate.

LGTM, thanks.

> That's what I was concerned about.  Please let me know if this updated 
> text is clear enough.
> 
> > I will wait for the next version to see if my questions are now
> > answered or we need to discuss them further.
> 
> New patch attached.  Feedback welcome.

Thanks, some comments below.

> +@findex global-window-tool-bar-mode
> +  The command @code{global-window-tool-bar-mode} toggles the display of
> +a tool bar at the top of each window.  When enabled, multiple windows
> +can display their own tool bar simultaneously.  To conserve space, a
> +window tool bar is only shown if the tool bar for that window's current
> +buffer is different from the global (default) tool bar.  The most common
> +way a buffer has a different tool bar is due to its major mode, so you
> +can think of the window tool bar as showing a major mode's tool bar if
> +it exists.

So, let me understand what happens under this global mode.  By
default, the frame's tool bar changes according to the major mode of
the buffer shown in the frame's selected window.  As a very frequent
example, entering the minibuffer from a buffer whose mode is, say,
Dired or Rmail or Info, changes the tool bar.  When that happens, some
windows which previously didn't display their tool bar because it was
identical to the frame's tool bar will now display their tool bars.
Right?  And when you exit the minibuffer, those window-specific tool
bars will again disappear, right?  Wouldn't that cause annoying
flickering of the display?  (I know that setting
tool-bar-always-show-default non-nil can prevent that, but I'm asking
about the default behavior.)

Btw, what exactly is the meaning of "the tool bar of the window is
different from the global tool bar"?  How are tool bars compared?  For
example, a button on the tool bar can have the :visible attribute,
which will cause the icon not to appear under some conditions -- will
a tool bar with that icon on display be considered "different" from a
tool bar where the icon is not shown due to :visible conditions?

> +@findex window-tool-bar-mode
> +If you want to toggle the display of a window tool bar for only some
> +buffers, run the command @code{window-tool-bar-mode}.

I guess this should say that window-tool-bar-mode should be run in the
buffer(s) where the window tool bar is desired?

> +bar will then only appear at the top of any window displaying that
> +buffer.  This is useful to put in a hook.

"in a mode hook", I suppose?

> +Note that the window tool bar displays in the same space as the tab
> +line, so only one of these can be display at a time unless you customize
> +the value of @code{tab-line-format} in Lisp.  In this case, add
> +@code{(:eval (window-tool-bar-strng))} to @code{tab-line-format}.
                                 ^^^^^
A typo?

> +  A window can have a @dfn{tab line} at the top.  If both the tab line
> +and header line are visible, the tab line appears above the header line.
> +The tab line feature works just like the mode line feature, except that
> +it's controlled by @code{tab-line-format}:
> +
> +@defvar tab-line-format
> +This variable, local in every buffer, specifies how to display the tab
> +line, for windows displaying the buffer.  The format of the value is the
> +same as for @code{mode-line-format} (@pxref{Mode Line Data}).  It is
> +normally @code{nil}, so that ordinary buffers have no tab line.
> +@end defvar

I wonder whether it's a good idea to tell that tab line works "the
same as mode line", since the purpose is very different, and the
default value of tab-line-format is very different from the default of
mode-line-format.  At the very least I think we should tell that
tab-line-format should cause the tab line appear as a row of buttons,
clicking on which should change the buffer shown in the window.

> ++++
> +** New user option 'tool-bar-always-show-default'.
> +This can be set so that the tool bar at the top of a frame does not show
> +buffer local customization of the tool bar.  This is convenient when
> +using the newly added 'global-window-tool-bar-mode'.

This should tell the values of the option and their effect.

Message #136 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Jared Finder <jared <at> finder.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: philipk <at> posteo.net, 68765 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 juri <at> linkov.net
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Sun, 26 May 2024 09:20:56 -0700

On 2024-05-26 02:44, Eli Zaretskii wrote:
>> Date: Sat, 25 May 2024 12:54:30 -0700
>> From: Jared Finder <jared <at> finder.org>
>> Cc: juri <at> linkov.net, 68765 <at> debbugs.gnu.org, philipk <at> posteo.net,
>>  monnier <at> iro.umontreal.ca
>> 
>> Ok, I tried to fit into the existing convention but not use upper case
>> letters with "@cindex mode, window tool bar".  Let me know if that's 
>> not
>> appropriate.
> 
> LGTM, thanks.
> 
>> That's what I was concerned about.  Please let me know if this updated
>> text is clear enough.
>> 
>> > I will wait for the next version to see if my questions are now
>> > answered or we need to discuss them further.
>> 
>> New patch attached.  Feedback welcome.
> 
> Thanks, some comments below.

The questions around current behavior may be clarified by describing how 
the internals work.

There's a new function, window-tool-bar-string, which converts the 
binding of <tool-bar> to a propertized string containing clickable 
buttons for the tool bar.  This function depends on the current buffer 
but that doesn't result in flickering, see the next paragraph.

window-tool-bar-mode toggles the buffer local value of tab-line-format 
between nil, which shows no tab line, and (:eval 
(window-tool-bar-string)), which shows tool bar buttons in the tab line. 
 When "evaluating" tab-line-format for display Emacs already temporarily 
sets the current buffer, so this evaluation only shows that buffer's 
tool bar, ignoring whatever the user's current buffer is.

And finally, window-tool-bar-mode will keep tab-line-format as nil if 
the binding of <tool-bar> is the same as the global binding because 
setting tab-line-format to non-nil causes the tab line to take up a row, 
even if the result is just an empty string.


Answers also for the questions below follow.

>> +@findex global-window-tool-bar-mode
>> +  The command @code{global-window-tool-bar-mode} toggles the display 
>> of
>> +a tool bar at the top of each window.  When enabled, multiple windows
>> +can display their own tool bar simultaneously.  To conserve space, a
>> +window tool bar is only shown if the tool bar for that window's 
>> current
>> +buffer is different from the global (default) tool bar.  The most 
>> common
>> +way a buffer has a different tool bar is due to its major mode, so 
>> you
>> +can think of the window tool bar as showing a major mode's tool bar 
>> if
>> +it exists.
> 
> So, let me understand what happens under this global mode.  By
> default, the frame's tool bar changes according to the major mode of
> the buffer shown in the frame's selected window.  As a very frequent
> example, entering the minibuffer from a buffer whose mode is, say,
> Dired or Rmail or Info, changes the tool bar.  When that happens, some
> windows which previously didn't display their tool bar because it was
> identical to the frame's tool bar will now display their tool bars.
> Right?  And when you exit the minibuffer, those window-specific tool
> bars will again disappear, right?  Wouldn't that cause annoying
> flickering of the display?  (I know that setting
> tool-bar-always-show-default non-nil can prevent that, but I'm asking
> about the default behavior.)

No, this is not the behavior.  The window tool bar displays that 
buffer's binding of <tool-bar>.  The window tool bar is displayed in the 
tab line of each window and doesn't pay attention to what the current 
buffer or window is.

> Btw, what exactly is the meaning of "the tool bar of the window is
> different from the global tool bar"?  How are tool bars compared?  For
> example, a button on the tool bar can have the :visible attribute,
> which will cause the icon not to appear under some conditions -- will
> a tool bar with that icon on display be considered "different" from a
> tool bar where the icon is not shown due to :visible conditions?

It's specifically using the following test:

(eq tool-bar-map
    (default-value 'tool-bar-map))

This is because the binding for <tool-bar> is actually :filter 
tool-bar-make-keymap which generates a keymap based on tool-bar-map.


Other comments all addressed in an upcoming patch except this one:

>> +  A window can have a @dfn{tab line} at the top.  If both the tab 
>> line
>> +and header line are visible, the tab line appears above the header 
>> line.
>> +The tab line feature works just like the mode line feature, except 
>> that
>> +it's controlled by @code{tab-line-format}:
>> +
>> +@defvar tab-line-format
>> +This variable, local in every buffer, specifies how to display the 
>> tab
>> +line, for windows displaying the buffer.  The format of the value is 
>> the
>> +same as for @code{mode-line-format} (@pxref{Mode Line Data}).  It is
>> +normally @code{nil}, so that ordinary buffers have no tab line.
>> +@end defvar
> 
> I wonder whether it's a good idea to tell that tab line works "the
> same as mode line", since the purpose is very different, and the
> default value of tab-line-format is very different from the default of
> mode-line-format.  At the very least I think we should tell that
> tab-line-format should cause the tab line appear as a row of buttons,
> clicking on which should change the buffer shown in the window.

Perhaps there's some sort of convention we can describe that talks about 
the difference between header-line-format and tab-line-format?  I'd like 
to copy back any changes to header-line-format which I based this text 
off of.

It seems to me that really the only difference is conventional.  I'm 
thinking something along the lines of "header line is usually modified 
by major modes to add additional info (examples: EWW, Info) and the tab 
line is usually modified by minor modes to add IDE-styled buttons 
(examples: tab line mode, window tool bar mode)".

Does that sound right to you?  I'd make the text more specific in its 
description, I just want to confirm the general intent.

  -- MJF

Message #139 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jared Finder <jared <at> finder.org>
Cc: philipk <at> posteo.net, 68765 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 juri <at> linkov.net
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Sun, 26 May 2024 21:40:28 +0300

> Date: Sun, 26 May 2024 09:20:56 -0700
> From: Jared Finder <jared <at> finder.org>
> Cc: juri <at> linkov.net, 68765 <at> debbugs.gnu.org, philipk <at> posteo.net,
>  monnier <at> iro.umontreal.ca
> 
> >> +@findex global-window-tool-bar-mode
> >> +  The command @code{global-window-tool-bar-mode} toggles the display 
> >> of
> >> +a tool bar at the top of each window.  When enabled, multiple windows
> >> +can display their own tool bar simultaneously.  To conserve space, a
> >> +window tool bar is only shown if the tool bar for that window's 
> >> current
> >> +buffer is different from the global (default) tool bar.  The most 
> >> common
> >> +way a buffer has a different tool bar is due to its major mode, so 
> >> you
> >> +can think of the window tool bar as showing a major mode's tool bar 
> >> if
> >> +it exists.
> > 
> > So, let me understand what happens under this global mode.  By
> > default, the frame's tool bar changes according to the major mode of
> > the buffer shown in the frame's selected window.  As a very frequent
> > example, entering the minibuffer from a buffer whose mode is, say,
> > Dired or Rmail or Info, changes the tool bar.  When that happens, some
> > windows which previously didn't display their tool bar because it was
> > identical to the frame's tool bar will now display their tool bars.
> > Right?  And when you exit the minibuffer, those window-specific tool
> > bars will again disappear, right?  Wouldn't that cause annoying
> > flickering of the display?  (I know that setting
> > tool-bar-always-show-default non-nil can prevent that, but I'm asking
> > about the default behavior.)
> 
> No, this is not the behavior.  The window tool bar displays that 
> buffer's binding of <tool-bar>.  The window tool bar is displayed in the 
> tab line of each window and doesn't pay attention to what the current 
> buffer or window is.

But this means that the following text:

>>                                                  To conserve space,
>> a window tool bar is only shown if the tool bar for that window's
>> current buffer is different from the global (default) tool bar

is at least inaccurate.  Specifically, if the frame's global tool bar
changes as result of some command, the identity of the tool bars of
windows to that of the frame is not re-examined.  So if some window
decided that its tool bar is not to be displayed, it will remain
undisplayed until that window shows a different buffer or its buffer
changes its major mode.  Is that correct?  If not, why not?

Or maybe I don't understand well enough what you mean by "The window
tool bar is displayed in the tab line of each window and doesn't pay
attention to what the current buffer or window is."  You are using
here terminology that is confusing: there's no "current window" in
Emacs, only the "selected window", and it is unclear whether by
"current buffer" you mean the global current buffer (the one returned
by the function current-buffer) or the buffer shown in a window (since
you also say "the window's current buffer", another term that we do
not use).

> > Btw, what exactly is the meaning of "the tool bar of the window is
> > different from the global tool bar"?  How are tool bars compared?  For
> > example, a button on the tool bar can have the :visible attribute,
> > which will cause the icon not to appear under some conditions -- will
> > a tool bar with that icon on display be considered "different" from a
> > tool bar where the icon is not shown due to :visible conditions?
> 
> It's specifically using the following test:
> 
> (eq tool-bar-map
>      (default-value 'tool-bar-map))

So a frame's tool bar can completely change its look due to :visible,
and Emacs will still consider it "equal" to the tool bar of some
window where those :visible conditions yield a completely different
look?  Does that make sense?

I'm beginning to think that the feature whereby we don't display the
window's tool bar under some conditions "to conserve space" is more
confusing and hard to document than is useful, and perhaps we should
simply show the window's tool bar unconditionally?

> > I wonder whether it's a good idea to tell that tab line works "the
> > same as mode line", since the purpose is very different, and the
> > default value of tab-line-format is very different from the default of
> > mode-line-format.  At the very least I think we should tell that
> > tab-line-format should cause the tab line appear as a row of buttons,
> > clicking on which should change the buffer shown in the window.
> 
> Perhaps there's some sort of convention we can describe that talks about 
> the difference between header-line-format and tab-line-format?  I'd like 
> to copy back any changes to header-line-format which I based this text 
> off of.
> 
> It seems to me that really the only difference is conventional.  I'm 
> thinking something along the lines of "header line is usually modified 
> by major modes to add additional info (examples: EWW, Info) and the tab 
> line is usually modified by minor modes to add IDE-styled buttons 
> (examples: tab line mode, window tool bar mode)".
> 
> Does that sound right to you?

It doesn't address my concerns.  My concerns are that the purpose of
tab-line and window's tool bar is conceptually very different from
that of the mode line and the header line.  Technically, they are all
controlled by a FOO-format variable that has the same rules and
syntax, but that's an implementation detail.  For example, it would
make no sense at all to use something similar to the default
mode-line-format for the tab-line or the window tool bar, would it?

So what I would like us to say in the manual is that the value of
window-tool-bar-format and of tab-line-format should produce what is
expected from these lines: a row of tabs for tab-line and a row of
icons for the tool bar.  The reader should understand that having a
tab-line-format whose value is "%b %f", for example, makes no sense,
although it will work.  (In general, most or all %-elements of mode
line and header line make no sense in tab-line and tool bar.  It is
hardly an accident that tab-line-format's value is just (:eval FUNC),
in stark contrast to the value of mode-line-format and
header-line-format in modes in which it is non-nil.)

Did I succeed to explain my concerns?

Message #142 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Jared Finder <jared <at> finder.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: philipk <at> posteo.net, 68765 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 juri <at> linkov.net
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Sat, 01 Jun 2024 21:17:58 -0700

[Message part 1 (text/plain, inline)]

Thank you for all the feedback.  Updated patch attached.

On 2024-05-26 11:40, Eli Zaretskii wrote:
>> Date: Sun, 26 May 2024 09:20:56 -0700
>> From: Jared Finder <jared <at> finder.org>
>> Cc: juri <at> linkov.net, 68765 <at> debbugs.gnu.org, philipk <at> posteo.net,
>>  monnier <at> iro.umontreal.ca
>> 
>> >> +@findex global-window-tool-bar-mode
>> >> +  The command @code{global-window-tool-bar-mode} toggles the display
>> >> of
>> >> +a tool bar at the top of each window.  When enabled, multiple windows
>> >> +can display their own tool bar simultaneously.  To conserve space, a
>> >> +window tool bar is only shown if the tool bar for that window's
>> >> current
>> >> +buffer is different from the global (default) tool bar.  The most
>> >> common
>> >> +way a buffer has a different tool bar is due to its major mode, so
>> >> you
>> >> +can think of the window tool bar as showing a major mode's tool bar
>> >> if
>> >> +it exists.
>> >
>> > So, let me understand what happens under this global mode.  By
>> > default, the frame's tool bar changes according to the major mode of
>> > the buffer shown in the frame's selected window.  As a very frequent
>> > example, entering the minibuffer from a buffer whose mode is, say,
>> > Dired or Rmail or Info, changes the tool bar.  When that happens, some
>> > windows which previously didn't display their tool bar because it was
>> > identical to the frame's tool bar will now display their tool bars.
>> > Right?  And when you exit the minibuffer, those window-specific tool
>> > bars will again disappear, right?  Wouldn't that cause annoying
>> > flickering of the display?  (I know that setting
>> > tool-bar-always-show-default non-nil can prevent that, but I'm asking
>> > about the default behavior.)
>> 
>> No, this is not the behavior.  The window tool bar displays that
>> buffer's binding of <tool-bar>.  The window tool bar is displayed in 
>> the
>> tab line of each window and doesn't pay attention to what the current
>> buffer or window is.
> 
> But this means that the following text:
> 
>>>                                                  To conserve space,
>>> a window tool bar is only shown if the tool bar for that window's
>>> current buffer is different from the global (default) tool bar
> 
> is at least inaccurate.  Specifically, if the frame's global tool bar
> changes as result of some command, the identity of the tool bars of
> windows to that of the frame is not re-examined.  So if some window
> decided that its tool bar is not to be displayed, it will remain
> undisplayed until that window shows a different buffer or its buffer
> changes its major mode.  Is that correct?  If not, why not?

Your understanding here is accurate.  I consider this behavior a bug 
that I'd like to fix (in normal use it rarely happens).  I suppose I 
could use add-variable-watcher to detect all changes to tool-bar-map and 
update tool bar visibility.  I'd like to make sure to do this correctly 
and not cause a significant performance hit.

Should such a temporary limitation be documented in the manual?  I added 
a section to describe this limitation to my patch, but I don't know if 
it is appropriate.

> Or maybe I don't understand well enough what you mean by "The window
> tool bar is displayed in the tab line of each window and doesn't pay
> attention to what the current buffer or window is."  You are using
> here terminology that is confusing: there's no "current window" in
> Emacs, only the "selected window", and it is unclear whether by
> "current buffer" you mean the global current buffer (the one returned
> by the function current-buffer) or the buffer shown in a window (since
> you also say "the window's current buffer", another term that we do
> not use).

Thanks for helping me here with terminology here.  Looking over the 
manual, it seems the proper terms are "current buffer", "selected 
window", and "buffer displayed in a window".  I have updated my patch to 
use these terms.

>> > Btw, what exactly is the meaning of "the tool bar of the window is
>> > different from the global tool bar"?  How are tool bars compared?  For
>> > example, a button on the tool bar can have the :visible attribute,
>> > which will cause the icon not to appear under some conditions -- will
>> > a tool bar with that icon on display be considered "different" from a
>> > tool bar where the icon is not shown due to :visible conditions?
>> 
>> It's specifically using the following test:
>> 
>> (eq tool-bar-map
>>      (default-value 'tool-bar-map))
> 
> So a frame's tool bar can completely change its look due to :visible,
> and Emacs will still consider it "equal" to the tool bar of some
> window where those :visible conditions yield a completely different
> look?  Does that make sense?

From a functionality standpoint, I think it makes a lot of sense.  It's 
about if there's a major mode specific tool bar, not just if the list of 
buttons is different.  Major modes with custom tool bars are recommended 
to set tool-bar-map locally and that's what all customizations in core 
Emacs do.  An eq comparison properly catches this.

I have updated the patch to refer to the tool bar binding being 
different to try to communicate this.

> I'm beginning to think that the feature whereby we don't display the
> window's tool bar under some conditions "to conserve space" is more
> confusing and hard to document than is useful, and perhaps we should
> simply show the window's tool bar unconditionally?

I find it really useful to have this auto-show/hide behavior.  As I 
wrote in the docs, "you can think of the window tool bar as showing a 
major mode specific tool bar if it exists".  The most prominent other 
source of a custom tool bar is isearch.  Having no space taken up when 
editing a text file, then having the tool bar appear when I hit C-s 
lines up with what I see in many other programs.  I'd be sad to see this 
behavior removed.

> So what I would like us to say in the manual is that the value of
> window-tool-bar-format

Just to be clear -- there is no new line as you requested a while back. 
The mode just alters the value of tab-line-format.

> and of tab-line-format should produce what is
> expected from these lines: a row of tabs for tab-line and a row of
> icons for the tool bar.  The reader should understand that having a
> tab-line-format whose value is "%b %f", for example, makes no sense,
> although it will work.  (In general, most or all %-elements of mode
> line and header line make no sense in tab-line and tool bar.  It is
> hardly an accident that tab-line-format's value is just (:eval FUNC),
> in stark contrast to the value of mode-line-format and
> header-line-format in modes in which it is non-nil.)
> 
> Did I succeed to explain my concerns?

Yes, I think I understand.  I have updated my patch.

  -- MJF

[0001-Adding-documentation-for-window-tool-bar-bug-68765.patch (text/x-diff, attachment)]

Message #145 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jared Finder <jared <at> finder.org>
Cc: philipk <at> posteo.net, 68765 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 juri <at> linkov.net
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Sun, 02 Jun 2024 08:21:58 +0300

> Date: Sat, 01 Jun 2024 21:17:58 -0700
> From: Jared Finder <jared <at> finder.org>
> Cc: juri <at> linkov.net, 68765 <at> debbugs.gnu.org, philipk <at> posteo.net,
>  monnier <at> iro.umontreal.ca
> 
> >> > So, let me understand what happens under this global mode.  By
> >> > default, the frame's tool bar changes according to the major mode of
> >> > the buffer shown in the frame's selected window.  As a very frequent
> >> > example, entering the minibuffer from a buffer whose mode is, say,
> >> > Dired or Rmail or Info, changes the tool bar.  When that happens, some
> >> > windows which previously didn't display their tool bar because it was
> >> > identical to the frame's tool bar will now display their tool bars.
> >> > Right?  And when you exit the minibuffer, those window-specific tool
> >> > bars will again disappear, right?  Wouldn't that cause annoying
> >> > flickering of the display?  (I know that setting
> >> > tool-bar-always-show-default non-nil can prevent that, but I'm asking
> >> > about the default behavior.)
> >> 
> >> No, this is not the behavior.  The window tool bar displays that
> >> buffer's binding of <tool-bar>.  The window tool bar is displayed in 
> >> the
> >> tab line of each window and doesn't pay attention to what the current
> >> buffer or window is.
> > 
> > But this means that the following text:
> > 
> >>>                                                  To conserve space,
> >>> a window tool bar is only shown if the tool bar for that window's
> >>> current buffer is different from the global (default) tool bar
> > 
> > is at least inaccurate.  Specifically, if the frame's global tool bar
> > changes as result of some command, the identity of the tool bars of
> > windows to that of the frame is not re-examined.  So if some window
> > decided that its tool bar is not to be displayed, it will remain
> > undisplayed until that window shows a different buffer or its buffer
> > changes its major mode.  Is that correct?  If not, why not?
> 
> Your understanding here is accurate.  I consider this behavior a bug 
> that I'd like to fix (in normal use it rarely happens).

If this is a bug which we will fix, then it gets me back to my
question above: Wouldn't these changes in window tool bars cause
annoying flickering of the display that distract the user's attention?

> I suppose I could use add-variable-watcher to detect all changes to
> tool-bar-map and update tool bar visibility.  I'd like to make sure
> to do this correctly and not cause a significant performance hit.

There should be no need to use watchers.  You can instead add a
function to pre-redisplay-function hook.  This hook is called each
time Emacs is about to update the menu bar and the tool bar of some
frames.

> Should such a temporary limitation be documented in the manual?  I added 
> a section to describe this limitation to my patch, but I don't know if 
> it is appropriate.

No, if this is a bug, we don't describe them in the manual.

> > I'm beginning to think that the feature whereby we don't display the
> > window's tool bar under some conditions "to conserve space" is more
> > confusing and hard to document than is useful, and perhaps we should
> > simply show the window's tool bar unconditionally?
> 
> I find it really useful to have this auto-show/hide behavior.  As I 
> wrote in the docs, "you can think of the window tool bar as showing a 
> major mode specific tool bar if it exists".

That's okay, I'm just saying that if the window's major mode has its
own tool bar, we should show that tool bar on a window regardless of
whether the frame's tool bar shows the same buttons.

> The most prominent other source of a custom tool bar is isearch.

Oh, there are many more modes that customize the tool bar in important
ways.  Help mode, Info mode, Customize, GUD modes, email modes, Grep,
and EWW, to name just a few popular ones.

> Having no space taken up when editing a text file, then having the
> tool bar appear when I hit C-s lines up with what I see in many
> other programs.  I'd be sad to see this behavior removed.

That's not what I'm talking about.  I'm talking about removing the
window's tool bar because the frame's tool bar became identical to it.
Here's a scenario:

  . you have a frame with Info mode and some other mode
  . the frame's tool bar is the default one, and the window under Info
    shows its own tool bar from info.el
  . now I type "C-h i" in the window that was previously in mode other
    than Info -- now the frame's tool bar is the one from Info mode,
    and suddenly the other window loses its window-specific tool bar!

IOW, what bothers me is that we _remove_ the window's tool bar because
the frame-global tool bar changed.  And you just confirmed above that,
while currently this is not the behavior, you'd like to fix the code
so it _is_ the behavior.

Am I missing something?

> +  On graphical displays, Emacs puts a @dfn{tool bar} at the top of each
> +frame, just below the menu bar.  This is a row of icons which you can
                                                     ^^^^^
"buttons with icons"

> +The tool bar is a line (sometimes multiple lines) of icons at the top of
                                                        ^^^^^
Likewise.

> +  The command @code{global-window-tool-bar-mode} toggles the display of
> +a tool bar at the top of each window.  When enabled, multiple windows
> +can display their own tool bar simultaneously.  To conserve space, a
> +window tool bar is only shown if the tool bar for the window's displayed
> +buffer has a different binding from the global (default) tool bar.  The

This should explain what does "global (default) tool bar" mean.  My
understanding is that this is the tool bar shown for the frame, which
will continue working as it does now, i.e. be determined by the major
mode of the frame's selected window.

> +Emacs can also display a single tool bar at the top of frames
> +(@pxref{Tool Bars}).  When showing a tool bar at the top of frames as
> +well as windows, you may want to have the frame tool bar not change
> +based on the current buffer's major mode.

There's a subtlety here: the frame's tool bar is determined by the
major mode of the buffer shown in the frame's selected window, not by
the current buffer (which is a single buffer, not per frame).

Message #148 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Jared Finder <jared <at> finder.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: philipk <at> posteo.net, 68765 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 juri <at> linkov.net
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Sun, 02 Jun 2024 08:57:52 -0700

(I mistakenly sent this to just Eli.  Sorry about that!)

On 2024-06-02 08:56, Jared Finder wrote:
> On 2024-06-01 22:21, Eli Zaretskii wrote:
>>> Date: Sat, 01 Jun 2024 21:17:58 -0700
>>> From: Jared Finder <jared <at> finder.org>
>>> Cc: juri <at> linkov.net, 68765 <at> debbugs.gnu.org, philipk <at> posteo.net,
>>>  monnier <at> iro.umontreal.ca
>>> 
>>> > I'm beginning to think that the feature whereby we don't display the
>>> > window's tool bar under some conditions "to conserve space" is more
>>> > confusing and hard to document than is useful, and perhaps we should
>>> > simply show the window's tool bar unconditionally?
>>> 
>>> I find it really useful to have this auto-show/hide behavior.  As I
>>> wrote in the docs, "you can think of the window tool bar as showing a
>>> major mode specific tool bar if it exists".
>> 
>> That's okay, I'm just saying that if the window's major mode has its
>> own tool bar, we should show that tool bar on a window regardless of
>> whether the frame's tool bar shows the same buttons.
> 
> <and also from later on>
> 
>>> +  The command @code{global-window-tool-bar-mode} toggles the display 
>>> of
>>> +a tool bar at the top of each window.  When enabled, multiple 
>>> windows
>>> +can display their own tool bar simultaneously.  To conserve space, a
>>> +window tool bar is only shown if the tool bar for the window's 
>>> displayed
>>> +buffer has a different binding from the global (default) tool bar.  
>>> The
>> 
>> This should explain what does "global (default) tool bar" mean.  My
>> understanding is that this is the tool bar shown for the frame, which
>> will continue working as it does now, i.e. be determined by the major
>> mode of the frame's selected window.
> 
> <and also>
> 
>>> Having no space taken up when editing a text file, then having the
>>> tool bar appear when I hit C-s lines up with what I see in many
>>> other programs.  I'd be sad to see this behavior removed.
>> 
>> That's not what I'm talking about.  I'm talking about removing the
>> window's tool bar because the frame's tool bar became identical to it.
>> Here's a scenario:
>> 
>>   . you have a frame with Info mode and some other mode
>>   . the frame's tool bar is the default one, and the window under Info
>>     shows its own tool bar from info.el
>>   . now I type "C-h i" in the window that was previously in mode other
>>     than Info -- now the frame's tool bar is the one from Info mode,
>>     and suddenly the other window loses its window-specific tool bar!
>> 
>> IOW, what bothers me is that we _remove_ the window's tool bar because
>> the frame-global tool bar changed.  And you just confirmed above that,
>> while currently this is not the behavior, you'd like to fix the code
>> so it _is_ the behavior.
>> 
>> Am I missing something?
> 
> There's a misunderstanding here.  Can you please suggest how I can 
> change the documentation?  I'm not sure why the documentation I wrote 
> is confusing.
> 
> Let me explicitly describe the terms I am using:
> 
> When I write "the global (default) tool bar", that refers to the value 
> of (default-value 'tool-bar-map).  It's the tool bar with New File, 
> Open File, Open Directory, Kill Buffer, Save, Undo, Cut, Copy, Paste, 
> Isearch.  This doesn't change with major mode.  As far as I can tell, 
> it doesn't change during normal Emacs operations (unless a user chooses 
> to modify it, of course).
> 
> When I write "the frame tool bar", that refers to the tool bar 
> displayed at the top of a frame, as controlled by M-x tool-bar-mode.  
> There is no change of behavior here except for the new user option 
> tool-bar-always-show-default.
> 
> When I write "the window tool bar", that refers to the tool bar 
> displayed at the top of a window, as controlled by (newly added) M-x 
> window-tool-bar-mode or M-x global-window-tool-bar-mode.  The window 
> tool bar displays the value of tool-bar-map the window's displayed 
> buffer.  But it only displays that tool bar if it is not the same as 
> the global (default) tool bar.  It does not pay attention to the frame 
> tool bar.  Specifically, the test is:
> 
>  (eq tool-bar-map ;a buffer's tool bar
>      (default-value 'tool-bar-map) ;the global (default) tool bar
>   )
> 
> I hope from the above description it is clear that switching windows 
> does not cause the window tool bar to flicker as the window tool bar 
> doesn't care what the frame tool bar shows.
> 
>>> The most prominent other source of a custom tool bar is isearch.
>> 
>> Oh, there are many more modes that customize the tool bar in important
>> ways.  Help mode, Info mode, Customize, GUD modes, email modes, Grep,
>> and EWW, to name just a few popular ones.
> 
> Are there any others that are minor modes like isearch?  I ask because 
> the "only display when tool bar is different" check isn't run on minor 
> mode changes.  That's the bug I'm referring to that I'd like to fix.  
> Currently Window Tool Bar mode adds a function to isearch-mode-hook and 
> isearch-mode-end-hook to work properly with isearch.  I would prefer a 
> generic way to handle this.
> 
> 
> All other suggested changes I'll add to my next patch.  I'm just 
> waiting to figure out how to properly document the above 
> misunderstanding.
> 
>   -- MJF

Message #151 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jared Finder <jared <at> finder.org>
Cc: philipk <at> posteo.net, 68765 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 juri <at> linkov.net
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Sun, 02 Jun 2024 19:46:55 +0300

> Date: Sun, 02 Jun 2024 08:57:52 -0700
> From: Jared Finder <jared <at> finder.org>
> Cc: juri <at> linkov.net, 68765 <at> debbugs.gnu.org, philipk <at> posteo.net,
>  monnier <at> iro.umontreal.ca
> There's a misunderstanding here.  Can you please suggest how I can 
> change the documentation?  I'm not sure why the documentation I wrote 
> is confusing.

My problem for now is not with the documentation, it is with the
behavior.  When we agree on the behavior, we will be able to describe
it clearly.

> Let me explicitly describe the terms I am using:
> 
> When I write "the global (default) tool bar", that refers to the value 
> of (default-value 'tool-bar-map).  It's the tool bar with New File, 
> Open File, Open Directory, Kill Buffer, Save, Undo, Cut, Copy, Paste, 
> Isearch.  This doesn't change with major mode.  As far as I can tell, 
> it doesn't change during normal Emacs operations (unless a user chooses 
> to modify it, of course).
> 
> When I write "the frame tool bar", that refers to the tool bar 
> displayed at the top of a frame, as controlled by M-x tool-bar-mode.  
> There is no change of behavior here except for the new user option 
> tool-bar-always-show-default.
> 
> When I write "the window tool bar", that refers to the tool bar 
> displayed at the top of a window, as controlled by (newly added) M-x 
> window-tool-bar-mode or M-x global-window-tool-bar-mode.  The window 
> tool bar displays the value of tool-bar-map the window's displayed 
> buffer.  But it only displays that tool bar if it is not the same as 
> the global (default) tool bar.  It does not pay attention to the frame 
> tool bar.  Specifically, the test is:
> 
>  (eq tool-bar-map ;a buffer's tool bar
>      (default-value 'tool-bar-map) ;the global (default) tool bar
>   )

So a window under, say, Info mode will show the Info tool bar even
though the frame also shows that tool bar, just because that tool bar
is different from the default value of tool-bar-map?  Why is this
behavior useful?

How about if we make the behavior simpler and more predictable:

  If a window's buffer has a non-nil value of window-toolbar-mode,
  show the window-specific tool bar regardless of what it is and
  whether it is the same as the default.

Why is this not good enough?

Message #154 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Jared Finder <jared <at> finder.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: philipk <at> posteo.net, 68765 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 juri <at> linkov.net
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Mon, 03 Jun 2024 22:24:55 -0700

On 2024-06-02 09:46, Eli Zaretskii wrote:
>> Date: Sun, 02 Jun 2024 08:57:52 -0700
>> From: Jared Finder <jared <at> finder.org>
>> Cc: juri <at> linkov.net, 68765 <at> debbugs.gnu.org, philipk <at> posteo.net,
>>  monnier <at> iro.umontreal.ca
>> Let me explicitly describe the terms I am using:
>> 
>> When I write "the global (default) tool bar", that refers to the value
>> of (default-value 'tool-bar-map).  It's the tool bar with New File,
>> Open File, Open Directory, Kill Buffer, Save, Undo, Cut, Copy, Paste,
>> Isearch.  This doesn't change with major mode.  As far as I can tell,
>> it doesn't change during normal Emacs operations (unless a user 
>> chooses
>> to modify it, of course).
>> 
>> When I write "the frame tool bar", that refers to the tool bar
>> displayed at the top of a frame, as controlled by M-x tool-bar-mode.
>> There is no change of behavior here except for the new user option
>> tool-bar-always-show-default.
>> 
>> When I write "the window tool bar", that refers to the tool bar
>> displayed at the top of a window, as controlled by (newly added) M-x
>> window-tool-bar-mode or M-x global-window-tool-bar-mode.  The window
>> tool bar displays the value of tool-bar-map the window's displayed
>> buffer.  But it only displays that tool bar if it is not the same as
>> the global (default) tool bar.  It does not pay attention to the frame
>> tool bar.  Specifically, the test is:
>> 
>>  (eq tool-bar-map ;a buffer's tool bar
>>      (default-value 'tool-bar-map) ;the global (default) tool bar
>>   )
> 
> So a window under, say, Info mode will show the Info tool bar even
> though the frame also shows that tool bar, just because that tool bar
> is different from the default value of tool-bar-map?  Why is this
> behavior useful?
> 
> How about if we make the behavior simpler and more predictable:
> 
>   If a window's buffer has a non-nil value of window-toolbar-mode,
>   show the window-specific tool bar regardless of what it is and
>   whether it is the same as the default.
> 
> Why is this not good enough?

I want the window-specfic tool bar to never be shown if there are no 
tool bar buttons, to conserve space.  However, if tab-line-format is non 
nil, the tab line takes up space even if the resulting tab line is nil.  
This can happen if one sets the default tool bar to nil, while keeping 
the mode specific tool bars.

I think there's also a useful case where the frame tool bar is used to 
show a "global" tool bar with buttons that do not act on the current 
buffer (in the current default tool bar: new file, open file, open 
directory, all the modifier tool bar buttons) and the window tool bar is 
used to show buttons that act on the buffer.  In this case, you don't 
want the "global" tool bar to change based on frame's selected window.  
The "tool-bar-always-show-default" variable I added as well as the logic 
with ignoring the default value of tool-bar-map was to enable this use 
case.  I treat the default value of tool-bar-map as "no tool bar buttons 
for this window" since all those buttons are for the global tool bar.  
It'd be fine to limit behavior to only when tool-bar-always-show-default 
was set.

  -- MJF

Message #157 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jared Finder <jared <at> finder.org>
Cc: philipk <at> posteo.net, 68765 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 juri <at> linkov.net
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Tue, 04 Jun 2024 18:59:21 +0300

> Date: Mon, 03 Jun 2024 22:24:55 -0700
> From: Jared Finder <jared <at> finder.org>
> Cc: juri <at> linkov.net, 68765 <at> debbugs.gnu.org, philipk <at> posteo.net,
>  monnier <at> iro.umontreal.ca
> 
> > How about if we make the behavior simpler and more predictable:
> > 
> >   If a window's buffer has a non-nil value of window-toolbar-mode,
> >   show the window-specific tool bar regardless of what it is and
> >   whether it is the same as the default.
> > 
> > Why is this not good enough?
> 
> I want the window-specfic tool bar to never be shown if there are no 
> tool bar buttons, to conserve space.  However, if tab-line-format is non 
> nil, the tab line takes up space even if the resulting tab line is nil.  
> This can happen if one sets the default tool bar to nil, while keeping 
> the mode specific tool bars.

If the issue is not to show an empty tool bar, then this could be done
by a special test, without affecting behavior in other cases.  And
having the tool bar completely empty is such a rare and strange
situation that we could even leave it alone, under the assumption that
such a "tool bar" is simply a bug of sorts.

Complicating the overall behavior, let alone the difficulties of
explaining the behavior in documentation, on behalf of such rare and
very special cases is hardly a good tradeoff, won't you agree?

> I think there's also a useful case where the frame tool bar is used to 
> show a "global" tool bar with buttons that do not act on the current 
> buffer (in the current default tool bar: new file, open file, open 
> directory, all the modifier tool bar buttons) and the window tool bar is 
> used to show buttons that act on the buffer.  In this case, you don't 
> want the "global" tool bar to change based on frame's selected window.  
> The "tool-bar-always-show-default" variable I added as well as the logic 
> with ignoring the default value of tool-bar-map was to enable this use 
> case.  I treat the default value of tool-bar-map as "no tool bar buttons 
> for this window" since all those buttons are for the global tool bar.  
> It'd be fine to limit behavior to only when tool-bar-always-show-default 
> was set.

I'm not against tool-bar-always-show-default and its effect.  But
introducing that optional behavior doesn't require any particular
behavior from window-specific tool bars, it's almost an orthogonal
feature.

My conclusion from this is that the two considerations you provided
in favor of a much more complex behavior do not contradict my
suggestion.  The first consideration is about a very rare case, which
we could simply ignore (but if you feel strongly about  detecting
empty tool bars and not displaying them, I won't object), while the
second consideration  does not require the complicated behavior of
window-specific tool bars.

If I missed something, or if you still disagree, please tell what and
why.

Thanks.

Message #160 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Jared Finder <jared <at> finder.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: philipk <at> posteo.net, 68765 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 juri <at> linkov.net
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Tue, 04 Jun 2024 21:22:55 -0700

[Message part 1 (text/plain, inline)]

On 2024-06-04 08:59, Eli Zaretskii wrote:
> I'm not against tool-bar-always-show-default and its effect.  But
> introducing that optional behavior doesn't require any particular
> behavior from window-specific tool bars, it's almost an orthogonal
> feature.
> 
> My conclusion from this is that the two considerations you provided
> in favor of a much more complex behavior do not contradict my
> suggestion.  The first consideration is about a very rare case, which
> we could simply ignore (but if you feel strongly about  detecting
> empty tool bars and not displaying them, I won't object), while the
> second consideration  does not require the complicated behavior of
> window-specific tool bars.

This sounds like a good result.  I do think that auto-hiding the window 
tool bar when tool-bar-map is nil is very valuable and is 
straightforward to implement.

I have attached a new documentation patch describing the intended 
behavior.  If this looks good to you I can draft a patch to 
window-tool-bar.el based on all the feedback thus far.

Thanks

  -- MJF

[0001-Adding-documentation-for-window-tool-bar-bug-68765.patch (text/x-diff, attachment)]

Message #163 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jared Finder <jared <at> finder.org>
Cc: philipk <at> posteo.net, 68765 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 juri <at> linkov.net
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Wed, 05 Jun 2024 15:10:42 +0300

> Date: Tue, 04 Jun 2024 21:22:55 -0700
> From: Jared Finder <jared <at> finder.org>
> Cc: juri <at> linkov.net, 68765 <at> debbugs.gnu.org, philipk <at> posteo.net,
>  monnier <at> iro.umontreal.ca
> 
> On 2024-06-04 08:59, Eli Zaretskii wrote:
> > I'm not against tool-bar-always-show-default and its effect.  But
> > introducing that optional behavior doesn't require any particular
> > behavior from window-specific tool bars, it's almost an orthogonal
> > feature.
> > 
> > My conclusion from this is that the two considerations you provided
> > in favor of a much more complex behavior do not contradict my
> > suggestion.  The first consideration is about a very rare case, which
> > we could simply ignore (but if you feel strongly about  detecting
> > empty tool bars and not displaying them, I won't object), while the
> > second consideration  does not require the complicated behavior of
> > window-specific tool bars.
> 
> This sounds like a good result.  I do think that auto-hiding the window 
> tool bar when tool-bar-map is nil is very valuable and is 
> straightforward to implement.
> 
> I have attached a new documentation patch describing the intended 
> behavior.  If this looks good to you I can draft a patch to 
> window-tool-bar.el based on all the feedback thus far.

This LGTM, with one comment: you mention in the section about window
tool bars that it cannot be shown if a tab line is shown in the
window, but you don't have a similar caveat in the section about tab
lines.  I think we should add a similar caveat there as well.

Message #166 received at 68765 <at> debbugs.gnu.org (full text, mbox):

From: Jared Finder <jared <at> finder.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: philipk <at> posteo.net, 68765 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 juri <at> linkov.net
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Sat, 08 Jun 2024 17:29:15 -0700

[Message part 1 (text/plain, inline)]

On 2024-06-05 05:10, Eli Zaretskii wrote:
>> Date: Tue, 04 Jun 2024 21:22:55 -0700
>> From: Jared Finder <jared <at> finder.org>
>> Cc: juri <at> linkov.net, 68765 <at> debbugs.gnu.org, philipk <at> posteo.net,
>>  monnier <at> iro.umontreal.ca
>> 
>> On 2024-06-04 08:59, Eli Zaretskii wrote:
>> > I'm not against tool-bar-always-show-default and its effect.  But
>> > introducing that optional behavior doesn't require any particular
>> > behavior from window-specific tool bars, it's almost an orthogonal
>> > feature.
>> >
>> > My conclusion from this is that the two considerations you provided
>> > in favor of a much more complex behavior do not contradict my
>> > suggestion.  The first consideration is about a very rare case, which
>> > we could simply ignore (but if you feel strongly about  detecting
>> > empty tool bars and not displaying them, I won't object), while the
>> > second consideration  does not require the complicated behavior of
>> > window-specific tool bars.
>> 
>> This sounds like a good result.  I do think that auto-hiding the 
>> window
>> tool bar when tool-bar-map is nil is very valuable and is
>> straightforward to implement.
>> 
>> I have attached a new documentation patch describing the intended
>> behavior.  If this looks good to you I can draft a patch to
>> window-tool-bar.el based on all the feedback thus far.
> 
> This LGTM, with one comment: you mention in the section about window
> tool bars that it cannot be shown if a tab line is shown in the
> window, but you don't have a similar caveat in the section about tab
> lines.  I think we should add a similar caveat there as well.

Patch attached for updated manual, NEWS, and changes to 
window-tool-bar.el.  This also extends support back to Emacs 27 in the 
package as requested earlier in this thread.

  -- MJF

[0001-Add-documentation-for-window-tool-bar-package.patch (text/x-diff, attachment)]

Message #171 received at 68765-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jared Finder <jared <at> finder.org>
Cc: philipk <at> posteo.net, 68765-done <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 juri <at> linkov.net
Subject: Re: bug#68765: 30.0.50; Adding window-tool-bar package.
Date: Sun, 09 Jun 2024 15:23:18 +0300

> Date: Sat, 08 Jun 2024 17:29:15 -0700
> From: Jared Finder <jared <at> finder.org>
> Cc: juri <at> linkov.net, 68765 <at> debbugs.gnu.org, philipk <at> posteo.net,
>  monnier <at> iro.umontreal.ca
> 
> > This LGTM, with one comment: you mention in the section about window
> > tool bars that it cannot be shown if a tab line is shown in the
> > window, but you don't have a similar caveat in the section about tab
> > lines.  I think we should add a similar caveat there as well.
> 
> Patch attached for updated manual, NEWS, and changes to 
> window-tool-bar.el.  This also extends support back to Emacs 27 in the 
> package as requested earlier in this thread.

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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.