bug#35756: [PATCH] file-size-human-readable: fix glitches and add optional space

bug#35756: [PATCH] file-size-human-readable: fix glitches and add optional space

[Mattias Engdegård]

[-- 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)

bug#35756: [PATCH] file-size-human-readable: fix glitches and add optional space

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Wed, 15 May 2019 22:02:50 +0200
> 
> 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.

I have no opinion regarding the change, but I have a minor comment
about the documentation:

> +** 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.

This uses the passive tense too much.  The "if nil or omitted, the
empty string is used" part could be worded more clearly as "it
defaults to the empty string".  The "It is recommended" part is better
worded as "We recommend".

> +Optional third argument SPACE is a string put between the number and unit.
> +If nil or omitted, the empty string is used.

Same here.

Thanks.

bug#35756: [PATCH] file-size-human-readable: fix glitches and add optional space

[Mattias Engdegård]

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

17 maj 2019 kl. 07.56 skrev Eli Zaretskii <eliz@gnu.org>:
> 
> This uses the passive tense too much.  The "if nil or omitted, the
> empty string is used" part could be worded more clearly as "it
> defaults to the empty string".  The "It is recommended" part is better
> worded as "We recommend".

Thank you, fixed.

>> +Optional third argument SPACE is a string put between the number and unit.
>> +If nil or omitted, the empty string is used.
> 
> Same here.

Fixed.

I also added the optional argument UNIT to settle that problem. New patch attached.


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

From 403ce6c70dbacf3a99785b730e026fca67ce9a4e 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 and unit in `file-size-human-readable'
 (bug#35756)

To improve readability of strings produced by
`file-size-human-readable', add two optional arguments:

- SPACE, 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.

- UNIT, a string to use as unit.  For compatibility, the default is
"B" in `iec' mode and the empty string otherwise.

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 and UNIT arguments and handle small numbers correctly.
(files--ask-user-about-large-file, warn-maybe-out-of-memory):
Use the new argument.
* test/lisp/files-tests.el (files-test-file-size-human-readable): New test.
* 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                 |  9 ++++++++
 lisp/files.el            | 49 +++++++++++++++++++++++++---------------
 lisp/url/url-http.el     | 11 +++++----
 test/lisp/files-tests.el | 23 +++++++++++++++++++
 4 files changed, 69 insertions(+), 23 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 8c059157ba..215d652a64 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -2005,6 +2005,15 @@ 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 more optional arguments.
+The new third argument is a string put between the number and unit; it
+defaults to the empty string.  The new fourth argument is a string
+representing the unit to use; it defaults to "B" when the second
+argument is 'iec' and the empty string otherwise.  We recomment a
+space or non-breaking space as third argument, and "B" as fourth
+argument, circumstances allowing.
+
 \f
 * Changes in Emacs 27.1 on Non-Free Operating Systems
 
diff --git a/lisp/files.el b/lisp/files.el
index 8fa7f16de0..e84775c6ac 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 unit)
   "Produce a string showing FILE-SIZE in human-readable form.
 
 Optional second argument FLAVOR controls the units and the display format:
@@ -1368,24 +1368,36 @@ 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.
+It defaults to the empty string.  We recommend a single space or
+non-breaking space, unless other constraints prohibit a space in that
+position.
+
+Optional fourth argument UNIT is the unit to use.  It defaults to \"B\"
+when FLAVOR is `iec' and the empty string otherwise.  We recommend \"B\"
+in all cases, since that is the standard symbol for byte."
   (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)))
+    (let* ((prefix (car prefixes))
+           (prefixed-unit (if (eq flavor 'iec)
+                              (concat
+                               (if (string= prefix "k") "K" prefix)
+                               (if (string-empty-p prefix) "" "i")
+                               (or unit "B"))
+                            (concat prefix unit))))
+      (format (if (> (mod file-size 1.0) 0.05)
+		  "%.1f%s%s"
+	        "%.0f%s%s")
+	      file-size
+              (if (string-empty-p prefixed-unit) "" (or space ""))
+              prefixed-unit))))
 
 (defcustom mounted-file-systems
   (if (memq system-type '(windows-nt cygwin))
@@ -2054,7 +2066,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 +2118,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..ebdfc12d5d 100644
--- a/test/lisp/files-tests.el
+++ b/test/lisp/files-tests.el
@@ -1259,5 +1259,28 @@ 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"))
+  (should (equal (file-size-human-readable 10000 nil " " "B") "9.8 kB"))
+  (should (equal (file-size-human-readable 10000 'si " " "B") "10 kB"))
+  (should (equal (file-size-human-readable 10000 'iec " " "bit") "9.8 Kibit")))
+
 (provide 'files-tests)
 ;;; files-tests.el ends here
-- 
2.20.1 (Apple Git-117)

bug#35756: [PATCH] file-size-human-readable: fix glitches and add optional space

[Lars Ingebrigtsen]

Mattias Engdegård <mattiase@acm.org> writes:

> I also added the optional argument UNIT to settle that problem. New
> patch attached.

Looks good to me.  The only thing is:

In end of data:
files.el:7717:1:Warning: the function `string-empty-p' is not known to be
    defined.

That function is from subr-x, and I if I remember correctly, files.el
isn't supposed to load that file...

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

bug#35756: [PATCH] file-size-human-readable: fix glitches and add optional space

[Mattias Engdegård]

23 juni 2019 kl. 18.56 skrev Lars Ingebrigtsen <larsi@gnus.org>:
> 
> Looks good to me.  The only thing is:
> 
> In end of data:
> files.el:7717:1:Warning: the function `string-empty-p' is not known to be
>    defined.
> 
> That function is from subr-x, and I if I remember correctly, files.el
> isn't supposed to load that file...

Thanks, pushed with that function call eliminated in favour of (string= x "").