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

[Tino Calancha]

On Sat, 20 Aug 2016, Stefan Monnier wrote:

Thank you very much for your time reviewing my patch.

> Thanks.  Now that I looked at the rest of the patch I see that you use
> some kind of after/before hooks, so this approach doesn't really lend
> itself to extracting into a more generally useful function which could
> be used by shell-command (and others).
Improving `shell-command' is out of the scope of my patch.  The goal is
to avoid that these commands truncate the output: currently the output
buffer just show the output for the last processed buffer.  The reason
to that is that original implementation uses `shell-command', which used
to erase the output buffer between two calls.
That raised the question about if it would have sense to allow not erasing
the output buffer between 2 `shell-comand' calls.
In fact, following your proposal to not use `shell-command' at all, it 
would
also prevent erasing the output buffer: that would be a cleaner patch
(see below).

> Could you explain what made you use this approach, to see if there might
> be some way to solve the problem while still making the code more
> generally useful (and reduce redundancy rather than augment it)?

I used the hooks (*) because at some point i thought that one user setting
'shell-command-not-erase-buffer' to a non-nil value,
may expect that these ibuffer commands also set the point
in the output buffer according with
'shell-command-not-erase-buffer'.

If we decide that the behaviour of these commands should not depend
on 'shell-command-not-erase-buffer', then a cleaner fix could be
as follows:

(*) Even if adding the hooks for fixing this bug is not well motivated,
i think `define-ibuffer-op' results a more flexible macro with
the addition of BEFORE and AFTER.  Maybe it could simplify some other
parts of the code or future extensions.  This is something i may
study separately.


;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
> From 2d2908b33b14a47b2afb077538f6f14735b30a54 Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha@gmail.com>
Date: Sun, 21 Aug 2016 23:20:52 +0900
Subject: [PATCH] Fix Bug#22679

* lisp/ibuf-ext.el (shell-command-pipe)
(shell-command-pipe-replace): Use 'call-process-region'
instead of 'shell-command' or 'shell-command-on-region'.
(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.
---
 lisp/ibuf-ext.el | 36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/lisp/ibuf-ext.el b/lisp/ibuf-ext.el
index f93957e..99ae400 100644
--- a/lisp/ibuf-ext.el
+++ b/lisp/ibuf-ext.el
@@ -352,8 +352,11 @@ 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-process-region (point-min) (point-max)
+                         shell-file-name nil out-buf nil
+                         shell-command-switch command)))

 ;;;###autoload (autoload 'ibuffer-do-shell-command-pipe-replace 
"ibuf-ext")
 (define-ibuffer-op shell-command-pipe-replace (command)
@@ -363,9 +366,9 @@ shell-command-pipe-replace
    :active-opstring "replace buffer contents in"
    :dangerous t
    :modifier-p t)
-  (with-current-buffer buf
-    (shell-command-on-region (point-min) (point-max)
-                            command nil t)))
+  (call-process-region (point-min) (point-max)
+                       shell-file-name t buf nil
+                       shell-command-switch command))

 ;;;###autoload (autoload 'ibuffer-do-shell-command-file "ibuf-ext")
 (define-ibuffer-op shell-command-file (command)
@@ -373,16 +376,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)
--
2.8.1

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

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

Tino