bug#47252: 28.0.50; [PATCH] Make minibuffer-depth-indicator-function a defcustom

bug#47252: 28.0.50; [PATCH] Make minibuffer-depth-indicator-function a defcustom

[Gabriel]

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

This simple patch makes 'minibuffer-depth-indicator-function' a
defcustom.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-lisp-mb-depth.el-Make-minibuffer-depth-indicator-fun.patch --]
[-- Type: text/x-diff, Size: 1026 bytes --]

From 6edb731766fa7fb34b62b3b1f9906d8c2119cb1c Mon Sep 17 00:00:00 2001
From: Gabriel do Nascimento Ribeiro <gabriel.nascimento@nubank.com.br>
Date: Fri, 19 Mar 2021 00:09:43 -0300
Subject: [PATCH] * lisp/mb-depth.el: Make minibuffer-depth-indicator-function
 a defcustom

(minibuffer-depth-indicator-function): Make a defcustom.
---
 lisp/mb-depth.el | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lisp/mb-depth.el b/lisp/mb-depth.el
index f9a24e34bf..71b4abb5cb 100644
--- a/lisp/mb-depth.el
+++ b/lisp/mb-depth.el
@@ -30,10 +30,12 @@
 
 ;;; Code:
 
-(defvar minibuffer-depth-indicator-function nil
+(defcustom minibuffer-depth-indicator-function nil
   "If non-nil, function to set up the minibuffer depth indicator.
 It is called with one argument, the minibuffer depth,
-and must return a string.")
+and must return a string."
+  :type 'function
+  :group 'minibuffer)
 
 (defface minibuffer-depth-indicator '((t :inherit highlight))
   "Face to use for minibuffer depth indicator."
-- 
2.27.0


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


There was a long discussion 14 years ago when mb-depth.el was
implemented about making it a defcustom or not:

[1] https://lists.gnu.org/archive/html/emacs-devel/2008-08/msg00025.html
[2] https://lists.gnu.org/archive/html/emacs-devel/2007-06/msg00304.html

These are the only references I could find about this topic. Perhaps
today the rationale for this decision is different and this patch can be
applied. If the decision is to keep it as a defvar, please close this
bug report.

Regards,
Gabriel

bug#47252: 28.0.50; [PATCH] Make minibuffer-depth-indicator-function a defcustom

[Lars Ingebrigtsen]

Gabriel <gabriel376@hotmail.com> writes:

> This simple patch makes 'minibuffer-depth-indicator-function' a
> defcustom.

Thanks; applied to Emacs 28.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#47252: 28.0.50; [PATCH] Make minibuffer-depth-indicator-function a defcustom

[Mauro Aranda]

Gabriel <gabriel376@hotmail.com> writes:

> This simple patch makes 'minibuffer-depth-indicator-function' a
> defcustom.

Since the default value is nil, I made a change after Lars pushed your
patch to include nil as a supported value; else, the Customize interface
reports a mismatch (and I think we do have tests to check for this?)

bug#47252: 28.0.50; [PATCH] Make minibuffer-depth-indicator-function a defcustom

[Lars Ingebrigtsen]

Mauro Aranda <maurooaranda@gmail.com> writes:

> Gabriel <gabriel376@hotmail.com> writes:
>
>> This simple patch makes 'minibuffer-depth-indicator-function' a
>> defcustom.
>
> Since the default value is nil, I made a change after Lars pushed your
> patch to include nil as a supported value; else, the Customize interface
> reports a mismatch 

Thanks.  But shouldn't

+  :type '(choice (const :tag "Default" nil)
+                 (function))

be

+  :type '(choice (const :tag "Default" nil)
+                 function)

?

> (and I think we do have tests to check for this?)

Yes, what's the test for this again?  

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#47252: 28.0.50; [PATCH] Make minibuffer-depth-indicator-function a defcustom

[Mauro Aranda]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Mauro Aranda <maurooaranda@gmail.com> writes:
>
>> Gabriel <gabriel376@hotmail.com> writes:
>>
>>> This simple patch makes 'minibuffer-depth-indicator-function' a
>>> defcustom.
>>
>> Since the default value is nil, I made a change after Lars pushed your
>> patch to include nil as a supported value; else, the Customize interface
>> reports a mismatch 
>
> Thanks.  But shouldn't
>
> +  :type '(choice (const :tag "Default" nil)
> +                 (function))
>
> be
>
> +  :type '(choice (const :tag "Default" nil)
> +                 function)
>
> ?

It's just a matter of style, I think.  If you prefer the latter, please
fix it.  I'm just used to write the available choices in a list, because
usually they have a :tag or a different default value.

>> (and I think we do have tests to check for this?)
>
> Yes, what's the test for this again?  

They were moved in:

commit 2790c6a572a905359c60f055c682b28ef5c8ff0d
Author: Stefan Kangas <stefan@marxist.se>
Date:   Fri Feb 19 12:31:56 2021 +0100

    Run admin/cus-tests.el tests from test suite
    
    * test/Makefile.in (SUBDIRS): Run tests in new directory "misc",
    intended for tests not belonging to any one file.
    * test/misc/test-custom-deps.el:
    * test/misc/test-custom-libs.el:
    * test/misc/test-custom-noloads.el:
    * test/misc/test-custom-opts.el: New files.
    * test/lisp/custom-tests.el (custom--test-local-option): Move test to
    above new file test-custom-opts.el.
    
    * admin/cus-test.el: Document running tests from regular test suite.
    * test/file-organization.org (Test Files): Document new test directory
    "misc" for tests not belonging to any one file.

They are easy to "misc" ;-)

I wonder if it would be useful to tweak the checks in admin/cus-test.el
to be able to run the defcustom tests for each individual file.  Then,

make foo-tests

will run the tests in foo-tests.el and then check the defcustom in
library foo.

bug#47252: 28.0.50; [PATCH] Make minibuffer-depth-indicator-function a defcustom

[Robert Pluim]

>>>>> On Fri, 19 Mar 2021 10:31:36 +0100, Lars Ingebrigtsen <larsi@gnus.org> said:

    Lars> Mauro Aranda <maurooaranda@gmail.com> writes:
    >> Gabriel <gabriel376@hotmail.com> writes:
    >> 
    >>> This simple patch makes 'minibuffer-depth-indicator-function' a
    >>> defcustom.
    >> 
    >> Since the default value is nil, I made a change after Lars pushed your
    >> patch to include nil as a supported value; else, the Customize interface
    >> reports a mismatch 

    Lars> Thanks.  But shouldn't

    Lars> +  :type '(choice (const :tag "Default" nil)
    Lars> +                 (function))

    Lars> be

    Lars> +  :type '(choice (const :tag "Default" nil)
    Lars> +                 function)

    Lars> ?

    >> (and I think we do have tests to check for this?)

    Lars> Yes, what's the test for this again?  

admin/cus-test.el

Robert
-- 

bug#47252: 28.0.50; [PATCH] Make minibuffer-depth-indicator-function a defcustom

[Lars Ingebrigtsen]

Mauro Aranda <maurooaranda@gmail.com> writes:

> They are easy to "misc" ;-)

Right; so you say "cd check; make test-custom-opts" etc to run them..

> I wonder if it would be useful to tweak the checks in admin/cus-test.el
> to be able to run the defcustom tests for each individual file.  Then,
>
> make foo-tests
>
> will run the tests in foo-tests.el and then check the defcustom in
> library foo.

Makes sense to me.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no