bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Eli Zaretskii]

Some functions in tab-bar.el and the data they return are documented
insufficiently, so much so that it makes the code there very hard to
develop and maintain by anyone except the original author.  Internal
functions and data strictures don't need to have doc strings, but they
do have to be explained enough for anyone to understand and modify the
code without the need to step through it with a debugger.

(Also, quite a few commands there lacked doc strings, which should be
avoided at all costs.  I fixed that today.)

I show below the places where IMO the lack of proper documentation is
particularly evident, or where the documentation is insufficient or
inaccurate.  Please fill at least those gaps; bonus points for
documenting more than this bare minimum.

  (defun tab-bar--key-to-number (key)

This function needs at least to document the meaning of each value it
can return: nil, t, or a number.

  (defun tab-bar--event-to-item (posn)
    (if (posn-window posn)
	(let ((caption (car (posn-string posn))))
	  (when caption
	    (get-text-property 0 'menu-item caption)))

This function should document the possible return values and their
meaning.  It should also say something about the text property it
retrieves from posn-string, and how that property is used.

  (defun tab-bar--format-tab (tab i)
    "Format TAB using its index I and return the result as a string."

the doc string of this function is at least inaccurate, if not
incorrect: it doesn't (always) return a string.  Please document what
it does return and the meaning of the various forms of the value.

  (defun tab-bar--format-tab-group (tab i &optional current-p)

This function lacks any documentation of the value it returns; please
add some minimal docs.

  (defun tab-bar-format-tabs-groups ()
    "Show tabs with their groups."

The doc string says "show", but this functions doesn't display
anything, AFAICT, it produces a list.  Please adjust the doc string
and add a description of the returned value.

  (defun tab-bar--tab (&optional frame)

Please add some minimal documentation of the return value.

  (defun tab-bar--current-tab (&optional tab frame)

  (defun tab-bar--current-tab-make (&optional tab)

  (defun tab-bar--current-tab-find (&optional tabs frame)

  (defun tab-bar--current-tab-index (&optional tabs frame)

  (defun tab-bar--tab-index (tab &optional tabs frame)

  (defun tab-bar--tab-index-by-name (name &optional tabs frame)

  (defun tab-bar--tab-index-recent (nth &optional tabs frame)

  (defun tab-bar--tabs-recent (&optional tabs frame)

These functions need at least some comment saying what each one of
them does.

  (defun tab-switcher-delete-from-list (tab)
    "Delete the window configuration from both lists."

Which "both lists"?

  (defun switch-to-buffer-other-tab (buffer-or-name &optional norecord)
    "Switch to buffer BUFFER-OR-NAME in another tab.
  Like \\[switch-to-buffer-other-frame] (which see), but creates a new tab.
  Interactively, prompt for the buffer to switch to."

This command should document the NORECORD argument.

Thanks.


In GNU Emacs 28.0.60 (build 72, i686-pc-mingw32)
 of 2021-10-17 built on HOME-C4E4A596F7
Repository revision: 35920791df78400a36bf4420584bd8349ce9bbee
Repository branch: emacs-28
Windowing system distributor 'Microsoft Corp.', version 5.1.2600
System Description: Microsoft Windows XP Service Pack 3 (v5.1.0.2600)

Configured using:
 'configure -C --prefix=/d/usr --with-wide-int --with-modules
 --enable-checking=yes,glyphs 'CFLAGS=-O0 -gdwarf-4 -g3''

Configured features:
ACL GIF GMP GNUTLS HARFBUZZ JPEG JSON LCMS2 LIBXML2 MODULES NOTIFY
W32NOTIFY PDUMPER PNG RSVG SOUND THREADS TIFF TOOLKIT_SCROLL_BARS XPM
ZLIB

Important settings:
  value of $LANG: ENU
  locale-coding-system: cp1255

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
rfc822 mml mml-sec epa derived epg rfc6068 epg-config gnus-util rmail
rmail-loaddefs auth-source cl-seq eieio eieio-core cl-macs
eieio-loaddefs password-cache json map text-property-search time-date
subr-x seq byte-opt gv bytecomp byte-compile cconv mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader cl-loaddefs
cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
iso-transl tooltip eldoc paren electric uniquify ediff-hook vc-hooks
lisp-float-type elisp-mode mwheel dos-w32 ls-lisp disp-table
term/w32-win w32-win w32-vars term/common-win tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch easymenu
timer select scroll-bar mouse jit-lock font-lock syntax font-core
term/tty-colors frame minibuffer cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese composite emoji-zwj charscript charprop case-table
epa-hook jka-cmpr-hook help simple abbrev obarray cl-preloaded nadvice
button loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote threads w32notify w32 lcms2 multi-tty
make-network-process emacs)

Memory information:
((conses 16 56715 8028)
 (symbols 48 7801 1)
 (strings 16 21649 1976)
 (string-bytes 1 632631)
 (vectors 16 13631)
 (vector-slots 8 180039 9875)
 (floats 8 23 59)
 (intervals 40 266 93)
 (buffers 888 10))

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Juri Linkov]

> Some functions in tab-bar.el and the data they return are documented
> insufficiently, so much so that it makes the code there very hard to
> develop and maintain by anyone except the original author.  Internal
> functions and data strictures don't need to have doc strings, but they
> do have to be explained enough for anyone to understand and modify the
> code without the need to step through it with a debugger.

It was on my TODO list to add docstrings to new functions
after their names and a list of arguments stabilizes.
For example, there was a need to rename some functions now in bug#51246.

> I show below the places where IMO the lack of proper documentation is
> particularly evident, or where the documentation is insufficient or
> inaccurate.  Please fill at least those gaps; bonus points for
> documenting more than this bare minimum.

I fixed all places that you noticed (please check), except the following:

>   (defun tab-bar-format-tabs-groups ()
>     "Show tabs with their groups."
>
> The doc string says "show", but this functions doesn't display
> anything, AFAICT, it produces a list.  Please adjust the doc string
> and add a description of the returned value.

This docstring is automatically displayed in the Customization buffer
of `tab-bar-format' that looks like this:

  Hide Tab Bar Format:
  Hook:
  [ ] tab-bar-format-menu-global
          Show global menu on clicking the Menu button.
  [X] tab-bar-format-history
          Show back and forward buttons when ‘tab-bar-history-mode’ is enabled. More
  [ ] tab-bar-format-tabs
          Show all tabs.
  [X] tab-bar-format-tabs-groups
          Show tabs with their groups.
  [ ] tab-bar-separator
          Separator between tabs.
  [ ] tab-bar-format-add-tab
          Button to add a new tab.
  [X] tab-bar-format-align-right
          Align the rest of tab bar items to the right.
  [X] tab-bar-format-global
          Format ‘global-mode-string’ to display it in the tab bar. More
  [INS]

So here its docstring explains what this option is useful for.

>   (defun switch-to-buffer-other-tab (buffer-or-name &optional norecord)
>     "Switch to buffer BUFFER-OR-NAME in another tab.
>   Like \\[switch-to-buffer-other-frame] (which see), but creates a new tab.
>   Interactively, prompt for the buffer to switch to."
>
> This command should document the NORECORD argument.

Oops, this was copy/paste from switch-to-buffer, switch-to-buffer-other-window,
switch-to-buffer-other-frame that all just pass NORECORD down to 'pop-to-buffer'.
But switch-to-buffer-other-tab doesn't use 'pop-to-buffer'.  It uses 'display-buffer'
that has no NORECORD arg.  So this arg should be obsoleted, or maybe deleted right away.

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 51247@debbugs.gnu.org
> Date: Sun, 17 Oct 2021 20:17:43 +0300
> 
> > I show below the places where IMO the lack of proper documentation is
> > particularly evident, or where the documentation is insufficient or
> > inaccurate.  Please fill at least those gaps; bonus points for
> > documenting more than this bare minimum.
> 
> I fixed all places that you noticed (please check)

Thanks.  A few questions/suggestions below:

   (defun tab-bar--key-to-number (key)
  +  "This function is used to interpret the key that represents a tab.
  +It returns `t' for the `nil' value, `nil' for the current tab,
  +returns the number for the symbol that begins with `tab-' like `tab-1',
  +and `t' for other values."

I'd rewrite as follows:

  (defun tab-bar--key-to-number (key)
    "Return the tab number represented by KEY.
  If KEY is a symbol 'tab-N', where N is a tab number, the value is N.
  If KEY is \\='current-tab, the value is nil.
  For any other value of KET, the value is t."

Is this correct?

   (defun tab-bar--event-to-item (posn)
  +  "This function extracts extra info from the mouse event POSN.
  +It returns a list that contains three elements: a key,
  +a key binding, and a boolean value whether the close button \"+\"
  +was clicked."

I'd rewrite as follows:

  (defun tab-bar--event-to-item (posn)
    "This function extracts extra info from the mouse event at position POSN.
  It returns a list of the form (KEY KEY-BINDING CLOSE-P), where:
   KEY is a symbol representing a tab, such as \\='tab-1 or \\='current-tab;
   KEY-BINDING is the binding of KEY;
   CLOSE-P is non-nil if the mouse event was a click on the close button \"x\",
     nil otherwise."

Is this correct?

   (defun tab-bar--format-tab-group (tab i &optional current-p)
  +  "Format TAB as a tab that represents a group of tabs.
  +Use the argument I as its index

Whose index is "its index" here?

   (defun tab-bar--current-tab-make (&optional tab)
  -  ;; `tab' here is an argument meaning "use tab as template".  This is
  -  ;; necessary when switching tabs, otherwise the destination tab
  -  ;; inherits the current tab's `explicit-name' parameter.
  +  "Make the current tab data structure from TAB.
  +TAB here is an argument meaning \"use tab as template\".  This is
  +necessary when switching tabs, otherwise the destination tab
  +inherits the current tab's `explicit-name' parameter."

I don't think I understand what do you mean by "use tab as template",
can you explain?

> except the following:
> 
> >   (defun tab-bar-format-tabs-groups ()
> >     "Show tabs with their groups."
> >
> > The doc string says "show", but this functions doesn't display
> > anything, AFAICT, it produces a list.  Please adjust the doc string
> > and add a description of the returned value.
> 
> This docstring is automatically displayed in the Customization buffer
> of `tab-bar-format' that looks like this:
> 
>   Hide Tab Bar Format:
>   Hook:
>   [ ] tab-bar-format-menu-global
>           Show global menu on clicking the Menu button.
>   [X] tab-bar-format-history
>           Show back and forward buttons when ‘tab-bar-history-mode’ is enabled. More
>   [ ] tab-bar-format-tabs
>           Show all tabs.
>   [X] tab-bar-format-tabs-groups
>           Show tabs with their groups.
>   [ ] tab-bar-separator
>           Separator between tabs.
>   [ ] tab-bar-format-add-tab
>           Button to add a new tab.
>   [X] tab-bar-format-align-right
>           Align the rest of tab bar items to the right.
>   [X] tab-bar-format-global
>           Format ‘global-mode-string’ to display it in the tab bar. More
>   [INS]
> 
> So here its docstring explains what this option is useful for.

Can't you provide a separate text for the Custom display?  The doc
string in its current form is simply misleading.

> >   (defun switch-to-buffer-other-tab (buffer-or-name &optional norecord)
> >     "Switch to buffer BUFFER-OR-NAME in another tab.
> >   Like \\[switch-to-buffer-other-frame] (which see), but creates a new tab.
> >   Interactively, prompt for the buffer to switch to."
> >
> > This command should document the NORECORD argument.
> 
> Oops, this was copy/paste from switch-to-buffer, switch-to-buffer-other-window,
> switch-to-buffer-other-frame that all just pass NORECORD down to 'pop-to-buffer'.
> But switch-to-buffer-other-tab doesn't use 'pop-to-buffer'.  It uses 'display-buffer'
> that has no NORECORD arg.  So this arg should be obsoleted, or maybe deleted right away.

Fine with me, please use advertised-calling-convention or somesuch to
hide that argument.

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Juri Linkov]

>    (defun tab-bar--current-tab-make (&optional tab)
>   -  ;; `tab' here is an argument meaning "use tab as template".  This is
>   -  ;; necessary when switching tabs, otherwise the destination tab
>   -  ;; inherits the current tab's `explicit-name' parameter.
>   +  "Make the current tab data structure from TAB.
>   +TAB here is an argument meaning \"use tab as template\".  This is
>   +necessary when switching tabs, otherwise the destination tab
>   +inherits the current tab's `explicit-name' parameter."
>
> I don't think I understand what do you mean by "use tab as template",
> can you explain?

Actually, "use tab as template" was not my text, but I improved it anyway,
together with all your suggestions.

>>   [ ] tab-bar-format-tabs
>>           Show all tabs.
>>   [X] tab-bar-format-tabs-groups
>>           Show tabs with their groups.
>>   [ ] tab-bar-separator
>>           Separator between tabs.
>>   [ ] tab-bar-format-add-tab
>>           Button to add a new tab.
>>   [X] tab-bar-format-align-right
>>           Align the rest of tab bar items to the right.
>>   [X] tab-bar-format-global
>>           Format ‘global-mode-string’ to display it in the tab bar. More
>>   [INS]
>>
>> So here its docstring explains what this option is useful for.
>
> Can't you provide a separate text for the Custom display?  The doc
> string in its current form is simply misleading.

It's a feature of :options in defcustom that it takes the text
from the docstrings.  Maybe it's possible to override this text,
but I currently don't know how.

>> >   (defun switch-to-buffer-other-tab (buffer-or-name &optional norecord)
>> >     "Switch to buffer BUFFER-OR-NAME in another tab.
>> >   Like \\[switch-to-buffer-other-frame] (which see), but creates a new tab.
>> >   Interactively, prompt for the buffer to switch to."
>> >
>> > This command should document the NORECORD argument.
>>
>> Oops, this was copy/paste from switch-to-buffer, switch-to-buffer-other-window,
>> switch-to-buffer-other-frame that all just pass NORECORD down to 'pop-to-buffer'.
>> But switch-to-buffer-other-tab doesn't use 'pop-to-buffer'.  It uses 'display-buffer'
>> that has no NORECORD arg.  So this arg should be obsoleted, or maybe deleted right away.
>
> Fine with me, please use advertised-calling-convention or somesuch to
> hide that argument.

Done.

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 51247@debbugs.gnu.org
> Date: Sun, 17 Oct 2021 21:29:21 +0300
> 
> >>   [ ] tab-bar-format-tabs
> >>           Show all tabs.
> >>   [X] tab-bar-format-tabs-groups
> >>           Show tabs with their groups.
> >>   [ ] tab-bar-separator
> >>           Separator between tabs.
> >>   [ ] tab-bar-format-add-tab
> >>           Button to add a new tab.
> >>   [X] tab-bar-format-align-right
> >>           Align the rest of tab bar items to the right.
> >>   [X] tab-bar-format-global
> >>           Format ‘global-mode-string’ to display it in the tab bar. More
> >>   [INS]
> >>
> >> So here its docstring explains what this option is useful for.
> >
> > Can't you provide a separate text for the Custom display?  The doc
> > string in its current form is simply misleading.
> 
> It's a feature of :options in defcustom that it takes the text
> from the docstrings.  Maybe it's possible to override this text,
> but I currently don't know how.

OK, how about the patch below?

diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el
index 82ec617..3dc95c9 100644
--- a/lisp/tab-bar.el
+++ b/lisp/tab-bar.el
@@ -705,11 +705,14 @@ tab-bar-format
   "Template for displaying tab bar items.
 Every item in the list is a function that returns
 a string, or a list of menu-item elements, or nil.
-When you add more items `tab-bar-format-align-right' and
-`tab-bar-format-global' to the end, then after enabling
-`display-time-mode' (or any other mode that uses `global-mode-string')
-it will display time aligned to the right on the tab bar instead of
-the mode line.  Replacing `tab-bar-format-tabs' with
+Adding a function to the list causes the tab bar to show
+that string, or display a menu with those menu items when
+you click on the tab bar.
+If the list ends with `tab-bar-format-align-right' and
+`tab-bar-format-global', then after enabling `display-time-mode'
+(or any other mode that uses `global-mode-string'),
+it will display time aligned to the right on the tab bar instead
+of the mode line.  Replacing `tab-bar-format-tabs' with
 `tab-bar-format-tabs-groups' will group tabs on the tab bar."
   :type 'hook
   :options '(tab-bar-format-menu-global
@@ -728,7 +731,7 @@ tab-bar-format
   :version "28.1")
 
 (defun tab-bar-format-menu-global ()
-  "Show global menu on clicking the Menu button."
+  "Produce the Menu button for the tab bar that shows a global menu."
   `((add-tab menu-item (propertize "Menu" 'face 'tab-bar-tab-inactive)
              (lambda (event) (interactive "e")
                (let ((menu (make-sparse-keymap
@@ -745,7 +748,8 @@ tab-bar-format-menu-global
              :help "Global Menu")))
 
 (defun tab-bar-format-history ()
-  "Show back and forward buttons when `tab-bar-history-mode' is enabled.
+  "Produce back and forward buttons for the tab bar.
+These buttons will be shown when `tab-bar-history-mode' is enabled.
 You can hide these buttons by customizing `tab-bar-format' and removing
 `tab-bar-format-history' from it."
   (when tab-bar-history-mode
@@ -781,7 +785,7 @@ tab-bar--format-tab
         ,(alist-get 'close-binding tab))))))
 
 (defun tab-bar-format-tabs ()
-  "Show all tabs."
+  "Produce all the tabs for the tab bar."
   (let ((i 0))
     (mapcan
      (lambda (tab)
@@ -855,7 +859,7 @@ tab-bar--format-tab-group
       :help "Click to visit group"))))
 
 (defun tab-bar-format-tabs-groups ()
-  "Show tabs with their groups."
+  "Produce tabs for the tab bar grouped according to their groups."
   (let* ((tabs (funcall tab-bar-tabs-function))
          (current-group (funcall tab-bar-tab-group-function
                                  (tab-bar--current-tab-find tabs)))
@@ -899,7 +903,7 @@ tab-bar-format-align-right
     `((align-right menu-item ,str ignore))))
 
 (defun tab-bar-format-global ()
-  "Format `global-mode-string' to display it in the tab bar.
+  "Produce display of `global-mode-string' in the tab bar.
 When `tab-bar-format-global' is added to `tab-bar-format'
 (possibly appended after `tab-bar-format-align-right'),
 then modes that display information on the mode line

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Juri Linkov]

>> It's a feature of :options in defcustom that it takes the text
>> from the docstrings.  Maybe it's possible to override this text,
>> but I currently don't know how.
>
> OK, how about the patch below?

Everything is right, except this sentence:

> @@ -705,11 +705,14 @@ tab-bar-format
> +Adding a function to the list causes the tab bar to show
> +that string, or display a menu with those menu items when
> +you click on the tab bar.

Since menu items are used to display tabs, something like this
would be more correct:

  Adding a function to the list causes the tab bar to show
  that string, or display tabs represented by menu items
  on the tab bar.

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 51247@debbugs.gnu.org
> Date: Sun, 17 Oct 2021 22:27:43 +0300
> 
> >> It's a feature of :options in defcustom that it takes the text
> >> from the docstrings.  Maybe it's possible to override this text,
> >> but I currently don't know how.
> >
> > OK, how about the patch below?
> 
> Everything is right, except this sentence:
> 
> > @@ -705,11 +705,14 @@ tab-bar-format
> > +Adding a function to the list causes the tab bar to show
> > +that string, or display a menu with those menu items when
> > +you click on the tab bar.
> 
> Since menu items are used to display tabs, something like this
> would be more correct:
> 
>   Adding a function to the list causes the tab bar to show
>   that string, or display tabs represented by menu items
>   on the tab bar.

But that's not what I see.  What I see is a single tab called "Menu",
which, when clicked, shows the global menu.

Or what do you mean by "tabs represented by menu items"?  How can a
tab be represented by a menu item?

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Juri Linkov]

>> Everything is right, except this sentence:
>>
>> > @@ -705,11 +705,14 @@ tab-bar-format
>> > +Adding a function to the list causes the tab bar to show
>> > +that string, or display a menu with those menu items when
>> > +you click on the tab bar.
>>
>> Since menu items are used to display tabs, something like this
>> would be more correct:
>>
>>   Adding a function to the list causes the tab bar to show
>>   that string, or display tabs represented by menu items
>>   on the tab bar.
>
> But that's not what I see.  What I see is a single tab called "Menu",
> which, when clicked, shows the global menu.

The menu from the tab called "Menu" is displayed with 'popup-menu'.

> Or what do you mean by "tabs represented by menu items"?  How can a
> tab be represented by a menu item?

Menu items are transformed to tab strings in parse_tab_bar_item.

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 51247@debbugs.gnu.org
> Date: Sun, 17 Oct 2021 22:48:37 +0300
> 
> >> Everything is right, except this sentence:
> >>
> >> > @@ -705,11 +705,14 @@ tab-bar-format
> >> > +Adding a function to the list causes the tab bar to show
> >> > +that string, or display a menu with those menu items when
> >> > +you click on the tab bar.
> >>
> >> Since menu items are used to display tabs, something like this
> >> would be more correct:
> >>
> >>   Adding a function to the list causes the tab bar to show
> >>   that string, or display tabs represented by menu items
> >>   on the tab bar.
> >
> > But that's not what I see.  What I see is a single tab called "Menu",
> > which, when clicked, shows the global menu.
> 
> The menu from the tab called "Menu" is displayed with 'popup-menu'.
> 
> > Or what do you mean by "tabs represented by menu items"?  How can a
> > tab be represented by a menu item?
> 
> Menu items are transformed to tab strings in parse_tab_bar_item.

Sorry, I'm still confused.  Again, checking this option:

>   [ ] tab-bar-format-menu-global
>           Show global menu on clicking the Menu button.

causes the tab bar to include a single tab called "Menu", and if I
click on that tab, I see the items from the global menu-bar.

In any case, I don't see the importance of the fine difference between
"menu items" and "menu items that represent tabs" in this case.
AFAIU, whatever menu is produced by that option is what's displayed on
the tab bar; the items in the menu are entirely up to the function
which produces the menu.  Isn't that right?

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Juri Linkov]

>> The menu from the tab called "Menu" is displayed with 'popup-menu'.
>>
>> > Or what do you mean by "tabs represented by menu items"?  How can a
>> > tab be represented by a menu item?
>>
>> Menu items are transformed to tab strings in parse_tab_bar_item.
>
> Sorry, I'm still confused.  Again, checking this option:
>
>>   [ ] tab-bar-format-menu-global
>>           Show global menu on clicking the Menu button.
>
> causes the tab bar to include a single tab called "Menu", and if I
> click on that tab, I see the items from the global menu-bar.
>
> In any case, I don't see the importance of the fine difference between
> "menu items" and "menu items that represent tabs" in this case.
> AFAIU, whatever menu is produced by that option is what's displayed on
> the tab bar; the items in the menu are entirely up to the function
> which produces the menu.  Isn't that right?

This text is still wrong:

  Adding a function to the list causes the tab bar to show
  that string, or display a menu with those menu items when
  you click on the tab bar.

The tab called "Menu" displays menu items from the global menu-bar,
not from those menu items mentioned in the doc string.
Those menu items are used only to display tabs.

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 51247@debbugs.gnu.org
> Date: Mon, 18 Oct 2021 18:53:40 +0300
> 
> > In any case, I don't see the importance of the fine difference between
> > "menu items" and "menu items that represent tabs" in this case.
> > AFAIU, whatever menu is produced by that option is what's displayed on
> > the tab bar; the items in the menu are entirely up to the function
> > which produces the menu.  Isn't that right?
> 
> This text is still wrong:
> 
>   Adding a function to the list causes the tab bar to show
>   that string, or display a menu with those menu items when
>   you click on the tab bar.
> 
> The tab called "Menu" displays menu items from the global menu-bar,
> not from those menu items mentioned in the doc string.
> Those menu items are used only to display tabs.

Then please point me to a function out of those mentioned in
tab-bar-format's :options that returns a list of menu items.  The
sentence in the doc string "Every item in the list is a function that
returns a string, or a list of menu-item elements, or nil" wasn't
mine, it was yours.  What did you have in mind when you wrote that?

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Juri Linkov]

>> This text is still wrong:
>>
>>   Adding a function to the list causes the tab bar to show
>>   that string, or display a menu with those menu items when
>>   you click on the tab bar.
>>
>> The tab called "Menu" displays menu items from the global menu-bar,
>> not from those menu items mentioned in the doc string.
>> Those menu items are used only to display tabs.
>
> Then please point me to a function out of those mentioned in
> tab-bar-format's :options that returns a list of menu items.  The
> sentence in the doc string "Every item in the list is a function that
> returns a string, or a list of menu-item elements, or nil" wasn't
> mine, it was yours.  What did you have in mind when you wrote that?

- tab-bar-format-menu-global returns a list of menu items
  that display the "Menu" tab.  This tab is bound to a command
  that uses popup-menu to display the global menu on clicking.

- tab-bar-format-history returns a list of menu items
  that produce back and forward buttons.

- tab-bar-format-tabs and tab-bar-format-tabs-groups
  return a list of menu items that produce the tabs for the tab bar.
...

- Only tab-bar-separator returns a string.

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 51247@debbugs.gnu.org
> Date: Mon, 18 Oct 2021 19:44:31 +0300
> 
> >> This text is still wrong:
> >>
> >>   Adding a function to the list causes the tab bar to show
> >>   that string, or display a menu with those menu items when
> >>   you click on the tab bar.
> >>
> >> The tab called "Menu" displays menu items from the global menu-bar,
> >> not from those menu items mentioned in the doc string.
> >> Those menu items are used only to display tabs.
> >
> > Then please point me to a function out of those mentioned in
> > tab-bar-format's :options that returns a list of menu items.  The
> > sentence in the doc string "Every item in the list is a function that
> > returns a string, or a list of menu-item elements, or nil" wasn't
> > mine, it was yours.  What did you have in mind when you wrote that?
> 
> - tab-bar-format-menu-global returns a list of menu items
>   that display the "Menu" tab.  This tab is bound to a command
>   that uses popup-menu to display the global menu on clicking.
> 
> - tab-bar-format-history returns a list of menu items
>   that produce back and forward buttons.
> 
> - tab-bar-format-tabs and tab-bar-format-tabs-groups
>   return a list of menu items that produce the tabs for the tab bar.
> ...
> 
> - Only tab-bar-separator returns a string.

Then the doc string after my changes is exactly right.  I have no idea
what you see that's wrong with it.  You say some functions return menu
items and some return a list of menu items, and that's what the doc
string says.  You say that these values are displayed on the tab bar,
and that's what the doc string says.  So what exactly is wrong??

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Juri Linkov]

> Then the doc string after my changes is exactly right.  I have no idea
> what you see that's wrong with it.  You say some functions return menu
> items and some return a list of menu items, and that's what the doc
> string says.  You say that these values are displayed on the tab bar,
> and that's what the doc string says.  So what exactly is wrong??

  Adding a function to the list causes the tab bar to show
  that string, or display a menu with those menu items when
  you click on the tab bar.

It says that only when you click on the tab bar,
those menu items are used to display some menu.
But in fact those menu items are used to display
tabs or buttons, without clicking.
This is more correct:

  Adding a function to the list causes the tab bar to show that string,
  or display tabs or buttons created from those menu items.

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Eli Zaretskii]

> Date: Mon, 18 Oct 2021 19:53:10 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 51247@debbugs.gnu.org
> 
> Then the doc string after my changes is exactly right.  I have no idea
> what you see that's wrong with it.  You say some functions return menu
> items and some return a list of menu items, and that's what the doc
> string says.  You say that these values are displayed on the tab bar,
> and that's what the doc string says.  So what exactly is wrong??

Or let me turn the table and ask you to explain to a potential Lisp
programmer what to expect from adding a function to the list in
tab-bar-format.  Suppose J.R. Hacker wants to write a function
suitable for inclusion in this list, and suppose the function returns
a string or a list of menu items -- what should J.R. Hacker expect
from putting that function in this list? how will the tab bar display
the value returned by the function?

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Juri Linkov]

> Or let me turn the table and ask you to explain to a potential Lisp
> programmer what to expect from adding a function to the list in
> tab-bar-format.  Suppose J.R. Hacker wants to write a function
> suitable for inclusion in this list, and suppose the function returns
> a string or a list of menu items -- what should J.R. Hacker expect
> from putting that function in this list? how will the tab bar display
> the value returned by the function?

It will display tabs or buttons from these menu items.

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 51247@debbugs.gnu.org
> Date: Mon, 18 Oct 2021 20:02:21 +0300
> 
> > Or let me turn the table and ask you to explain to a potential Lisp
> > programmer what to expect from adding a function to the list in
> > tab-bar-format.  Suppose J.R. Hacker wants to write a function
> > suitable for inclusion in this list, and suppose the function returns
> > a string or a list of menu items -- what should J.R. Hacker expect
> > from putting that function in this list? how will the tab bar display
> > the value returned by the function?
> 
> It will display tabs or buttons from these menu items.

When will it be tabs and when will it be buttons?  And to which of
these does "menu" belong?

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Juri Linkov]

>> It will display tabs or buttons from these menu items.
>
> When will it be tabs and when will it be buttons?

Tabs are buttons that switch window configurations.

> And to which of these does "menu" belong?

"Menu" should not be mentioned at all, it's
some unimportant detail from one non-default option.

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 51247@debbugs.gnu.org
> Date: Mon, 18 Oct 2021 20:21:48 +0300
> 
> >> It will display tabs or buttons from these menu items.
> >
> > When will it be tabs and when will it be buttons?
> 
> Tabs are buttons that switch window configurations.

But how do I explain in the doc string to the future programmers what
they should do to get tabs and what they should do to get buttons that
drop a menu?  Presumably, each of these needs a list of menu items of
different forms, but how should they be different?  How does the tab
bar know what to display given the menu items returned by these
functions?

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Juri Linkov]

>> >> It will display tabs or buttons from these menu items.
>> >
>> > When will it be tabs and when will it be buttons?
>>
>> Tabs are buttons that switch window configurations.
>
> But how do I explain in the doc string to the future programmers what
> they should do to get tabs and what they should do to get buttons that
> drop a menu?

You don't need to explain about buttons that drop a menu.
This is irrelevant for the description of tab-bar-format.
Just imagine there is no tab-bar-format-menu-global
that uses own unrelated menu items.

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 51247@debbugs.gnu.org
> Date: Tue, 19 Oct 2021 09:49:59 +0300
> 
> >> >> It will display tabs or buttons from these menu items.
> >> >
> >> > When will it be tabs and when will it be buttons?
> >>
> >> Tabs are buttons that switch window configurations.
> >
> > But how do I explain in the doc string to the future programmers what
> > they should do to get tabs and what they should do to get buttons that
> > drop a menu?
> 
> You don't need to explain about buttons that drop a menu.
> This is irrelevant for the description of tab-bar-format.
> Just imagine there is no tab-bar-format-menu-global
> that uses own unrelated menu items.

Sorry, that won't fly.  The doc string should explain enough for other
Lisp programmers to be able to produce buttons that drop down menus.

So please humor me with a detailed enough answer to my questions, I'm
investing enough energy in this stuff to be entitled to a bit more
than a couple of dismissive sentences.

We must have a decent doc string for this defcustom.  Please help me
fix what we have now.

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Juri Linkov]

>> You don't need to explain about buttons that drop a menu.
>> This is irrelevant for the description of tab-bar-format.
>> Just imagine there is no tab-bar-format-menu-global
>> that uses own unrelated menu items.
>
> Sorry, that won't fly.  The doc string should explain enough for other
> Lisp programmers to be able to produce buttons that drop down menus.

This is the whole point.  The doc string of this variable is not an
appropriate place to teach Lisp programmers how to produce buttons
that drop down menus.  Or you might as well include the whole
Emacs Lisp Reference Manual in this doc string.

> So please humor me with a detailed enough answer to my questions, I'm
> investing enough energy in this stuff to be entitled to a bit more
> than a couple of dismissive sentences.
>
> We must have a decent doc string for this defcustom.  Please help me
> fix what we have now.

You assumed in the doc string that those menu items as used
to display a menu, whereas in fact those menu items as used
to display tabs and other buttons on the tab bar.

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 51247@debbugs.gnu.org
> Date: Tue, 19 Oct 2021 22:09:27 +0300
> 
> >> You don't need to explain about buttons that drop a menu.
> >> This is irrelevant for the description of tab-bar-format.
> >> Just imagine there is no tab-bar-format-menu-global
> >> that uses own unrelated menu items.
> >
> > Sorry, that won't fly.  The doc string should explain enough for other
> > Lisp programmers to be able to produce buttons that drop down menus.
> 
> This is the whole point.  The doc string of this variable is not an
> appropriate place to teach Lisp programmers how to produce buttons
> that drop down menus.

That's not what I meant.  I meant to explain to people who already
know to produce menus how to format those menus in these two different
manners for two different effects.

> Or you might as well include the whole Emacs Lisp Reference Manual
> in this doc string.

This is not constructive and not helpful, let alone uncalled-for.

> > So please humor me with a detailed enough answer to my questions, I'm
> > investing enough energy in this stuff to be entitled to a bit more
> > than a couple of dismissive sentences.
> >
> > We must have a decent doc string for this defcustom.  Please help me
> > fix what we have now.
> 
> You assumed in the doc string that those menu items as used
> to display a menu, whereas in fact those menu items as used
> to display tabs and other buttons on the tab bar.

I'm trying to understand the difference between the two kinds of
menus, the one which causes the tab bar to display tabs, vs the one
that causes the display of a single tab that drops down a menu.
Please help me understand that difference.

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Juri Linkov]

>> You assumed in the doc string that those menu items as used
>> to display a menu, whereas in fact those menu items as used
>> to display tabs and other buttons on the tab bar.
>
> I'm trying to understand the difference between the two kinds of
> menus, the one which causes the tab bar to display tabs, vs the one
> that causes the display of a single tab that drops down a menu.
> Please help me understand that difference.

The tab bar is created from the list where elements are either a string,
or a menu-item with a title and a binding.  The title is displayed as
a button on the tab bar.  Clicking on such button calls the binding.
The binding usually is a command that switches tabs.  But the binding
can do everything.

To make the distinction more clear, I created a new command
`tab-bar-menu-bar' that is bound to the button created by
`tab-bar-format-menu-bar'.

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 51247@debbugs.gnu.org
> Date: Wed, 20 Oct 2021 19:50:53 +0300
> 
> The tab bar is created from the list where elements are either a string,
> or a menu-item with a title and a binding.  The title is displayed as
> a button on the tab bar.  Clicking on such button calls the binding.
> The binding usually is a command that switches tabs.  But the binding
> can do everything.

Thanks.  What if the function in tab-bar-format returns nil -- how is
that handled?

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Juri Linkov]

>> The tab bar is created from the list where elements are either a string,
>> or a menu-item with a title and a binding.  The title is displayed as
>> a button on the tab bar.  Clicking on such button calls the binding.
>> The binding usually is a command that switches tabs.  But the binding
>> can do everything.
>
> Thanks.  What if the function in tab-bar-format returns nil -- how is
> that handled?

Interesting question, I tried this

  (add-hook 'tab-bar-format 'ignore 1)

and everything still works fine.

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 51247@debbugs.gnu.org
> Date: Thu, 21 Oct 2021 19:43:34 +0300
> 
> > Thanks.  What if the function in tab-bar-format returns nil -- how is
> > that handled?
> 
> Interesting question, I tried this
> 
>   (add-hook 'tab-bar-format 'ignore 1)
> 
> and everything still works fine.

I'm afraid this doesn't help me.  The doc strings says:

  Every item in the list is a function that returns
  a string, or a list of menu-item elements, or nil.

I now have a good idea what to say about return values that are
strings and those that are lists of menu items.  What do I say about
return values of nil? what is their effect on the tab bar's
appearance?  Are you saying that there's no effect whatsoever?

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Juri Linkov]

>>   (add-hook 'tab-bar-format 'ignore 1)
>>
>> and everything still works fine.
>
> I'm afraid this doesn't help me.  The doc strings says:
>
>   Every item in the list is a function that returns
>   a string, or a list of menu-item elements, or nil.
>
> I now have a good idea what to say about return values that are
> strings and those that are lists of menu items.  What do I say about
> return values of nil? what is their effect on the tab bar's
> appearance?  Are you saying that there's no effect whatsoever?

The nil value is supported only for the case when a function
is on the tab-bar-format list, but its return value depends
on some variable, and on some conditions might return nil.

An example is tab-bar-format-history: when tab-bar-history-mode
is nil, then tab-bar-format-history returns nil, but it doesn't break
the tab bar.

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 51247@debbugs.gnu.org
> Date: Thu, 21 Oct 2021 20:27:03 +0300
> 
> >>   (add-hook 'tab-bar-format 'ignore 1)
> >>
> >> and everything still works fine.
> >
> > I'm afraid this doesn't help me.  The doc strings says:
> >
> >   Every item in the list is a function that returns
> >   a string, or a list of menu-item elements, or nil.
> >
> > I now have a good idea what to say about return values that are
> > strings and those that are lists of menu items.  What do I say about
> > return values of nil? what is their effect on the tab bar's
> > appearance?  Are you saying that there's no effect whatsoever?
> 
> The nil value is supported only for the case when a function
> is on the tab-bar-format list, but its return value depends
> on some variable, and on some conditions might return nil.
> 
> An example is tab-bar-format-history: when tab-bar-history-mode
> is nil, then tab-bar-format-history returns nil, but it doesn't break
> the tab bar.

OK, but what I want to say is what effect do such functions have on
the tab bar appearance when they return nil?  It sounds like you say
that the appearance is not affected in any way in that case?

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Juri Linkov]

>> The nil value is supported only for the case when a function
>> is on the tab-bar-format list, but its return value depends
>> on some variable, and on some conditions might return nil.
>>
>> An example is tab-bar-format-history: when tab-bar-history-mode
>> is nil, then tab-bar-format-history returns nil, but it doesn't break
>> the tab bar.
>
> OK, but what I want to say is what effect do such functions have on
> the tab bar appearance when they return nil?  It sounds like you say
> that the appearance is not affected in any way in that case?

Exactly, they have no effect.

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 51247@debbugs.gnu.org
> Date: Thu, 21 Oct 2021 20:48:47 +0300
> 
> >> The nil value is supported only for the case when a function
> >> is on the tab-bar-format list, but its return value depends
> >> on some variable, and on some conditions might return nil.
> >>
> >> An example is tab-bar-format-history: when tab-bar-history-mode
> >> is nil, then tab-bar-format-history returns nil, but it doesn't break
> >> the tab bar.
> >
> > OK, but what I want to say is what effect do such functions have on
> > the tab bar appearance when they return nil?  It sounds like you say
> > that the appearance is not affected in any way in that case?
> 
> Exactly, they have no effect.

Thanks.

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[martin rudalics]

 > Some functions in tab-bar.el and the data they return are documented
 > insufficiently, so much so that it makes the code there very hard to
 > develop and maintain by anyone except the original author.

I now added a few tab bar and tab line related elements to the Elisp
manual.  Please have a look and tell me if anything else should be added
or done.

Some reshuffling (especially that of the "Selecting Window" section)
will be inconvenient to trace.  Apologies for that but I think the
general accessibility of some of the contained terms is hopefully better
now.

Thanks, martin

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Eli Zaretskii]

> Cc: Juri Linkov <juri@linkov.net>
> From: martin rudalics <rudalics@gmx.at>
> Date: Mon, 18 Oct 2021 10:21:40 +0200
> 
>  > Some functions in tab-bar.el and the data they return are documented
>  > insufficiently, so much so that it makes the code there very hard to
>  > develop and maintain by anyone except the original author.
> 
> I now added a few tab bar and tab line related elements to the Elisp
> manual.  Please have a look and tell me if anything else should be added
> or done.

Thanks.  Some comments/questions:

> +@item Tab Bar
> +@cindex internal tab bar

Why "internal"? copy/pasta?

You use the term "minibuffer window", which I think is sub-optimal:
that window is also used for displaying the echo-area.  I prefer the
term "mini-window" instead.

> +Note that the window returned by @code{minibuffer-window} called with
> +the argument @var{frame} is returned by @code{window-list} called with
> +the same argument if and only if that window actually belongs to
> +@var{frame}.

This is a mouthful (is it really important to say this here?).  Please
rephrase, in particular with fewer instances of the passive tense.
(The next sentence after that is also unnecessarily passive and hard
to read.)

I made a few minor editing changes in the text you installed.

Thanks for taking care of the manual.

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[martin rudalics]

 > Thanks.  Some comments/questions:
 >
 >> +@item Tab Bar
 >> +@cindex internal tab bar
 >
 > Why "internal"? copy/pasta?

Intentional but silly.  Fixed.

 > You use the term "minibuffer window", which I think is sub-optimal:
 > that window is also used for displaying the echo-area.  I prefer the
 > term "mini-window" instead.

That ship has sailed somehow.  We have an entire section called
"Minibuffer Windows".

 >> +Note that the window returned by @code{minibuffer-window} called with
 >> +the argument @var{frame} is returned by @code{window-list} called with
 >> +the same argument if and only if that window actually belongs to
 >> +@var{frame}.
 >
 > This is a mouthful (is it really important to say this here?).  Please
 > rephrase, in particular with fewer instances of the passive tense.
 > (The next sentence after that is also unnecessarily passive and hard
 > to read.)

I removed that paragraph.  Honestly, it would have taken me too much
time to understand it myself.

 > I made a few minor editing changes in the text you installed.

Thanks.  This one

-  A @dfn{window} describes a portion of the screen that Emacs uses to
+  A @dfn{window} describes the portion of the screen that Emacs uses to

is not really correct because with multiple windows we have several such
portions but maybe it's more intuitive for the reader.

Thanks, martin

bug#51247: 28.0.60; Insufficient documentation of tab-bar.el internal functions

[Eli Zaretskii]

> Cc: 51247@debbugs.gnu.org, juri@linkov.net
> From: martin rudalics <rudalics@gmx.at>
> Date: Mon, 18 Oct 2021 19:43:45 +0200
> 
> -  A @dfn{window} describes a portion of the screen that Emacs uses to
> +  A @dfn{window} describes the portion of the screen that Emacs uses to
> 
> is not really correct because with multiple windows we have several such
> portions but maybe it's more intuitive for the reader.

My English is not really native, but AFAIK once you describe a thing,
it should be "the thing", not "a thing".  Let's hear from natives,
though.