emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Oleh Krehel
Subject: Re: [Emacs-diffs] master fd54102: * lisp/files.el (file-size-function): New defcustom
Date: Sat, 20 Jul 2019 19:38:28 +0200

Hi all,

> > [I wish changes like this (and its recent predecessors, which caused
> >  test failues that others had to fix)

Sorry about that. I didn't realize where the tests were, since there's
no "make test" at top-level.

> > even if small, to central
> >  user-facing features were instead discussed and tested a little before
> >  being unilaterally pushed.]
>
> Seconded.

While I agree in general, in this case the user-visible change was
done around a year ago, and I noticed only now when I switched to
using Emacs27. The change from a year ago was displaying the amount of
free space in kilobytes, which is almost unusable.
I added a custom var with a reasonable default that any user can
easily change. Very uncontroversial change, IMO.

> > Did you try radio+function-item instead of choice+const?  I usually find
> > the former nicer, as recommended in (info "(elisp) Composite Types").

OK.

> > > +          (const :tag "default" file-size-human-readable)
> > > +          (const :tag "iec"
> >
> > Nit: Please capitalise and uppercase these tags, respectively.

OK.

> > > +           (lambda (size) (file-size-human-readable size 'iec " ")))
> >
> > Please do not use an unevaluated anonymous function here.

OK.

> This defcustom also needs a :version tag.

OK. I've pushed the requested changes.

regards,
Oleh



reply via email to

[Prev in Thread] Current Thread [Next in Thread]