bug#22679: 25.0.91; ibuffer-do-shell-command-pipe truncate output

[Tino Calancha <f92capac@gmail.com>]

On Mon, 22 Aug 2016, Stefan Monnier wrote:
> I understand.  I do not suggest to improve shell-command.
> I suggest instead to extract from shell-command a new Elisp function
> which includes the part of shell-command that you need, and then rewrite
> shell-command by making it use the new function.
>
> So shell-command would still work exactly as before, but its
> implementation would now be spread over 2 functions, the inner one of
> which would be useful to other Elisp libraries such as ibuffer.
Thank you Stefan, i understand now.  Its a good idea.
See the patch below.

> Hmm... I now see this new variable.  It has several problems indeed.
> The "set the point" part is weird and I'm not sure it makes much sense
> to fold it this way into the same var as the "don-t erase" part.
If you could write a few lines with yor main concerns about
this option here:
http://lists.gnu.org/archive/html/emacs-devel/2016-07/msg00610.html
i guess it might encourage others to give their opinion helping to decide
if we keep the option, discard it, or implement it in a better way.

> Here, for example, I'd expect maybe something like
>
>    (new-shell-command-on-region
>     (point-min) (point-max) command))
>
> Maybe with one or two new additional args.  Or maybe
>
>    (let ((out-buf (get-buffer-create shell-command-buffer-name)))
>      (with-current-buffer out-buf (goto-char (point-max)))
>      (call-shell-on-region (point-min) (point-max)
>                            out-buf command)))
I have called the new function: call-shell-region
in analogy with call-process-region; but call-shell-on-region seems
a little more descriptive.
How should i call it?
Do you thing the call-shell-region discussion should move to emacs-devel 
list?

> I think we should first aim at a simple and clean fix, yes.
Following patch, concerning just the lisp/ibuf-ext,
(the call-shell-command part eventually should go to a separated commit)
looks quite simple (compared with my first patch in this report):

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
From 9ba85fdf3c5543e87a7b1905b2404a334891581f Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha@gmail.com>
Date: Tue, 23 Aug 2016 23:46:37 +0900
Subject: [PATCH] call-shell-region: New defun

Fix Bug#22679
* lisp/subr.el (call-shell-region): New defun; execute a command
in an inferior shell with the buffer region as input.
* lisp/ibuf-ext.el (shell-command-pipe, shell-command-pipe-replace):
Use it (Bug#22679).
* lisp/simple.el (shell-command-on-region): Idem.
* lisp/ibuf-ext.el (shell-command-file):
Use call-process-shell-command instead of shell-command.
If FILE, the file that the buffer object is visiting,
exists and the buffer object is up-to-date, then use
FILE instead of creating a temporary file (Bug#22679).
* doc/lispref/processes.texi: Document call-shell-region in the manual.
;* etc/NEWS: Add entry for this new function.
---
  doc/lispref/processes.texi | 20 +++++++++++++-------
  etc/NEWS                   |  4 ++++
  lisp/ibuf-ext.el           | 33 +++++++++++++++++++--------------
  lisp/simple.el             | 36 +++++++++++++++---------------------
  lisp/subr.el               | 22 ++++++++++++++++++++++
  5 files changed, 73 insertions(+), 42 deletions(-)

diff --git a/doc/lispref/processes.texi b/doc/lispref/processes.texi
index cd12012..e043578 100644
--- a/doc/lispref/processes.texi
+++ b/doc/lispref/processes.texi
@@ -492,20 +492,17 @@ Synchronous Processes
  @end smallexample

    For example, the @code{shell-command-on-region} command uses
-@code{call-process-region} in a manner similar to this:
+@code{call-shell-region} in a manner similar to this:

  @smallexample
  @group
-(call-process-region
+(call-shell-region
   start end
- shell-file-name      ; @r{name of program}
+ command              ; @r{shell command}
   nil                  ; @r{do not delete region}
- buffer               ; @r{send output to @code{buffer}}
- nil                  ; @r{no redisplay during output}
- "-c" command)        ; @r{arguments for the shell}
+ buffer)              ; @r{send output to @code{buffer}}
  @end group
  @end smallexample
-@c It actually uses shell-command-switch, but no need to mention that 
here.
  @end defun

  @defun call-process-shell-command command &optional infile destination 
display
@@ -525,6 +522,15 @@ Synchronous Processes
  supported, but strongly discouraged.
  @end defun

+@defun call-shell-region start end command &optional delete destination
+This function sends the text from @var{start} to @var{end} as
+standard input to an inferior shell running @var{command}.  This function
+is similar than @code{call-process-region}, with process being a shell.
+The arguments @code{delete}, @code{destination} and the return value
+are like in @code{call-process-region}.
+Note that this funtion doesn't accept additional arguments.
+@end defun
+
  @defun shell-command-to-string command
  This function executes @var{command} (a string) as a shell command,
  then returns the command's output as a string.
diff --git a/etc/NEWS b/etc/NEWS
index 494a091..d30d1fa 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -56,6 +56,10 @@ affected by this, as SGI stopped supporting IRIX in 
December 2013.
  * Changes in Emacs 25.2

  +++
+** The new funtion 'call-shell-region' executes a command in an
+inferior shell with the buffer region as input.
+
++++
  ** The new user option 'shell-command-not-erase-buffer' controls
  if the output buffer is erased between shell commands; if non-nil,
  the output buffer is not erased; this variable also controls where
diff --git a/lisp/ibuf-ext.el b/lisp/ibuf-ext.el
index f93957e..a34c264 100644
--- a/lisp/ibuf-ext.el
+++ b/lisp/ibuf-ext.el
@@ -352,8 +352,10 @@ shell-command-pipe
    (:interactive "sPipe to shell command: "
     :opstring "Shell command executed on"
     :modifier-p nil)
-  (shell-command-on-region
-   (point-min) (point-max) command))
+  (let ((out-buf (get-buffer-create "*Shell Command Output*")))
+    (with-current-buffer out-buf (goto-char (point-max)))
+    (call-shell-region (point-min) (point-max)
+                       command nil out-buf)))

  ;;;###autoload (autoload 'ibuffer-do-shell-command-pipe-replace 
"ibuf-ext")
  (define-ibuffer-op shell-command-pipe-replace (command)
@@ -364,8 +366,8 @@ shell-command-pipe-replace
     :dangerous t
     :modifier-p t)
    (with-current-buffer buf
-    (shell-command-on-region (point-min) (point-max)
-			     command nil t)))
+    (call-shell-region (point-min) (point-max)
+                       command 'delete buf)))

  ;;;###autoload (autoload 'ibuffer-do-shell-command-file "ibuf-ext")
  (define-ibuffer-op shell-command-file (command)
@@ -373,16 +375,19 @@ shell-command-file
    (:interactive "sShell command on buffer's file: "
     :opstring "Shell command executed on"
     :modifier-p nil)
-  (shell-command (concat command " "
-			 (shell-quote-argument
-			  (or buffer-file-name
-			      (let ((file
-				     (make-temp-file
-				      (substring
-				       (buffer-name) 0
-				       (min 10 (length (buffer-name)))))))
-				(write-region nil nil file nil 0)
-				file))))))
+  (let ((file (and (not (buffer-modified-p))
+                   buffer-file-name))
+        (out-buf (get-buffer-create "*Shell Command Output*")))
+    (when (or (null file) (not (file-exists-p file)))
+      (setq file
+            (make-temp-file
+             (substring
+              (buffer-name) 0
+              (min 10 (length (buffer-name))))))
+      (write-region nil nil file nil 0))
+    (with-current-buffer out-buf (goto-char (point-max)))
+    (call-process-shell-command (format "%s %s" command file)
+                                nil out-buf nil)))

  ;;;###autoload (autoload 'ibuffer-do-eval "ibuf-ext")
  (define-ibuffer-op eval (form)
diff --git a/lisp/simple.el b/lisp/simple.el
index 51b24bb..dbfaae3 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -3651,10 +3651,7 @@ shell-command-on-region
                output)
            (with-temp-buffer
              (insert input)
-            (call-process-region (point-min) (point-max)
-                                 shell-file-name t t
-                                 nil shell-command-switch
-                                 command)
+            (call-shell-region (point-min) (point-max) command 'delete t)
              (setq output (split-string (buffer-string) "\n")))
            (goto-char start)
            (funcall region-insert-function output))
@@ -3667,11 +3664,10 @@ shell-command-on-region
              (goto-char start)
              (and replace (push-mark (point) 'nomsg))
              (setq exit-status
-                  (call-process-region start end shell-file-name replace
-                                       (if error-file
-                                           (list t error-file)
-                                         t)
-                                       nil shell-command-switch command))
+                  (call-shell-region start end command replace
+                                     (if error-file
+                                         (list t error-file)
+                                       t)))
              ;; It is rude to delete a buffer which the command is not 
using.
              ;; (let ((shell-buffer (get-buffer "*Shell Command 
Output*")))
              ;;   (and shell-buffer (not (eq shell-buffer 
(current-buffer)))
@@ -3694,13 +3690,12 @@ shell-command-on-region
                           (delete-region (max start end) (point-max))
                           (delete-region (point-min) (min start end))
                           (setq exit-status
-                               (call-process-region (point-min) 
(point-max)
-                                                    shell-file-name t
-                                                    (if error-file
-                                                        (list t 
error-file)
-                                                      t)
-                                                    nil 
shell-command-switch
-                                                    command)))
+                               (call-shell-region (point-min) (point-max)
+                                                  command 'delete
+                                                  (if error-file
+                                                      (list t error-file)
+                                                    t)
+                                                  )))
                  ;; Clear the output buffer, then run the command with
                  ;; output there.
                  (let ((directory default-directory))
@@ -3709,11 +3704,10 @@ shell-command-on-region
                          (setq default-directory directory))
                      (shell-command--save-pos-or-erase)))
                  (setq exit-status
-                      (call-process-region start end shell-file-name nil
-                                           (if error-file
-                                               (list buffer error-file)
-                                             buffer)
-                                           nil shell-command-switch 
command)))
+                      (call-shell-region start end command nil
+                                         (if error-file
+                                             (list buffer error-file)
+                                           buffer))))
              ;; Report the output.
              (with-current-buffer buffer
                (setq mode-line-process
diff --git a/lisp/subr.el b/lisp/subr.el
index 8ab1178..a33f997 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -3078,6 +3078,28 @@ process-file-shell-command
     infile buffer display
     (if (file-remote-p default-directory) "-c" shell-command-switch)
     (mapconcat 'identity (cons command args) " ")))
+
+(defun call-shell-region (start end command &optional delete buffer)
+"Send text from START to END as input to an inferior shell running 
COMMAND.
+Delete the text if fourth arg DELETE is non-nil.
+
+Insert output in BUFFER before point; t means current buffer; nil for
+ BUFFER means discard it; 0 means discard and don't wait; and `(:file
+ FILE)', where FILE is a file name string, means that it should be
+ written to that file (if the file already exists it is overwritten).
+BUFFER can also have the form (REAL-BUFFER STDERR-FILE); in that case,
+REAL-BUFFER says what to do with standard output, as above,
+while STDERR-FILE says what to do with standard error in the child.
+STDERR-FILE may be nil (discard standard error output),
+t (mix it with ordinary output), or a file name string.
+
+If BUFFER is 0, `call-shell-region' returns immediately with value nil.
+Otherwise it waits for COMMAND to terminate
+and returns a numeric exit status or a signal description string.
+If you quit, the process is killed with SIGINT, or SIGKILL if you quit 
again."
+(call-process-region start end
+                     shell-file-name delete buffer nil
+                     shell-command-switch command))

  ;;;; Lisp macros to do various things temporarily.

-- 
2.8.1


;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

In GNU Emacs 25.1.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.20.7)
  of 2016-08-23 built on calancha-pc
Repository revision: f345fdd7e64064194a9235406971f62b9da09ae2

Tino