bug#58307: 29.0.50; Fix battery mode over TRAMP on *BSD

bug#58307: 29.0.50; Fix battery mode over TRAMP on *BSD

[Manuel Giraud]

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


Hi,

I'm using OpenBSD on a laptop.  I'm also using battery-mode.  Whenever,
I'm accessing another machine over TRAMP, the string displayed by
battery-mode become useless in this buffer.

The attached patch fixes this issue.  AFAIU, shell-command-to-string is
using the current directory of the buffer and fails to find a usable
"/usr/sbin/apm" on a remote Linux, for instance.  So I'm using
call-process that does call the local "/usr/sbin/apm".

Best regards,

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-BSD-battery-mode-over-TRAMP.patch --]
[-- Type: text/x-patch, Size: 1873 bytes --]

From e0a4eaa69171378664e28e8ab30a0e83e09ce19c Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Wed, 5 Oct 2022 15:59:14 +0200
Subject: [PATCH] Fix BSD battery mode over TRAMP

* lisp/battery.el (call-process-to-string): Simple wrapper to
  `call-process' that outputs to a string.
  (battery-bsd-apm): Use it instead of `shell-command-to-string'.
---
 lisp/battery.el | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/lisp/battery.el b/lisp/battery.el
index 72b3dfdae7..0fae92ab98 100644
--- a/lisp/battery.el
+++ b/lisp/battery.el
@@ -913,6 +913,10 @@ battery-upower
 
 \f
 ;;; `apm' interface for BSD.
+(defun call-process-to-string (program &rest args)
+  (with-output-to-string
+    (with-current-buffer standard-output
+      (apply #'call-process program nil t nil args))))
 
 (defun battery-bsd-apm ()
   "Get APM status information from BSD apm binary.
@@ -929,13 +933,13 @@ battery-bsd-apm
 %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"))))
+                        (call-process-to-string "/usr/bin/uname"))))
          (apm-flag (pcase os-name
                      ("OpenBSD" "mP")
                      ("FreeBSD" "st")
                      (_         "ms")))
-         (apm-cmd (concat "/usr/sbin/apm -abl" apm-flag))
-         (apm-output (split-string (shell-command-to-string apm-cmd)))
+         (apm-args (concat "-abl" apm-flag))
+         (apm-output (split-string (call-process-to-string "/usr/sbin/apm" apm-args)))
          (indices (pcase os-name
                     ;; FreeBSD's manpage documents that multiple
                     ;; outputs are ordered by "the order in which
-- 
2.37.3


[-- Attachment #3: Type: text/plain, Size: 7770 bytes --]



In GNU Emacs 29.0.50 (build 2, x86_64-unknown-openbsd7.2, cairo version
 1.17.6) of 2022-10-05 built on elite.giraud
Repository revision: 6430c4c44f3cbfdaa702302017afbc5d442960f3
Repository branch: mgi/battery
Windowing system distributor 'The X.Org Foundation', version 11.0.12101004
System Description: OpenBSD elite.giraud 7.2 GENERIC.MP#739 amd64

Configured using:
 'configure --prefix=/home/manuel/emacs --bindir=/home/manuel/bin
 --with-x-toolkit=no --without-sound --without-compress-install
 CPPFLAGS=-I/usr/local/include LDFLAGS=-L/usr/local/lib'

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON
LCMS2 LIBOTF LIBXML2 MODULES NOTIFY KQUEUE OLDXMENU PDUMPER PNG RSVG
SQLITE3 THREADS TIFF WEBP X11 XDBE XIM XINPUT2 XPM ZLIB

Important settings:
  value of $LC_ALL: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Group

Minor modes in effect:
  gnus-topic-mode: t
  global-git-commit-mode: t
  magit-auto-revert-mode: t
  icomplete-mode: t
  display-time-mode: t
  display-battery-mode: t
  gnus-undo-mode: t
  shell-dirtrack-mode: t
  global-so-long-mode: t
  repeat-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  buffer-read-only: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
/home/manuel/.emacs.d/elpa/transient-20220918.2101/transient hides /home/manuel/emacs/share/emacs/29.0.50/lisp/transient

Features:
(shadow emacsbug gnus-fun shr-color mailalias sort gnus-cite mail-extr
textsec uni-scripts idna-mapping ucs-normalize uni-confusable
textsec-check gnus-async gnus-bcklg gnus-draft gnus-ml gnus-topic
mm-archive url-http url-gw url-cache url-auth qp utf-7 imap rfc2104
nndoc nndraft nnmh network-stream nsm nnfolder nnml gnus-agent gnus-srvr
gnus-score score-mode nnvirtual nntp gnus-cache nnrss mm-url w3m
doc-view jka-compr image-mode exif w3m-hist w3m-fb bookmark-w3m w3m-ems
w3m-favicon w3m-image tab-line w3m-proc w3m-util magit-patch
magit-extras face-remap magit-bookmark magit-submodule magit-obsolete
magit-blame magit-stash magit-reflog magit-bisect magit-push magit-pull
magit-fetch magit-clone magit-remote magit-commit magit-sequence
magit-notes magit-worktree magit-tag magit-merge magit-branch
magit-reset magit-files magit-refs magit-status magit magit-repos
magit-apply magit-wip magit-log which-func imenu magit-diff smerge-mode
diff git-commit log-edit pcvs-util add-log magit-core magit-autorevert
autorevert filenotify magit-margin magit-transient magit-process
with-editor magit-mode transient magit-git magit-base magit-section dash
compat-27 compat-26 compat compat-macs shortdoc vc-hg vc-bzr sh-script
smie executable dabbrev cl-print help-fns radix-tree gnus-dired tramp-sh
tramp-cache time-stamp pulse misearch multi-isearch vc-git diff-mode
vc-dispatcher vc-svn bug-reference paredit edmacro icomplete time
battery exwm-randr xcb-randr exwm-config exwm exwm-input xcb-keysyms
xcb-xkb exwm-manage exwm-floating xcb-cursor xcb-render exwm-layout
exwm-workspace exwm-core xcb-ewmh xcb-icccm xcb xcb-xproto xcb-types
xcb-debug kmacro server stimmung-themes modus-operandi-theme
modus-themes ytdious osm mingus libmpdee reporter edebug debug backtrace
transmission diary-lib diary-loaddefs color calc-bin calc-ext calc
calc-loaddefs rect calc-macs w3m-load mu4e mu4e-org mu4e-main mu4e-view
mu4e-headers mu4e-compose mu4e-draft mu4e-actions smtpmail mu4e-search
mu4e-lists mu4e-bookmarks mu4e-mark mu4e-message flow-fill mule-util
hl-line mu4e-contacts mu4e-update mu4e-folders mu4e-server mu4e-context
mu4e-vars mu4e-helpers mu4e-config bookmark ido supercite regi
ebdb-message ebdb-gnus gnus-msg gnus-art mm-uu mml2015 mm-view mml-smime
smime gnutls dig gnus-sum shr pixel-fill kinsoku url-file svg dom
gnus-group gnus-undo gnus-start gnus-dbus gnus-cloud nnimap nnmail
mail-source utf7 nnoo gnus-spec gnus-int gnus-range message sendmail
yank-media puny rfc822 mml mml-sec epa epg rfc6068 epg-config mm-decode
mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045 ietf-drums
gmm-utils mailheader gnus-win gnus nnheader gnus-util mail-utils range
mm-util mail-prsvr ebdb-mua ebdb-com crm ebdb-format ebdb mailabbrev
eieio-opt speedbar ezimage dframe eieio-base pcase timezone org ob
ob-tangle ob-ref ob-lob ob-table ob-exp org-macro org-footnote org-src
ob-comint org-pcomplete org-list org-faces org-entities org-version
ob-emacs-lisp ob-core ob-eval org-table oc-basic bibtex ol org-keys oc
org-compat org-macs org-loaddefs find-func cal-menu calendar
cal-loaddefs visual-basic-mode cl web-mode disp-table erlang-start
smart-tabs-mode skeleton cc-mode cc-fonts cc-guess cc-menus cc-cmds
cc-styles cc-align cc-engine cc-vars cc-defs slime-asdf grep slime-tramp
tramp tramp-loaddefs trampver tramp-integration cus-edit cus-load
wid-edit files-x tramp-compat rx shell pcomplete parse-time iso8601
time-date ls-lisp format-spec slime-fancy slime-indentation
slime-cl-indent cl-indent slime-trace-dialog slime-fontifying-fu
slime-package-fu slime-references slime-compiler-notes-tree
slime-scratch slime-presentations advice bridge slime-macrostep
macrostep slime-mdot-fu slime-enclosing-context slime-fuzzy
slime-fancy-trace slime-fancy-inspector slime-c-p-c
slime-editing-commands slime-autodoc slime-repl elp slime-parse slime
derived cl-extra help-mode lisp-mnt gud apropos compile
text-property-search etags fileloop generator xref project arc-mode
archive-mode noutline outline icons pp comint ansi-osc ansi-color ring
hyperspec thingatpt slime-autoloads dired-aux dired-x dired
dired-loaddefs so-long notifications dbus xml repeat easy-mmode
auctex-autoloads tex-site boxquote-autoloads debbugs-autoloads
hyperbole-autoloads magit-autoloads git-commit-autoloads
magit-section-autoloads dash-autoloads paredit-autoloads
rust-mode-autoloads stimmung-themes-autoloads transient-autoloads
with-editor-autoloads info compat-autoloads ytdious-autoloads package
browse-url url url-proxy url-privacy url-expand url-methods url-history
url-cookie generate-lisp-file url-domsuf url-util mailcap url-handlers
url-parse auth-source cl-seq eieio eieio-core cl-macs password-cache
json subr-x map byte-opt gv bytecomp byte-compile cconv url-vars
cl-loaddefs cl-lib rmc iso-transl tooltip eldoc paren electric uniquify
ediff-hook vc-hooks lisp-float-type elisp-mode mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode lisp-mode prog-mode register
page tab-bar menu-bar rfn-eshadow isearch easymenu timer select
scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors
frame minibuffer nadvice seq simple cl-generic indonesian philippine
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs faces cus-face macroexp files window
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget keymap hashtable-print-readable backquote threads dbusbind
kqueue lcms2 dynamic-setting system-font-setting font-render-setting
cairo xinput2 x multi-tty make-network-process emacs)

Memory information:
((conses 16 1167279 633143)
 (symbols 48 63271 58)
 (strings 32 303254 61151)
 (string-bytes 1 9534116)
 (vectors 16 184075)
 (vector-slots 8 3124174 86979)
 (floats 8 683 436)
 (intervals 56 11854 1676)
 (buffers 1000 42))

-- 
Manuel Giraud

bug#58307: 29.0.50; Fix battery mode over TRAMP on *BSD

[Michael Albinus]

Manuel Giraud <manuel@ledu-giraud.fr> writes:

> Hi,

Hi Manual,

> I'm using OpenBSD on a laptop.  I'm also using battery-mode.  Whenever,
> I'm accessing another machine over TRAMP, the string displayed by
> battery-mode become useless in this buffer.
>
> The attached patch fixes this issue.  AFAIU, shell-command-to-string is
> using the current directory of the buffer and fails to find a usable
> "/usr/sbin/apm" on a remote Linux, for instance.  So I'm using
> call-process that does call the local "/usr/sbin/apm".

The patch LGTM. There's one point: you have called your helper function
`call-process-to-string'. This looks like a general function, which
moght confuse users. Could you rename it to something like
`battery--call-process-to-string'? Also a comment might be helpful
explaining the problem.

> Best regards,

Best regards, Michael.

bug#58307: 29.0.50; Fix battery mode over TRAMP on *BSD

[Stefan Kangas]

Manuel Giraud <manuel@ledu-giraud.fr> writes:

> @@ -929,13 +933,13 @@ battery-bsd-apm
>  %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"))))
> +                        (call-process-to-string "/usr/bin/uname"))))

Here's a minor nit:

I realize that the OpenBSD folks are not likely to move uname to /bin
(which is its location on my GNU/Linux system, for example), but I'd
probably still prefer

    (executable-find "uname")

to hard coding its location.

bug#58307: 29.0.50; Fix battery mode over TRAMP on *BSD

[Manuel Giraud]

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

Thank you Stefan and Michael!

Here is an updated version.  My wording in the comment might not be
correct.  Best regards.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-BSD-battery-mode-over-TRAMP-bug-58307.patch --]
[-- Type: text/x-patch, Size: 2257 bytes --]

From f9b1ee4df0280daab91a103fde4ee3a707d4224c Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Wed, 5 Oct 2022 15:59:14 +0200
Subject: [PATCH] Fix BSD battery mode over TRAMP (bug#58307)

* lisp/battery.el (battery--call-process-to-string): Simple wrapper
  to `call-process' that outputs to a string.
  (battery-bsd-apm): Use it instead of `shell-command-to-string'.
---
 lisp/battery.el | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/lisp/battery.el b/lisp/battery.el
index 72b3dfdae7..4c83f8a7d3 100644
--- a/lisp/battery.el
+++ b/lisp/battery.el
@@ -914,6 +914,15 @@ battery-upower
 \f
 ;;; `apm' interface for BSD.
 
+;; This function is a wrapper on `call-process' that return the
+;; standard output in a string.  We are using it instead
+;; `shell-command-to-string' because this last one is trying to run
+;; PROGRAM on the remote host if the buffer is remote.
+(defun battery--call-process-to-string (program &rest args)
+  (with-output-to-string
+    (with-current-buffer standard-output
+      (apply #'call-process program nil t nil args))))
+
 (defun battery-bsd-apm ()
   "Get APM status information from BSD apm binary.
 The following %-sequences are provided:
@@ -929,13 +938,14 @@ battery-bsd-apm
 %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"))))
+                        (battery--call-process-to-string (executable-find "uname")))))
          (apm-flag (pcase os-name
                      ("OpenBSD" "mP")
                      ("FreeBSD" "st")
                      (_         "ms")))
-         (apm-cmd (concat "/usr/sbin/apm -abl" apm-flag))
-         (apm-output (split-string (shell-command-to-string apm-cmd)))
+         (apm-args (concat "-abl" apm-flag))
+         (apm-output (split-string
+                      (battery--call-process-to-string (executable-find "apm") apm-args)))
          (indices (pcase os-name
                     ;; FreeBSD's manpage documents that multiple
                     ;; outputs are ordered by "the order in which
-- 
2.37.3


[-- Attachment #3: Type: text/plain, Size: 18 bytes --]

-- 
Manuel Giraud

bug#58307: 29.0.50; Fix battery mode over TRAMP on *BSD

[Lars Ingebrigtsen]

Manuel Giraud <manuel@ledu-giraud.fr> writes:

> Thank you Stefan and Michael!
>
> Here is an updated version.  My wording in the comment might not be
> correct.  Best regards.

Thanks; pushed to Emacs 29 (with some whitespace changes to get the
line length under 80).

bug#58307: 29.0.50; Fix battery mode over TRAMP on *BSD

[Manuel Giraud]

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Manuel Giraud <manuel@ledu-giraud.fr> writes:
>
>> Thank you Stefan and Michael!
>>
>> Here is an updated version.  My wording in the comment might not be
>> correct.  Best regards.
>
> Thanks; pushed to Emacs 29 (with some whitespace changes to get the
> line length under 80).

Thanks.  And here is an update on top of it with the suggestion of
Stefan Monnier on the list.  FWIW, it still works for me.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-lisp-battery.el-battery-bsd-apm-No-need-for-executab.patch --]
[-- Type: text/x-patch, Size: 1458 bytes --]

From dbf45fa6677dc6ac71988db39afa54f3527a48b3 Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Thu, 6 Oct 2022 16:03:34 +0200
Subject: [PATCH] ; * lisp/battery.el (battery-bsd-apm): No need for
 `executable-find'.

---
 lisp/battery.el | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/lisp/battery.el b/lisp/battery.el
index a874f0640e..8de80109c6 100644
--- a/lisp/battery.el
+++ b/lisp/battery.el
@@ -938,16 +938,14 @@ battery-bsd-apm
 %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'?
-                        (battery--call-process-to-string
-                         (executable-find "uname")))))
+                        (battery--call-process-to-string "uname"))))
          (apm-flag (pcase os-name
                      ("OpenBSD" "mP")
                      ("FreeBSD" "st")
                      (_         "ms")))
          (apm-args (concat "-abl" apm-flag))
          (apm-output (split-string
-                      (battery--call-process-to-string
-                       (executable-find "apm") apm-args)))
+                      (battery--call-process-to-string "apm" apm-args)))
          (indices (pcase os-name
                     ;; FreeBSD's manpage documents that multiple
                     ;; outputs are ordered by "the order in which
-- 
2.37.3


[-- Attachment #3: Type: text/plain, Size: 18 bytes --]

-- 
Manuel Giraud

bug#58307: 29.0.50; Fix battery mode over TRAMP on *BSD

[Stefan Kangas]

Manuel Giraud <manuel@ledu-giraud.fr> writes:

> Thanks.  And here is an update on top of it with the suggestion of
> Stefan Monnier on the list.  FWIW, it still works for me.

Pushed to master.