Various battery.el improvements (bug#41808)

* lisp/battery.el: Mention BSD support in Commentary.  Don't load
preloaded lisp/emacs-lisp/timer.el.
(battery--files): New function.
(battery--find-linux-sysfs-batteries): Use it and make fewer
syscalls.
(battery-status-function): Perform GNU/Linux checks in increasing
order of obsolescence: sysfs, ACPI, and then APM.  Simplify Darwin
check.  Add :version tag now that battery-upower is the default.
(battery-echo-area-format, battery-mode-line-format): Mention %s.
(battery-load-low, battery-load-critical): New faces.
(battery-update): Display battery-mode-line-format even if
percentage is N/A.  Apply faces battery-load-low or
battery-load-critical according to the percentage, but append them
so they don't override user customizations.  Update all mode lines
since we are in global-mode-string.
(battery-linux-proc-apm-regexp): Mark as obsolete, replacing with...
(battery--linux-proc-apm): ...this new rx definition.
(battery-linux-proc-apm): Use it.  Fix indentation.  Simplify.
(battery--acpi-rate, battery--acpi-capacity): New rx definitions.
(battery-linux-proc-acpi): Use them.  Fix pathological whitespace
regexps.  Simplify.
(battery-linux-sysfs): Fix docstring and indentation.  Reduce number
of file searches.  Simplify.
(battery-bsd-apm): Fix docstring.  Simplify.
(battery-pmset): Fix docstring.  Simplify ID regexp.

* lisp/emacs-lisp/rx.el (rx-define): Indent as a defun.

* test/lisp/battery-tests.el (battery-linux-proc-apm-regexp): Test
new battery--linux-proc-apm rx definition.
(battery-acpi-rate-regexp, battery-acpi-capacity-regexp): New tests.

1 files changed, 192 insertions, 171 deletions

diff --git a/lisp/battery.el b/lisp/battery.el
index 38728196507..f6f70b2f16d 100644
--- a/lisp/battery.el
+++ b/lisp/battery.el
@@ -23,18 +23,18 @@
 
 ;;; Commentary:
 
-;; There is at present support for GNU/Linux, macOS, and Windows.
+;; There is at present support for GNU/Linux, BSD, macOS, and Windows.
 ;; This library supports:
 ;; - UPower (https://upower.freedesktop.org) via D-Bus API.
-;; - the `/sys/class/power_supply/' files of Linux >= 2.6.39.
-;; - the `/proc/acpi/' directory structure of Linux 2.4.20 and 2.6.
-;; - the `/proc/apm' file format of Linux version 1.3.58 or newer.
+;; - The `/sys/class/power_supply/' files of Linux >= 2.6.39.
+;; - The `/proc/acpi/' directory structure of Linux 2.4.20 and 2.6.
+;; - The `/proc/apm' file format of Linux version 1.3.58 or newer.
+;; - BSD by using the `apm' program.
 ;; - Darwin (macOS) by using the `pmset' program.
 ;; - Windows via the GetSystemPowerStatus API call.
 
 ;;; Code:
 
-(require 'timer)
 (require 'dbus)
 (eval-when-compile (require 'cl-lib))
 
@@ -60,37 +60,39 @@ If set to nil, then autodetect `:battery' device."
 (defconst battery-upower-dbus-service "org.freedesktop.UPower"
   "Well-known UPower service name for the D-Bus system.")
 
+(defun battery--files (dir)
+  "Return a list of absolute file names in DIR or nil on error.
+Value does not include \".\" or \"..\"."
+  (ignore-errors (directory-files dir t directory-files-no-dot-files-regexp)))
+
 (defun battery--find-linux-sysfs-batteries ()
-  (let ((dirs nil))
-    (dolist (file (directory-files "/sys/class/power_supply/" t))
-      (when (and (or (file-directory-p file)
-                     (file-symlink-p file))
-                 (file-exists-p (expand-file-name "capacity" file)))
-        (push file dirs)))
+  "Return a list of all sysfs battery directories."
+  (let (dirs)
+    (dolist (dir (battery--files "/sys/class/power_supply/"))
+      (when (file-exists-p (expand-file-name "capacity" dir))
+        (push dir dirs)))
     (nreverse dirs)))
 
 (defcustom battery-status-function
   (cond ((dbus-ping :system battery-upower-dbus-service)
          #'battery-upower)
         ((and (eq system-type 'gnu/linux)
-	      (file-readable-p "/proc/apm"))
-	 #'battery-linux-proc-apm)
+              (battery--find-linux-sysfs-batteries))
+         #'battery-linux-sysfs)
 	((and (eq system-type 'gnu/linux)
 	      (file-directory-p "/proc/acpi/battery"))
 	 #'battery-linux-proc-acpi)
 	((and (eq system-type 'gnu/linux)
-	      (file-directory-p "/sys/class/power_supply/")
-              (battery--find-linux-sysfs-batteries))
-	 #'battery-linux-sysfs)
+              (file-readable-p "/proc/apm"))
+         #'battery-linux-proc-apm)
 	((and (eq system-type 'berkeley-unix)
 	      (file-executable-p "/usr/sbin/apm"))
 	 #'battery-bsd-apm)
 	((and (eq system-type 'darwin)
-	      (condition-case nil
-		  (with-temp-buffer
-		    (and (eq (call-process "pmset" nil t nil "-g" "ps") 0)
-			 (> (buffer-size) 0)))
-		(error nil)))
+              (ignore-errors
+                (with-temp-buffer
+                  (and (eq (call-process "pmset" nil t nil "-g" "ps") 0)
+                       (not (bobp))))))
 	 #'battery-pmset)
 	((fboundp 'w32-battery-status)
 	 #'w32-battery-status))
@@ -102,6 +104,7 @@ Its cons cells are of the form
 
 CONVERSION is the character code of a \"conversion specification\"
 introduced by a `%' character in a control string."
+  :version "28.1"
   :type '(choice (const nil) function))
 
 (defcustom battery-echo-area-format
@@ -113,12 +116,13 @@ string are substituted as defined by the current value of the variable
 `battery-status-function'.  Here are the ones generally available:
 %c Current capacity (mAh or mWh)
 %r Current rate of charge or discharge
+%L AC line status (verbose)
 %B Battery status (verbose)
 %b Battery status: empty means high, `-' means low,
    `!' means critical, and `+' means charging
 %d Temperature (in degrees Celsius)
-%L AC line status (verbose)
 %p Battery load percentage
+%s Remaining time (to charge or discharge) in seconds
 %m Remaining time (to charge or discharge) in minutes
 %h Remaining time (to charge or discharge) in hours
 %t Remaining time (to charge or discharge) in the form `h:min'
@@ -137,7 +141,7 @@ The full `format-spec' formatting syntax is supported."
   :type 'integer)
 
 (defcustom battery-mode-line-format
-  (cond ((eq battery-status-function 'battery-linux-proc-acpi)
+  (cond ((eq battery-status-function #'battery-linux-proc-acpi)
 	 "[%b%p%%,%d°C]")
 	(battery-status-function
 	 "[%b%p%%]"))
@@ -148,12 +152,13 @@ string are substituted as defined by the current value of the variable
 `battery-status-function'.  Here are the ones generally available:
 %c Current capacity (mAh or mWh)
 %r Current rate of charge or discharge
+%L AC line status (verbose)
 %B Battery status (verbose)
 %b Battery status: empty means high, `-' means low,
    `!' means critical, and `+' means charging
 %d Temperature (in degrees Celsius)
-%L AC line status (verbose)
 %p Battery load percentage
+%s Remaining time (to charge or discharge) in seconds
 %m Remaining time (to charge or discharge) in minutes
 %h Remaining time (to charge or discharge) in hours
 %t Remaining time (to charge or discharge) in the form `h:min'
@@ -176,6 +181,18 @@ A battery load percentage below this number is considered low."
 A battery load percentage below this number is considered critical."
   :type 'integer)
 
+(defface battery-load-low
+  '((t :inherit warning))
+  "Face used in mode line string when battery load is low.
+See the option `battery-load-low'."
+  :version "28.1")
+
+(defface battery-load-critical
+  '((t :inherit error))
+  "Face used in mode line string when battery load is critical.
+See the option `battery-load-critical'."
+  :version "28.1")
+
 (defvar battery-update-timer nil
   "Interval timer object.")
 
@@ -208,7 +225,7 @@ seconds."
 		(delq 'battery-mode-line-string global-mode-string))
 	(add-to-list 'global-mode-string 'battery-mode-line-string t)
 	(setq battery-update-timer (run-at-time nil battery-update-interval
-						'battery-update-handler))
+                                                #'battery-update-handler))
 	(battery-update))
     (message "Battery status not available")
     (setq display-battery-mode nil)))
@@ -220,34 +237,42 @@ seconds."
 (defun battery-update ()
   "Update battery status information in the mode line."
   (let* ((data (and battery-status-function (funcall battery-status-function)))
-         (percentage (car (read-from-string (cdr (assq ?p data))))))
-    (setq battery-mode-line-string
-	  (propertize (if (and battery-mode-line-format
-			       (numberp percentage)
-                               (<= percentage battery-mode-line-limit))
-			  (battery-format battery-mode-line-format data)
-			"")
-		      'face
-                      (and (numberp percentage)
-                           (<= percentage battery-load-critical)
-                           'error)
-		      'help-echo "Battery status information")))
-  (force-mode-line-update))
+         (percentage (car (read-from-string (cdr (assq ?p data)))))
+         (res (and battery-mode-line-format
+                   (or (not (numberp percentage))
+                       (<= percentage battery-mode-line-limit))
+                   (battery-format battery-mode-line-format data)))
+         (len (length res)))
+    (unless (zerop len)
+      (cond ((not (numberp percentage)))
+            ((< percentage battery-load-critical)
+             (add-face-text-property 0 len 'battery-load-critical t res))
+            ((< percentage battery-load-low)
+             (add-face-text-property 0 len 'battery-load-low t res)))
+      (put-text-property 0 len 'help-echo "Battery status information" res))
+    (setq battery-mode-line-string (or res "")))
+  (force-mode-line-update t))
+
 
 ;;; `/proc/apm' interface for Linux.
 
-(defconst battery-linux-proc-apm-regexp
-  (concat "^\\([^ ]+\\)"		; Driver version.
-	  " \\([^ ]+\\)"		; APM BIOS version.
-	  " 0x\\([0-9a-f]+\\)"		; APM BIOS flags.
-	  " 0x\\([0-9a-f]+\\)"		; AC line status.
-	  " 0x\\([0-9a-f]+\\)"		; Battery status.
-	  " 0x\\([0-9a-f]+\\)"		; Battery flags.
-	  " \\(-?[0-9]+\\)%"		; Load percentage.
-	  " \\(-?[0-9]+\\)"		; Remaining time.
-	  " \\(.*\\)"			; Time unit.
-	  "$")
+;; Regular expression matching contents of `/proc/apm'.
+(rx-define battery--linux-proc-apm
+  (: bol   (group (+ (not ?\s)))        ; Driver version.
+     " "   (group (+ (not ?\s)))        ; APM BIOS version.
+     " 0x" (group (+ xdigit))           ; APM BIOS flags.
+     " 0x" (group (+ xdigit))           ; AC line status.
+     " 0x" (group (+ xdigit))           ; Battery status.
+     " 0x" (group (+ xdigit))           ; Battery flags.
+     " "   (group (? ?-) (+ digit)) ?%  ; Load percentage.
+     " "   (group (? ?-) (+ digit))     ; Remaining time.
+     " "   (group (* nonl))             ; Time unit
+     eol))
+
+(defconst battery-linux-proc-apm-regexp (rx battery--linux-proc-apm)
   "Regular expression matching contents of `/proc/apm'.")
+(make-obsolete-variable 'battery-linux-proc-apm-regexp
+                        "it is no longer used." "28.1")
 
 (defun battery-linux-proc-apm ()
   "Get APM status information from Linux (the kernel).
@@ -267,12 +292,12 @@ The following %-sequences are provided:
 %m Remaining time (to charge or discharge) in minutes
 %h Remaining time (to charge or discharge) in hours
 %t Remaining time (to charge or discharge) in the form `h:min'"
-  (let (driver-version bios-version bios-interface line-status
-	battery-status battery-status-symbol load-percentage
-	seconds minutes hours remaining-time tem)
+  (let ( driver-version bios-version bios-interface line-status
+         battery-status battery-status-symbol load-percentage
+         seconds minutes hours remaining-time tem )
     (with-temp-buffer
       (ignore-errors (insert-file-contents "/proc/apm"))
-      (when (re-search-forward battery-linux-proc-apm-regexp)
+      (when (re-search-forward (rx battery--linux-proc-apm) nil t)
 	(setq driver-version (match-string 1))
 	(setq bios-version (match-string 2))
 	(setq tem (string-to-number (match-string 3) 16))
@@ -285,9 +310,7 @@ The following %-sequences are provided:
 	  (cond ((= tem 0) (setq line-status "off-line"))
 		((= tem 1) (setq line-status "on-line"))
 		((= tem 2) (setq line-status "on backup")))
-	  (setq tem (string-to-number (match-string 6) 16))
-	  (if (= tem 255)
-	      (setq battery-status "N/A")
+          (unless (= (string-to-number (match-string 6) 16) 255)
 	    (setq tem (string-to-number (match-string 5) 16))
 	    (cond ((= tem 0) (setq battery-status "high"
 				   battery-status-symbol ""))
@@ -304,7 +327,7 @@ The following %-sequences are provided:
 	    (setq minutes (/ seconds 60)
 		  hours (/ seconds 3600))
 	    (setq remaining-time
-		  (format "%d:%02d" hours (- minutes (* 60 hours))))))))
+                  (format "%d:%02d" hours (% minutes 60)))))))
     (list (cons ?v (or driver-version "N/A"))
 	  (cons ?V (or bios-version "N/A"))
 	  (cons ?I (or bios-interface "N/A"))
@@ -312,27 +335,31 @@ The following %-sequences are provided:
 	  (cons ?B (or battery-status "N/A"))
 	  (cons ?b (or battery-status-symbol ""))
 	  (cons ?p (or load-percentage "N/A"))
-	  (cons ?s (or (and seconds (number-to-string seconds)) "N/A"))
-	  (cons ?m (or (and minutes (number-to-string minutes)) "N/A"))
-	  (cons ?h (or (and hours (number-to-string hours)) "N/A"))
+          (cons ?s (if seconds (number-to-string seconds) "N/A"))
+          (cons ?m (if minutes (number-to-string minutes) "N/A"))
+          (cons ?h (if hours (number-to-string hours) "N/A"))
 	  (cons ?t (or remaining-time "N/A")))))
 
 
 ;;; `/proc/acpi/' interface for Linux.
 
+(rx-define battery--acpi-rate (&rest hour)
+  (: (group (+ digit)) " " (group ?m (in "AW") hour)))
+(rx-define battery--acpi-capacity (battery--acpi-rate ?h))
+
 (defun battery-linux-proc-acpi ()
   "Get ACPI status information from Linux (the kernel).
-This function works only with the `/proc/acpi/' format introduced
-in Linux version 2.4.20 and 2.6.0.
+This function works only with the `/proc/acpi/' interface
+introduced in Linux version 2.4.20 and 2.6.0.
 
 The following %-sequences are provided:
 %c Current capacity (mAh)
-%r Current rate
+%r Current rate of charge or discharge
+%L AC line status (verbose)
 %B Battery status (verbose)
 %b Battery status, empty means high, `-' means low,
    `!' means critical, and `+' means charging
 %d Temperature (in degrees Celsius)
-%L AC line status (verbose)
 %p Battery load percentage
 %m Remaining time (to charge or discharge) in minutes
 %h Remaining time (to charge or discharge) in hours
@@ -348,45 +375,51 @@ The following %-sequences are provided:
     ;; information together since displaying for a variable amount of
     ;; batteries seems overkill for format-strings.
     (with-temp-buffer
-      (dolist (dir (ignore-errors (directory-files "/proc/acpi/battery/"
-						   t "\\`[^.]")))
-	(erase-buffer)
-	(ignore-errors (insert-file-contents (expand-file-name "state" dir)))
-	(when (re-search-forward "present: +yes$" nil t)
-	  (and (re-search-forward "charging state: +\\(.*\\)$" nil t)
+      (dolist (dir (battery--files "/proc/acpi/battery/"))
+        (ignore-errors
+          (insert-file-contents (expand-file-name "state" dir) nil nil nil t))
+        (goto-char (point-min))
+        (when (re-search-forward (rx "present:" (+ space) "yes" eol) nil t)
+          (and (re-search-forward (rx "charging state:" (+ space)
+                                      (group (not space) (* nonl)) eol)
+                                  nil t)
 	       (member charging-state '("unknown" "charged" nil))
 	       ;; On most multi-battery systems, most of the time only one
 	       ;; battery is "charging"/"discharging", the others are
 	       ;; "unknown".
 	       (setq charging-state (match-string 1)))
-	  (when (re-search-forward "present rate: +\\([0-9]+\\) \\(m[AW]\\)$"
+          (when (re-search-forward (rx "present rate:" (+ space)
+                                       (battery--acpi-rate) eol)
 				   nil t)
 	    (setq rate (+ (or rate 0) (string-to-number (match-string 1))))
 	    (when (> rate 0)
-	      (setq rate-type (or (and rate-type
-				     (if (string= rate-type (match-string 2))
-					 rate-type
-				       (error
-					"Inconsistent rate types (%s vs. %s)"
-					rate-type (match-string 2))))
-				  (match-string 2)))))
-	  (when (re-search-forward "remaining capacity: +\\([0-9]+\\) m[AW]h$"
+              (cond ((not rate-type)
+                     (setq rate-type (match-string 2)))
+                    ((not (string= rate-type (match-string 2)))
+                     (error "Inconsistent rate types (%s vs. %s)"
+                            rate-type (match-string 2))))))
+          (when (re-search-forward (rx "remaining capacity:" (+ space)
+                                       battery--acpi-capacity eol)
 				   nil t)
 	    (setq capacity
 		  (+ (or capacity 0) (string-to-number (match-string 1))))))
 	(goto-char (point-max))
 	(ignore-errors (insert-file-contents (expand-file-name "info" dir)))
-	(when (re-search-forward "present: +yes$" nil t)
-	  (when (re-search-forward "design capacity: +\\([0-9]+\\) m[AW]h$"
+        (when (re-search-forward (rx "present:" (+ space) "yes" eol) nil t)
+          (when (re-search-forward (rx "design capacity:" (+ space)
+                                       battery--acpi-capacity eol)
 				   nil t)
 	    (cl-incf design-capacity (string-to-number (match-string 1))))
-	  (when (re-search-forward "last full capacity: +\\([0-9]+\\) m[AW]h$"
+          (when (re-search-forward (rx "last full capacity:" (+ space)
+                                       battery--acpi-capacity eol)
 				   nil t)
 	    (cl-incf last-full-capacity (string-to-number (match-string 1))))
-	  (when (re-search-forward
-		 "design capacity warning: +\\([0-9]+\\) m[AW]h$" nil t)
+          (when (re-search-forward (rx "design capacity warning:" (+ space)
+                                       battery--acpi-capacity eol)
+                                   nil t)
 	    (cl-incf warn (string-to-number (match-string 1))))
-	  (when (re-search-forward "design capacity low: +\\([0-9]+\\) m[AW]h$"
+          (when (re-search-forward (rx "design capacity low:" (+ space)
+                                       battery--acpi-capacity eol)
 				   nil t)
 	    (cl-incf low (string-to-number (match-string 1)))))))
     (setq full-capacity (if (> last-full-capacity 0)
@@ -400,79 +433,70 @@ The following %-sequences are provided:
 				   60)
 				rate))
 	       hours (/ minutes 60)))
-    (list (cons ?c (or (and capacity (number-to-string capacity)) "N/A"))
+    (list (cons ?c (if capacity (number-to-string capacity) "N/A"))
 	  (cons ?L (or (battery-search-for-one-match-in-files
-			(mapcar (lambda (e) (concat e "/state"))
-				(ignore-errors
-				  (directory-files "/proc/acpi/ac_adapter/"
-						   t "\\`[^.]")))
-			"state: +\\(.*\\)$" 1)
-
+                        (mapcar (lambda (d) (expand-file-name "state" d))
+                                (battery--files "/proc/acpi/ac_adapter/"))
+                        (rx "state:" (+ space) (group (not space) (* nonl)) eol)
+                        1)
 		       "N/A"))
 	  (cons ?d (or (battery-search-for-one-match-in-files
-			(mapcar (lambda (e) (concat e "/temperature"))
-				(ignore-errors
-				  (directory-files "/proc/acpi/thermal_zone/"
-						   t "\\`[^.]")))
-			"temperature: +\\([0-9]+\\) C$" 1)
-
+                        (mapcar (lambda (d) (expand-file-name "temperature" d))
+                                (battery--files "/proc/acpi/thermal_zone/"))
+                        (rx "temperature:" (+ space) (group (+ digit)) " C" eol)
+                        1)
 		       "N/A"))
-	  (cons ?r (or (and rate (concat (number-to-string rate) " "
-					 rate-type)) "N/A"))
+          (cons ?r (if rate
+                       (concat (number-to-string rate) " " rate-type)
+                     "N/A"))
 	  (cons ?B (or charging-state "N/A"))
-	  (cons ?b (or (and (string= charging-state "charging") "+")
-		       (and capacity (< capacity low) "!")
-		       (and capacity (< capacity warn) "-")
-		       ""))
-	  (cons ?h (or (and hours (number-to-string hours)) "N/A"))
-	  (cons ?m (or (and minutes (number-to-string minutes)) "N/A"))
-	  (cons ?t (or (and minutes
-			    (format "%d:%02d" hours (- minutes (* 60 hours))))
-		       "N/A"))
-	  (cons ?p (or (and full-capacity capacity
-			    (> full-capacity 0)
-			    (number-to-string
-			     (floor (* 100 capacity) full-capacity)))
-		       "N/A")))))
+          (cons ?b (cond ((string= charging-state "charging") "+")
+                         ((and capacity (< capacity low)) "!")
+                         ((and capacity (< capacity warn)) "-")
+                         ("")))
+          (cons ?h (if hours (number-to-string hours) "N/A"))
+          (cons ?m (if minutes (number-to-string minutes) "N/A"))
+          (cons ?t (if minutes (format "%d:%02d" hours (% minutes 60)) "N/A"))
+          (cons ?p (if (and full-capacity capacity (> full-capacity 0))
+                       (number-to-string (floor (* 100 capacity) full-capacity))
+                     "N/A")))))
 
 
 ;;; `/sys/class/power_supply/BATN' interface for Linux.
 
 (defun battery-linux-sysfs ()
-  "Get ACPI status information from Linux kernel.
+  "Get sysfs status information from Linux kernel.
 This function works only with the new `/sys/class/power_supply/'
-format introduced in Linux version 2.4.25.
+interface introduced in Linux version 2.4.25.
 
 The following %-sequences are provided:
 %c Current capacity (mAh or mWh)
-%r Current rate
+%r Current rate of charge or discharge
+%L Power source (verbose)
 %B Battery status (verbose)
 %b Battery status, empty means high, `-' means low,
    `!' means critical, and `+' means charging
 %d Temperature (in degrees Celsius)
 %p Battery load percentage
-%L AC line status (verbose)
 %m Remaining time (to charge or discharge) in minutes
 %h Remaining time (to charge or discharge) in hours
 %t Remaining time (to charge or discharge) in the form `h:min'"
-  (let (charging-state temperature hours percentage-now
-        ;; Some batteries report charges and current, other energy and power.
+  (let (;; Some batteries report charges and current, others energy and power.
         ;; In order to reliably be able to combine those data, we convert them
         ;; all to energy/power (since we can't combine different charges if
         ;; they're not at the same voltage).
 	(energy-full 0.0)
 	(energy-now 0.0)
 	(power-now 0.0)
-	(voltage-now 10.8))    ;Arbitrary default, in case the info is missing.
+        (voltage-now 10.8) ; Arbitrary default, in case the info is missing.
+        charging-state temperature hours percentage-now)
     ;; SysFS provides information about each battery present in the
     ;; system in a separate subdirectory.  We are going to merge the
     ;; available information together.
     (with-temp-buffer
-      (dolist (dir (ignore-errors
-                     (battery--find-linux-sysfs-batteries)))
-	(erase-buffer)
-	(ignore-errors (insert-file-contents
-			(expand-file-name "uevent" dir)))
+      (dolist (dir (battery--find-linux-sysfs-batteries))
+        (ignore-errors
+          (insert-file-contents (expand-file-name "uevent" dir) nil nil nil t))
 	(goto-char (point-min))
 	(when (re-search-forward
 	       "POWER_SUPPLY_VOLTAGE_NOW=\\([0-9]*\\)$" nil t)
@@ -508,7 +532,7 @@ The following %-sequences are provided:
                                            voltage-now))
 		   (cl-incf energy-now  (* (string-to-number now-string)
                                            voltage-now)))
-		  ((and (progn (goto-char (point-min)) t)
+                  ((and (goto-char (point-min))
 			(re-search-forward
 			 "POWER_SUPPLY_ENERGY_FULL=\\([0-9]*\\)$" nil t)
 			(setq full-string (match-string 1))
@@ -517,7 +541,6 @@ The following %-sequences are provided:
 			(setq now-string (match-string 1)))
 		   (cl-incf energy-full (string-to-number full-string))
 		   (cl-incf energy-now  (string-to-number now-string)))))
-	  (goto-char (point-min))
 	  (unless (zerop power-now)
 	    (let ((remaining (if (string= charging-state "Discharging")
 				 energy-now
@@ -525,9 +548,9 @@ The following %-sequences are provided:
 	      (setq hours (/ remaining power-now)))))))
     (when (and (> energy-full 0) (> energy-now 0))
       (setq percentage-now (/ (* 100 energy-now) energy-full)))
-    (list (cons ?c (cond ((or (> energy-full 0) (> energy-now 0))
-			  (number-to-string (/ energy-now voltage-now)))
-			 (t "N/A")))
+    (list (cons ?c (if (or (> energy-full 0) (> energy-now 0))
+                       (number-to-string (/ energy-now voltage-now))
+                     "N/A"))
 	  (cons ?r (if (> power-now 0.0)
 		       (format "%.1f" (/ power-now 1000000.0))
 		     "N/A"))
@@ -538,27 +561,20 @@ The following %-sequences are provided:
 		     "N/A"))
 	  (cons ?d (or temperature "N/A"))
 	  (cons ?B (or charging-state "N/A"))
-	  (cons ?b (or (and (string= charging-state "Charging") "+")
-		       (and percentage-now (< percentage-now battery-load-critical) "!")
-		       (and percentage-now (< percentage-now battery-load-low) "-")
-		       ""))
-	  (cons ?p (cond
-                    ((and percentage-now (format "%.1f" percentage-now)))
-                    (t "N/A")))
-	  (cons ?L (cond
-                    ((battery-search-for-one-match-in-files
-                      (list "/sys/class/power_supply/AC/online"
-                            "/sys/class/power_supply/ACAD/online"
-                            "/sys/class/power_supply/ADP1/online")
-                      "1" 0)
-                     "AC")
-                    ((battery-search-for-one-match-in-files
-                      (list "/sys/class/power_supply/AC/online"
-                            "/sys/class/power_supply/ACAD/online"
-                            "/sys/class/power_supply/ADP1/online")
-                      "0" 0)
-                     "BAT")
-                    (t "N/A"))))))
+          (cons ?b (cond ((string= charging-state "Charging") "+")
+                         ((not percentage-now) "")
+                         ((< percentage-now battery-load-critical) "!")
+                         ((< percentage-now battery-load-low) "-")
+                         ("")))
+          (cons ?p (if percentage-now (format "%.1f" percentage-now) "N/A"))
+          (cons ?L (pcase (battery-search-for-one-match-in-files
+                           '("/sys/class/power_supply/AC/online"
+                             "/sys/class/power_supply/ACAD/online"
+                             "/sys/class/power_supply/ADP1/online")
+                           (rx (in "01")) 0)
+                     ("0" "BAT")
+                     ("1" "AC")
+                     (_ "N/A"))))))
 
 
 ;;; `upowerd' interface.
@@ -681,19 +697,20 @@ The following %-sequences are provided:
 
 
 ;;; `apm' interface for BSD.
+
 (defun battery-bsd-apm ()
   "Get APM status information from BSD apm binary.
 The following %-sequences are provided:
+%P Advanced power saving mode state (verbose)
 %L AC line status (verbose)
 %B Battery status (verbose)
 %b Battery status, empty means high, `-' means low,
- `!' means critical, and `+' means charging
-%P Advanced power saving mode state (verbose)
-%p Battery charge percentage
-%s Remaining battery charge time in seconds
-%m Remaining battery charge time in minutes
-%h Remaining battery charge time in hours
-%t Remaining battery charge time in the form `h:min'"
+   `!' means critical, and `+' means charging
+%p Battery load percentage
+%s Remaining time (to charge or discharge) in seconds
+%m Remaining time (to charge or discharge) in minutes
+%h Remaining time (to charge or discharge) in hours
+%t Remaining time (to charge or discharge) in the form `h:min'"
   (let* ((os-name (car (split-string
                         ;; FIXME: Can't we use something like `system-type'?
                         (shell-command-to-string "/usr/bin/uname"))))
@@ -759,7 +776,7 @@ The following %-sequences are provided:
 	(setq seconds (string-to-number battery-life)
 	      minutes (truncate seconds 60)))
       (setq hours (truncate minutes 60)
-	    remaining-time (format "%d:%02d" hours (mod minutes 60))))
+            remaining-time (format "%d:%02d" hours (% minutes 60))))
     (list (cons ?L (or line-status "N/A"))
 	  (cons ?B (or (car battery-status) "N/A"))
 	  (cons ?b (or (cdr battery-status) "N/A"))
@@ -767,9 +784,9 @@ The following %-sequences are provided:
 		       "N/A"
 		     battery-percentage))
 	  (cons ?P (or apm-mode "N/A"))
-	  (cons ?s (or (and seconds (number-to-string seconds)) "N/A"))
-	  (cons ?m (or (and minutes (number-to-string minutes)) "N/A"))
-	  (cons ?h (or (and hours (number-to-string hours)) "N/A"))
+          (cons ?s (if seconds (number-to-string seconds) "N/A"))
+          (cons ?m (if minutes (number-to-string minutes) "N/A"))
+          (cons ?h (if hours (number-to-string hours) "N/A"))
 	  (cons ?t (or remaining-time "N/A")))))
 
 
@@ -784,21 +801,25 @@ The following %-sequences are provided:
 %b Battery status, empty means high, `-' means low,
    `!' means critical, and `+' means charging
 %p Battery load percentage
-%h Remaining time in hours
-%m Remaining time in minutes
-%t Remaining time in the form `h:min'"
-  (let (power-source load-percentage battery-status battery-status-symbol
-	remaining-time hours minutes)
+%m Remaining time (to charge or discharge) in minutes
+%h Remaining time (to charge or discharge) in hours
+%t Remaining time (to charge or discharge) in the form `h:min'"
+  (let ( power-source load-percentage battery-status battery-status-symbol
+         remaining-time hours minutes )
     (with-temp-buffer
       (ignore-errors (call-process "pmset" nil t nil "-g" "ps"))
       (goto-char (point-min))
-      (when (re-search-forward "\\(?:Currentl?y\\|Now\\) drawing from '\\(AC\\|Battery\\) Power'" nil t)
+      (when (re-search-forward ;; Handle old typo in output.
+             "\\(?:Currentl?y\\|Now\\) drawing from '\\(AC\\|Battery\\) Power'"
+             nil t)
 	(setq power-source (match-string 1))
-	(when (re-search-forward "^ -InternalBattery-0\\([ \t]+(id=[0-9]+)\\)*[ \t]+" nil t)
+        (when (re-search-forward (rx bol " -InternalBattery-0" (+ space)
+                                     (* "(id=" (+ digit) ")" (+ space)))
+                                 nil t)
 	  (when (looking-at "\\([0-9]\\{1,3\\}\\)%")
 	    (setq load-percentage (match-string 1))
 	    (goto-char (match-end 0))
-	    (cond ((looking-at "; charging")
+            (cond ((looking-at-p "; charging")
 		   (setq battery-status "charging"
 			 battery-status-symbol "+"))
 		  ((< (string-to-number load-percentage) battery-load-critical)