unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#35756: [PATCH] file-size-human-readable: fix glitches and add optional space
@ 2019-05-15 20:02 Mattias Engdegård
  2019-05-17  5:56 ` Eli Zaretskii
  0 siblings, 1 reply; 5+ messages in thread
From: Mattias Engdegård @ 2019-05-15 20:02 UTC (permalink / raw)
  To: 35756

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

The `file-size-human-readable' function is very useful but could do with some better formatting: normally, a space goes between the number and unit; you don't write '3kg' or '25m/s' but '3 kg' and '25 m/s' (sloppy British newspapers notwithstanding). We could add an optional argument so that the caller can use the spacing of preference; the default should probably be no space, for compatibility.

For some reason, only the `iec' mode adds an actual unit (B) to the result; the default and `si' modes just append a scale prefix. Of course a user can append the unit of choice, as in:

  (concat (file-size-human-readable size 'si) "B")

which permits the function to be used for other units than bytes, such as "bit/s" (although the name makes it clear that it is intended for file sizes only). However, spacing complicates things, since we want

  (file-size-human-readable 14 'si " ")

to return "14", not "14 ", but the latter is what we need when appending the unit.
I'm not sure how to fix this. We could add another optional argument, UNIT say, which is the string to use as unit, defaulting to "B" in `iec' mode and the empty string otherwise. The attached patch does not address this.

There is also a small glitch to be fixed:

 (file-size-human-readable 3 'iec) => "3iB"

which of course should be "3B".


[-- Attachment #2: 0001-Optional-space-in-file-size-human-readable.patch --]
[-- Type: application/octet-stream, Size: 9266 bytes --]

From 9b4cce48e39ecf6261c2a2104425c0f276935a13 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Mon, 13 May 2019 17:05:24 +0200
Subject: [PATCH] Optional space in `file-size-human-readable'

To improve readability of strings produced by
`file-size-human-readable', add an optional argument to provide a
string (typically a space or non-breaking space) to put between the
number and unit.  For compatibility, the default is an empty string.

Also fix a glitch with small numbers in `iec' mode which caused a
stray "i" in the result.

* lisp/files.el (file-size-human-readable):
Add optional SPACE argument and handle small numbers correctly.
(files--ask-user-about-large-file, warn-maybe-out-of-memory):
Use the new argument.
* lisp/url/url-http.el (url-http-simple-after-change-function)
(url-http-content-length-after-change-function):
Use the new argument.
* etc/NEWS (Lisp Changes): Mention the change.
---
 etc/NEWS                 |  6 +++++
 lisp/files.el            | 47 +++++++++++++++++++++++++---------------
 lisp/url/url-http.el     | 11 +++++-----
 test/lisp/files-tests.el | 20 +++++++++++++++++
 4 files changed, 61 insertions(+), 23 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 699a04b524..66c1d7883d 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1994,6 +1994,12 @@ case.
 It is a convenient and readable way to specify a regexp that should
 not match anything, and is as fast as any such regexp can be.
 
++++
+** The function 'file-size-human-readable' accepts another optional argument.
+The new third argument is a string put between the number and unit;
+if nil or omitted, the empty string is used.  It is recommended to use
+a single space or non-breaking space for readability.
+
 \f
 * Changes in Emacs 27.1 on Non-Free Operating Systems
 
diff --git a/lisp/files.el b/lisp/files.el
index 8fa7f16de0..7a1bb9fcae 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -1358,7 +1358,7 @@ it means chase no more than that many links and then stop."
 
 ;; A handy function to display file sizes in human-readable form.
 ;; See http://en.wikipedia.org/wiki/Kibibyte for the reference.
-(defun file-size-human-readable (file-size &optional flavor)
+(defun file-size-human-readable (file-size &optional flavor space)
   "Produce a string showing FILE-SIZE in human-readable form.
 
 Optional second argument FLAVOR controls the units and the display format:
@@ -1368,24 +1368,34 @@ Optional second argument FLAVOR controls the units and the display format:
  If FLAVOR is `si', each kilobyte is 1000 bytes and the produced suffixes
     are \"k\", \"M\", \"G\", \"T\", etc.
  If FLAVOR is `iec', each kilobyte is 1024 bytes and the produced suffixes
-    are \"KiB\", \"MiB\", \"GiB\", \"TiB\", etc."
+    are \"KiB\", \"MiB\", \"GiB\", \"TiB\", etc.
+
+Optional third argument SPACE is a string put between the number and unit.
+If nil or omitted, the empty string is used.
+Recommended value is a single space or non-breaking space, unless other
+constraints prohibit a space in that position."
   (let ((power (if (or (null flavor) (eq flavor 'iec))
 		   1024.0
 		 1000.0))
-	(post-fixes
-	 ;; none, kilo, mega, giga, tera, peta, exa, zetta, yotta
-	 (list "" "k" "M" "G" "T" "P" "E" "Z" "Y")))
-    (while (and (>= file-size power) (cdr post-fixes))
+	(prefixes '("" "k" "M" "G" "T" "P" "E" "Z" "Y")))
+    (while (and (>= file-size power) (cdr prefixes))
       (setq file-size (/ file-size power)
-	    post-fixes (cdr post-fixes)))
-    (format (if (> (mod file-size 1.0) 0.05)
-		"%.1f%s%s"
-	      "%.0f%s%s")
-	    file-size
-	    (if (and (eq flavor 'iec) (string= (car post-fixes) "k"))
-		"K"
-	      (car post-fixes))
-	    (if (eq flavor 'iec) "iB" ""))))
+	    prefixes (cdr prefixes)))
+    ;; It's a bit inconsistent that only `iec' includes an actual unit,
+    ;; while the two other flavours just generate a prefix.
+    (let* ((prefix (car prefixes))
+           (unit (if (eq flavor 'iec)
+                     (concat
+                      (if (string= prefix "k") "K" prefix)
+                      (if (string-empty-p prefix) "" "i")
+                      "B")
+                   prefix)))
+      (format (if (> (mod file-size 1.0) 0.05)
+		  "%.1f%s%s"
+	        "%.0f%s%s")
+	      file-size
+              (if (string-empty-p unit) "" (or space ""))
+              unit))))
 
 (defcustom mounted-file-systems
   (if (memq system-type '(windows-nt cygwin))
@@ -2054,7 +2064,7 @@ think it does, because \"free\" is pretty hard to define in practice."
 (defun files--ask-user-about-large-file (size op-type filename offer-raw)
   (let ((prompt (format "File %s is large (%s), really %s?"
 		        (file-name-nondirectory filename)
-		        (file-size-human-readable size) op-type)))
+		        (file-size-human-readable size 'iec " ") op-type)))
     (if (not offer-raw)
         (if (y-or-n-p prompt) nil 'abort)
       (let* ((use-dialog (and (display-popup-menus-p)
@@ -2106,9 +2116,10 @@ returns nil or exits non-locally."
 exceeds the %S%% of currently available free memory (%s).
 If that fails, try to open it with `find-file-literally'
 \(but note that some characters might be displayed incorrectly)."
-	     (file-size-human-readable size)
+	     (file-size-human-readable size 'iec " ")
 	     out-of-memory-warning-percentage
-	     (file-size-human-readable (* total-free-memory 1024)))))))))
+	     (file-size-human-readable (* total-free-memory 1024)
+                                       'iec " "))))))))
 
 (defun files--message (format &rest args)
   "Like `message', except sometimes don't print to minibuffer.
diff --git a/lisp/url/url-http.el b/lisp/url/url-http.el
index 48e29987a5..d5d44190e1 100644
--- a/lisp/url/url-http.el
+++ b/lisp/url/url-http.el
@@ -999,7 +999,8 @@ should be shown to the user."
 (defun url-http-simple-after-change-function (_st _nd _length)
   ;; Function used when we do NOT know how long the document is going to be
   ;; Just _very_ simple 'downloaded %d' type of info.
-  (url-lazy-message "Reading %s..." (file-size-human-readable (buffer-size))))
+  (url-lazy-message "Reading %s..."
+                    (file-size-human-readable (buffer-size) 'iec " ")))
 
 (defun url-http-content-length-after-change-function (_st nd _length)
   "Function used when we DO know how long the document is going to be.
@@ -1012,16 +1013,16 @@ the callback to be triggered."
        (url-percentage (- nd url-http-end-of-headers)
 		       url-http-content-length)
        url-http-content-type
-       (file-size-human-readable (- nd url-http-end-of-headers))
-       (file-size-human-readable url-http-content-length)
+       (file-size-human-readable (- nd url-http-end-of-headers) 'iec " ")
+       (file-size-human-readable url-http-content-length 'iec " ")
        (url-percentage (- nd url-http-end-of-headers)
 		       url-http-content-length))
     (url-display-percentage
      "Reading... %s of %s (%d%%)"
      (url-percentage (- nd url-http-end-of-headers)
 		     url-http-content-length)
-     (file-size-human-readable (- nd url-http-end-of-headers))
-     (file-size-human-readable url-http-content-length)
+     (file-size-human-readable (- nd url-http-end-of-headers) 'iec " ")
+     (file-size-human-readable url-http-content-length 'iec " ")
      (url-percentage (- nd url-http-end-of-headers)
 		     url-http-content-length)))
 
diff --git a/test/lisp/files-tests.el b/test/lisp/files-tests.el
index fe2e958f1c..b000c3070d 100644
--- a/test/lisp/files-tests.el
+++ b/test/lisp/files-tests.el
@@ -1259,5 +1259,25 @@ renaming only, rather than modified in-place."
       (ignore-errors (advice-remove #'write-region advice))
       (ignore-errors (delete-file temp-file-name)))))
 
+(ert-deftest files-test-file-size-human-readable ()
+  (should (equal (file-size-human-readable 13) "13"))
+  (should (equal (file-size-human-readable 13 'si) "13"))
+  (should (equal (file-size-human-readable 13 'iec) "13B"))
+  (should (equal (file-size-human-readable 10000) "9.8k"))
+  (should (equal (file-size-human-readable 10000 'si) "10k"))
+  (should (equal (file-size-human-readable 10000 'iec) "9.8KiB"))
+  (should (equal (file-size-human-readable 4294967296 nil) "4G"))
+  (should (equal (file-size-human-readable 4294967296 'si) "4.3G"))
+  (should (equal (file-size-human-readable 4294967296 'iec) "4GiB"))
+  (should (equal (file-size-human-readable 13 nil " ") "13"))
+  (should (equal (file-size-human-readable 13 'si " ") "13"))
+  (should (equal (file-size-human-readable 13 'iec " ") "13 B"))
+  (should (equal (file-size-human-readable 10000 nil " ") "9.8 k"))
+  (should (equal (file-size-human-readable 10000 'si " ") "10 k"))
+  (should (equal (file-size-human-readable 10000 'iec " ") "9.8 KiB"))
+  (should (equal (file-size-human-readable 4294967296 nil " ") "4 G"))
+  (should (equal (file-size-human-readable 4294967296 'si " ") "4.3 G"))
+  (should (equal (file-size-human-readable 4294967296 'iec " ") "4 GiB")))
+
 (provide 'files-tests)
 ;;; files-tests.el ends here
-- 
2.20.1 (Apple Git-117)


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-06-23 18:29 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-05-15 20:02 bug#35756: [PATCH] file-size-human-readable: fix glitches and add optional space Mattias Engdegård
2019-05-17  5:56 ` Eli Zaretskii
2019-05-17 11:01   ` Mattias Engdegård
2019-06-23 16:56     ` Lars Ingebrigtsen
2019-06-23 18:29       ` Mattias Engdegård

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).