bug#59438: 29.0.50; [PATCH] Make defvar's into defcustom's in tab-line

bug#59438: 29.0.50; [PATCH] Make defvar's into defcustom's in tab-line

[Gabriel]

[-- Attachment #1: Type: text/plain, Size: 755 bytes --]

Description:

Please find attached a patch that proposes the following minor changes
to tab-line.  I can refactor the patch if we decide that some change are
not worth or if it's better to split it into smaller patches.

1) Turn the following defvar's into defcustom's:

- `tab-line-tabs-buffer-group-function'
- `tab-line-tabs-buffer-group-sort-function'
- `tab-line-tabs-buffer-groups-sort-function'

2) Add conditional around function call of
`tab-line-tabs-buffer-groups-sort-function', in
`tab-line-tabs-buffer-groups'.  This guards against nil values, similar
to how `tab-line-tabs-buffer-group-sort-function is handled, making the
behavior more consistent across these two (similar) variables.

Should we announce these "new" defcustom's in NEWS?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-defvar-s-into-defcustom-s-in-tab-line.patch --]
[-- Type: text/x-diff, Size: 4966 bytes --]

From da1053e7211d8bb17769fe43650a9bcdd359aeab Mon Sep 17 00:00:00 2001
From: Gabriel do Nascimento Ribeiro <gabriel376@hotmail.com>
Date: Mon, 21 Nov 2022 06:17:49 -0300
Subject: [PATCH 1/1] Make defvar's into defcustom's in tab-line

* lisp/tab-line.el (tab-line-tabs-buffer-group-function)
  (tab-line-tabs-buffer-group-sort-function)
  (tab-line-tabs-buffer-groups-sort-function): Make into defcustom's.
  (Make defvar's into defcustom's in tab-line): Handle case when
  variable tab-line-tabs-buffer-groups-sort-function is nil.
---
 lisp/tab-line.el | 51 ++++++++++++++++++++++++++++--------------------
 1 file changed, 30 insertions(+), 21 deletions(-)

diff --git a/lisp/tab-line.el b/lisp/tab-line.el
index 99a785ee3e..9b1c101718 100644
--- a/lisp/tab-line.el
+++ b/lisp/tab-line.el
@@ -341,16 +341,26 @@ tab-line-tabs-mode-buffers
                                            (derived-mode-p mode)))
                              (funcall tab-line-tabs-buffer-list-function)))))
 
-(defvar tab-line-tabs-buffer-group-function nil
+(defcustom tab-line-tabs-buffer-group-function nil
   "Function to add a buffer to the appropriate group of tabs.
-Takes a buffer as arg and should return a group name as a string.
-If the return value is nil, the buffer should be filtered out.")
+Takes a buffer as argument and should return a group name as a
+string.  If the return value is nil, the buffer should be
+ filtered out."
+  :type 'function
+  :group 'tab-line
+  :version "29.1")
 
-(defvar tab-line-tabs-buffer-group-sort-function nil
-  "Function to sort buffers in a group.")
+(defcustom tab-line-tabs-buffer-group-sort-function nil
+  "Function to sort buffers in a group."
+  :type 'function
+  :group 'tab-line
+  :version "29.1")
 
-(defvar tab-line-tabs-buffer-groups-sort-function #'string<
-  "Function to sort group names.")
+(defcustom tab-line-tabs-buffer-groups-sort-function #'string<
+  "Function to sort group names."
+  :type 'function
+  :group 'tab-line
+  :version "29.1")
 
 (defvar tab-line-tabs-buffer-groups mouse-buffer-menu-mode-groups
   "How to group various major modes together in the tab line.
@@ -378,13 +388,14 @@ tab-line-tabs-buffer-groups
 generate the group name."
   (if (window-parameter nil 'tab-line-groups)
       (let* ((buffers (funcall tab-line-tabs-buffer-list-function))
-             (groups
-              (seq-sort tab-line-tabs-buffer-groups-sort-function
-                        (delq nil (mapcar #'car (seq-group-by
-                                                 (lambda (buffer)
-                                                   (tab-line-tabs-buffer-group-name
-                                                    buffer))
-                                                 buffers)))))
+             (groups (delq nil
+                           (mapcar #'car
+                                   (seq-group-by #'tab-line-tabs-buffer-group-name
+                                                 buffers))))
+             (sorted-groups (if (functionp tab-line-tabs-buffer-groups-sort-function)
+                                (seq-sort tab-line-tabs-buffer-groups-sort-function
+                                          groups)
+                              groups))
              (selected-group (window-parameter nil 'tab-line-group))
              (tabs
               (mapcar (lambda (group)
@@ -395,9 +406,8 @@ tab-line-tabs-buffer-groups
                                        (set-window-parameter nil 'tab-line-groups nil)
                                        (set-window-parameter nil 'tab-line-group group)
                                        (set-window-parameter nil 'tab-line-hscroll nil)))))
-                      groups)))
-        tabs)
-
+                      sorted-groups)))
+             tabs)
     (let* ((window-parameter (window-parameter nil 'tab-line-group))
            (group-name (tab-line-tabs-buffer-group-name (current-buffer)))
            (group (prog1 (or window-parameter group-name "All")
@@ -410,10 +420,9 @@ tab-line-tabs-buffer-groups
                                      (set-window-parameter nil 'tab-line-groups t)
                                      (set-window-parameter nil 'tab-line-group group)
                                      (set-window-parameter nil 'tab-line-hscroll nil)))))
-           (buffers
-            (seq-filter (lambda (b)
-                          (equal (tab-line-tabs-buffer-group-name b) group))
-                        (funcall tab-line-tabs-buffer-list-function)))
+           (buffers (seq-filter (lambda (b)
+                                  (equal (tab-line-tabs-buffer-group-name b) group))
+                                (funcall tab-line-tabs-buffer-list-function)))
            (sorted-buffers (if (functionp tab-line-tabs-buffer-group-sort-function)
                                (seq-sort tab-line-tabs-buffer-group-sort-function
                                          buffers)
-- 
2.34.1


[-- Attachment #3: Type: text/plain, Size: 13 bytes --]


---
Gabriel

bug#59438: 29.0.50; [PATCH] Make defvar's into defcustom's in tab-line

[Stefan Kangas]

Gabriel <gabriel376@hotmail.com> writes:

> Please find attached a patch that proposes the following minor changes
> to tab-line.  I can refactor the patch if we decide that some change are
> not worth or if it's better to split it into smaller patches.
>
> 1) Turn the following defvar's into defcustom's:
>
> - `tab-line-tabs-buffer-group-function'
> - `tab-line-tabs-buffer-group-sort-function'
> - `tab-line-tabs-buffer-groups-sort-function'
>
> 2) Add conditional around function call of
> `tab-line-tabs-buffer-groups-sort-function', in
> `tab-line-tabs-buffer-groups'.  This guards against nil values, similar
> to how `tab-line-tabs-buffer-group-sort-function is handled, making the
> behavior more consistent across these two (similar) variables.
>
> Should we announce these "new" defcustom's in NEWS?

Juri, thoughts?

bug#59438: 29.0.50; [PATCH] Make defvar's into defcustom's in tab-line

[Stefan Kangas]

Stefan Kangas <stefankangas@gmail.com> writes:

> Gabriel <gabriel376@hotmail.com> writes:
>
>> Please find attached a patch that proposes the following minor changes
>> to tab-line.  I can refactor the patch if we decide that some change are
>> not worth or if it's better to split it into smaller patches.
>>
>> 1) Turn the following defvar's into defcustom's:
>>
>> - `tab-line-tabs-buffer-group-function'
>> - `tab-line-tabs-buffer-group-sort-function'
>> - `tab-line-tabs-buffer-groups-sort-function'
>>
>> 2) Add conditional around function call of
>> `tab-line-tabs-buffer-groups-sort-function', in
>> `tab-line-tabs-buffer-groups'.  This guards against nil values, similar
>> to how `tab-line-tabs-buffer-group-sort-function is handled, making the
>> behavior more consistent across these two (similar) variables.
>>
>> Should we announce these "new" defcustom's in NEWS?
>
> Juri, thoughts?

Ping.  Does anyone have any comments?

bug#59438: 29.0.50; [PATCH] Make defvar's into defcustom's in tab-line

[Mauro Aranda]

On 7/9/23 18:07, Stefan Kangas wrote:
> Stefan Kangas <stefankangas@gmail.com> writes:
> 
>> Gabriel <gabriel376@hotmail.com> writes:
>>
>>> Please find attached a patch that proposes the following minor changes
>>> to tab-line.  I can refactor the patch if we decide that some change are
>>> not worth or if it's better to split it into smaller patches.
>>>
>>> 1) Turn the following defvar's into defcustom's:
>>>
>>> - `tab-line-tabs-buffer-group-function'
>>> - `tab-line-tabs-buffer-group-sort-function'
>>> - `tab-line-tabs-buffer-groups-sort-function'
>>>
>>> 2) Add conditional around function call of
>>> `tab-line-tabs-buffer-groups-sort-function', in
>>> `tab-line-tabs-buffer-groups'.  This guards against nil values, similar
>>> to how `tab-line-tabs-buffer-group-sort-function is handled, making the
>>> behavior more consistent across these two (similar) variables.
>>>
>>> Should we announce these "new" defcustom's in NEWS?
>>
>> Juri, thoughts?
> 
> Ping.  Does anyone have any comments?

I do have minor comments about the defcustom changes.


Gabriel <gabriel376@hotmail.com> writes:

 >>From da1053e7211d8bb17769fe43650a9bcdd359aeab Mon Sep 17 00:00:00 2001
 > From: Gabriel do Nascimento Ribeiro <gabriel376@hotmail.com>
 > Date: Mon, 21 Nov 2022 06:17:49 -0300
 > Subject: [PATCH 1/1] Make defvar's into defcustom's in tab-line
 >
 > * lisp/tab-line.el (tab-line-tabs-buffer-group-function)
 >   (tab-line-tabs-buffer-group-sort-function)
 >   (tab-line-tabs-buffer-groups-sort-function): Make into defcustom's.
 >   (Make defvar's into defcustom's in tab-line): Handle case when
 >   variable tab-line-tabs-buffer-groups-sort-function is nil.
 > ---
 >  lisp/tab-line.el | 51 ++++++++++++++++++++++++++++--------------------
 >  1 file changed, 30 insertions(+), 21 deletions(-)
 >
 > diff --git a/lisp/tab-line.el b/lisp/tab-line.el
 > index 99a785ee3e..9b1c101718 100644
 > --- a/lisp/tab-line.el
 > +++ b/lisp/tab-line.el
 > @@ -341,16 +341,26 @@ tab-line-tabs-mode-buffers
 >                                             (derived-mode-p mode)))
 >                               (funcall 
tab-line-tabs-buffer-list-function)))))
 >
 > -(defvar tab-line-tabs-buffer-group-function nil
 > +(defcustom tab-line-tabs-buffer-group-function nil
 >    "Function to add a buffer to the appropriate group of tabs.
 > -Takes a buffer as arg and should return a group name as a string.
 > -If the return value is nil, the buffer should be filtered out.")
 > +Takes a buffer as argument and should return a group name as a
 > +string.  If the return value is nil, the buffer should be
 > + filtered out."
 > +  :type 'function
 > +  :group 'tab-line
 > +  :version "29.1")

Since the option can be nil, please add a choice type here.

 > -(defvar tab-line-tabs-buffer-group-sort-function nil
 > -  "Function to sort buffers in a group.")
 > +(defcustom tab-line-tabs-buffer-group-sort-function nil
 > +  "Function to sort buffers in a group."
 > +  :type 'function
 > +  :group 'tab-line
 > +  :version "29.1")

Same here.

 > -(defvar tab-line-tabs-buffer-groups-sort-function #'string<
 > -  "Function to sort group names.")
 > +(defcustom tab-line-tabs-buffer-groups-sort-function #'string<
 > +  "Function to sort group names."
 > +  :type 'function
 > +  :group 'tab-line
 > +  :version "29.1")

And version should be updated, I guess.

bug#59438: 29.0.50; [PATCH] Make defvar's into defcustom's in tab-line

[Juri Linkov]

close 59438 30.0.50
thanks

> Description:
>
> Please find attached a patch that proposes the following minor changes
> to tab-line.  I can refactor the patch if we decide that some change are
> not worth or if it's better to split it into smaller patches.
>
> 1) Turn the following defvar's into defcustom's:
>
> - `tab-line-tabs-buffer-group-function'
> - `tab-line-tabs-buffer-group-sort-function'
> - `tab-line-tabs-buffer-groups-sort-function'
>
> 2) Add conditional around function call of
> `tab-line-tabs-buffer-groups-sort-function', in
> `tab-line-tabs-buffer-groups'.  This guards against nil values, similar
> to how `tab-line-tabs-buffer-group-sort-function is handled, making the
> behavior more consistent across these two (similar) variables.

I'm very sorry this took so long since I wanted first to do some related changes.
But your patch will get into the next release anyway.  Thanks for the patch.

So now everything is pushed together with the suggestion by Mauro
to add choice type with nil.