Re: [Emacs-diffs] master fd54102: * lisp/files.el (file-size-function): New defcustom

[Eli Zaretskii]

> From: "Basil L. Contovounesios" <address@hidden>
> Date: Sat, 20 Jul 2019 17:53:13 +0100
> Cc: Oleh Krehel <address@hidden>
> 
> address@hidden (Oleh Krehel) writes:
> 
> > branch: master
> > commit fd5410217ff23810edc16e97c10934ad622f8e4b
> > Author: Oleh Krehel <address@hidden>
> > Commit: Oleh Krehel <address@hidden>
> >
> >     * lisp/files.el (file-size-function): New defcustom
> 
> [I wish changes like this (and its recent predecessors, which caused
>  test failues that others had to fix), even if small, to central
>  user-facing features were instead discussed and tested a little before
>  being unilaterally pushed.]

Seconded.

> > +(defcustom file-size-function #'file-size-human-readable
> > +  "Function that transforms the number of bytes into a human-readable 
> > string."
> 
> The number of which bytes?  I think a phrase similar to "for display"
> would be more accurate than "human-readable" here.
> 
> > +  :type '(choice
> 
> Did you try radio+function-item instead of choice+const?  I usually find
> the former nicer, as recommended in (info "(elisp) Composite Types").
> 
> > +          (const :tag "default" file-size-human-readable)
> > +          (const :tag "iec"
> 
> Nit: Please capitalise and uppercase these tags, respectively.
> 
> > +           (lambda (size) (file-size-human-readable size 'iec " ")))
> 
> Please do not use an unevaluated anonymous function here.
> 
> > +          (function :tag "Custom function")))

This defcustom also needs a :version tag.