bug#1321: process-lines

bug#1321: process-lines

[Peder O. Klingenberg]

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

Just adding an optional argument to process-lines is difficult without breaking the API, due to its use of &rest.  I suggest something along the lines of the following patch:

...Peder...
-- 
Sløv uten dop

[-- Attachment #2: 0001-Extend-process-lines-to-allow-exit-status-handling.patch --]
[-- Type: application/octet-stream, Size: 3312 bytes --]

From 36387738fa6dc45db5a0ea4bbcc7ad982ba95da3 Mon Sep 17 00:00:00 2001
From: "Peder O. Klingenberg" <peder@klingenberg.no>
Date: Sat, 14 Mar 2015 23:00:16 +0100
Subject: [PATCH] Extend process-lines to allow exit status handling

* subr.el (process-lines-handling-status): Extension of the old
process-lines, with more flexible handling of the exit status.
(process-lines): Old API implemented using the new function.
(process-lines-ignore-status): Another use of the new function -
return the output lines regardless of the exit status.

Fixes: Bug#1321
---
 lisp/ChangeLog |  8 ++++++++
 lisp/subr.el   | 24 ++++++++++++++++++++----
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index d393190..a93a306 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,11 @@
+2015-03-14  Peder O. Klingenberg  <peder@klingenberg.no>
+
+	* subr.el (process-lines-handling-status): Extension of the old
+	process-lines, with more flexible handling of the exit status.
+	(process-lines): Old API implemented using the new function.
+	(process-lines-ignore-status): Another use of the new function -
+	return the output lines regardless of the exit status.
+
 2015-03-13  Kevin Ryde  <user42_kevin@yahoo.com.au>
 
 	info-look fixes for Texinfo 5
diff --git a/lisp/subr.el b/lisp/subr.el
index deadca6..45b23dc 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -1901,13 +1901,19 @@ and the file name is displayed in the echo area."
 \f
 ;;;; Process stuff.
 
-(defun process-lines (program &rest args)
+(defun process-lines-handling-status (program status-handler &rest args)
   "Execute PROGRAM with ARGS, returning its output as a list of lines.
-Signal an error if the program returns with a non-zero exit status."
+If STATUS-HANDLER is non-NIL, it must be a function with one
+argument, which will be called with the exit status of the
+program before the output is collected.  If STATUS-HANDLER is
+NIL, an error is signalled if the program returns with a non-zero
+exit status."
   (with-temp-buffer
     (let ((status (apply 'call-process program nil (current-buffer) nil args)))
-      (unless (eq status 0)
-	(error "%s exited with status %s" program status))
+      (if status-handler
+	  (apply #'status-handler program args status)
+	(unless (eq status 0)
+	  (error "%s exited with status %s" program status)))
       (goto-char (point-min))
       (let (lines)
 	(while (not (eobp))
@@ -1918,6 +1924,16 @@ Signal an error if the program returns with a non-zero exit status."
 	  (forward-line 1))
 	(nreverse lines)))))
 
+(defun process-lines (program &rest args)
+  "Execute PROGRAM with ARGS, returning its output as a list of lines.
+Signal an error if the program returns with a non-zero exit status."
+  (apply #'process-lines-handling-status program nil args))
+
+(defun process-lines-ignore-status (program &rest args)
+  "Execute PROGRAM with ARGS, returning its output as a list of lines.
+The exit status of the program is ignored."
+  (apply #'process-lines-handling-status program #'identity args))
+
 (defun process-live-p (process)
   "Returns non-nil if PROCESS is alive.
 A process is considered alive if its status is `run', `open',
-- 
1.9.5 (Apple Git-50.3)

bug#1321: process-lines

[Daniel Colascione]

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

On 03/14/2015 03:36 PM, Peder O. Klingenberg wrote:
> Just adding an optional argument to process-lines is difficult
> without
breaking the API, due to its use of &rest. I suggest something along the
lines of the following patch:

What about defining a `process-lines-2' that takes keyword arguments and
accepts a list for PROGRAM?



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

bug#1321: process-lines

[Peder O. Klingenberg]

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

On 15 Mar, 2015, at 0:45, Daniel Colascione <dancol@dancol.org> wrote:
> 
> What about defining a `process-lines-2' that takes keyword arguments and
> accepts a list for PROGRAM?

That’s another possibility, of course.  But in my opinion, the &rest is usually a more convenient API for the callers, saving them the trouble of consing up a new list.  Also, I find functions with -N suffixes to be less descriptive names than they should be.

In my haste to send a patch last night, I managed to send the wrong version, one that didn’t work as advertised.  Sorry.  Please ignore my previous patch and consider this one instead:


...Peder...
-- 
Sløv uten dop

[-- Attachment #2: 0001-Extend-process-lines-to-allow-exit-status-handling-v2.patch --]
[-- Type: application/octet-stream, Size: 3299 bytes --]

From a64514df4fcd00413580d16e6418223ae0eb6ae6 Mon Sep 17 00:00:00 2001
From: "Peder O. Klingenberg" <peder@klingenberg.no>
Date: Sat, 14 Mar 2015 23:00:16 +0100
Subject: [PATCH] Extend process-lines to allow exit status handling

* subr.el (process-lines-handling-status): Extension of the old
process-lines, with more flexible handling of the exit status.
(process-lines): Old API implemented using the new function.
(process-lines-ignore-status): Another use of the new function -
return the output lines regardless of the exit status.

Fixes: Bug#1321
---
 lisp/ChangeLog |  8 ++++++++
 lisp/subr.el   | 24 ++++++++++++++++++++----
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index d393190..a93a306 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,11 @@
+2015-03-14  Peder O. Klingenberg  <peder@klingenberg.no>
+
+	* subr.el (process-lines-handling-status): Extension of the old
+	process-lines, with more flexible handling of the exit status.
+	(process-lines): Old API implemented using the new function.
+	(process-lines-ignore-status): Another use of the new function -
+	return the output lines regardless of the exit status.
+
 2015-03-13  Kevin Ryde  <user42_kevin@yahoo.com.au>
 
 	info-look fixes for Texinfo 5
diff --git a/lisp/subr.el b/lisp/subr.el
index deadca6..ec471f0 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -1901,13 +1901,19 @@ and the file name is displayed in the echo area."
 \f
 ;;;; Process stuff.
 
-(defun process-lines (program &rest args)
+(defun process-lines-handling-status (program status-handler &rest args)
   "Execute PROGRAM with ARGS, returning its output as a list of lines.
-Signal an error if the program returns with a non-zero exit status."
+If STATUS-HANDLER is non-NIL, it must be a function with one
+argument, which will be called with the exit status of the
+program before the output is collected.  If STATUS-HANDLER is
+NIL, an error is signalled if the program returns with a non-zero
+exit status."
   (with-temp-buffer
     (let ((status (apply 'call-process program nil (current-buffer) nil args)))
-      (unless (eq status 0)
-	(error "%s exited with status %s" program status))
+      (if status-handler
+	  (funcall status-handler status)
+	(unless (eq status 0)
+	  (error "%s exited with status %s" program status)))
       (goto-char (point-min))
       (let (lines)
 	(while (not (eobp))
@@ -1918,6 +1924,16 @@ Signal an error if the program returns with a non-zero exit status."
 	  (forward-line 1))
 	(nreverse lines)))))
 
+(defun process-lines (program &rest args)
+  "Execute PROGRAM with ARGS, returning its output as a list of lines.
+Signal an error if the program returns with a non-zero exit status."
+  (apply #'process-lines-handling-status program nil args))
+
+(defun process-lines-ignore-status (program &rest args)
+  "Execute PROGRAM with ARGS, returning its output as a list of lines.
+The exit status of the program is ignored."
+  (apply #'process-lines-handling-status program #'identity args))
+
 (defun process-live-p (process)
   "Returns non-nil if PROCESS is alive.
 A process is considered alive if its status is `run', `open',
-- 
1.9.5 (Apple Git-50.3)

bug#1321: process-lines

[Stefan Monnier]

> Just adding an optional argument to process-lines is difficult without
> breaking the API, due to its use of &rest.  I suggest something along the
> lines of the following patch:

FWIW, I don't really like process-lines at all.  It's used rather
rarely, so most uses would be just as happy with something like

    (with-temp-buffer
      (call-process <prog> nil t nil <args>)
      (get-buffer-lines))


-- Stefan

bug#1321: process-lines

[Lars Ingebrigtsen]

"Peder O. Klingenberg" <peder@klingenberg.no> writes:

> That’s another possibility, of course.  But in my opinion, the &rest
> is usually a more convenient API for the callers, saving them the
> trouble of consing up a new list.

Yup, makes sense to me.

Stefan raises the excellent objection that `process-lines' is pretty
trivial, and that it's not much used.  The former is definitely true,
but I wonder whether the reason it's not used much is because of the
very awkward interface.  Peder's patch fixes this by introducing a
-ignore-status version of the function (which is how it should have been
all along), so perhaps this version will get more usage.

So I've applied Peder's patch to Emacs 28.

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