From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Tino Calancha Newsgroups: gmane.emacs.bugs Subject: bug#22679: 25.0.91; ibuffer-do-shell-command-pipe truncate output Date: Sun, 21 Aug 2016 23:37:26 +0900 (JST) Message-ID: References: <7oa66k9es.fsf@fencepost.gnu.org> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; format=flowed; charset=US-ASCII X-Trace: blaine.gmane.org 1471790304 17143 195.159.176.226 (21 Aug 2016 14:38:24 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sun, 21 Aug 2016 14:38:24 +0000 (UTC) User-Agent: Alpine 2.20 (DEB 67 2015-01-07) Cc: Tino Calancha , 22679@debbugs.gnu.org To: Stefan Monnier Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Sun Aug 21 16:38:20 2016 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bbTtD-0004BI-9f for geb-bug-gnu-emacs@m.gmane.org; Sun, 21 Aug 2016 16:38:19 +0200 Original-Received: from localhost ([::1]:37139 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bbTtA-0007bH-FI for geb-bug-gnu-emacs@m.gmane.org; Sun, 21 Aug 2016 10:38:16 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:52709) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bbTt4-0007X8-6U for bug-gnu-emacs@gnu.org; Sun, 21 Aug 2016 10:38:11 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bbTsw-00079y-EO for bug-gnu-emacs@gnu.org; Sun, 21 Aug 2016 10:38:09 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:38209) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bbTsw-00079r-6L for bug-gnu-emacs@gnu.org; Sun, 21 Aug 2016 10:38:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1bbTsw-0006pr-0s for bug-gnu-emacs@gnu.org; Sun, 21 Aug 2016 10:38:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Tino Calancha Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 21 Aug 2016 14:38:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 22679 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 22679-submit@debbugs.gnu.org id=B22679.147179025926245 (code B ref 22679); Sun, 21 Aug 2016 14:38:01 +0000 Original-Received: (at 22679) by debbugs.gnu.org; 21 Aug 2016 14:37:39 +0000 Original-Received: from localhost ([127.0.0.1]:35921 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bbTsY-0006pE-Sg for submit@debbugs.gnu.org; Sun, 21 Aug 2016 10:37:39 -0400 Original-Received: from mail-pa0-f68.google.com ([209.85.220.68]:33938) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bbTsX-0006oz-1v for 22679@debbugs.gnu.org; Sun, 21 Aug 2016 10:37:37 -0400 Original-Received: by mail-pa0-f68.google.com with SMTP id hh10so6500735pac.1 for <22679@debbugs.gnu.org>; Sun, 21 Aug 2016 07:37:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:date:to:cc:subject:in-reply-to:message-id:references :user-agent:mime-version; bh=tVmhLJLQ5T/y5hQOGuIreG3g5krgYpxchQb5QC76B5s=; b=Cd868JrfdOsnsOYgSn1rM4MyaBfv6mFQ6KmZk/6llFf4EIIJCba3n1kSuSl2CvwxHf 8qqwzg4VyvwB/SwnDpogDlUfPUkjKMrpofpPJ5do8wsUrT0EfxsNtwDgIGfQLo8xLydv ErjRJKiCbStvXIZ2IcT7yjajmcZQPHGTCuPK55v1a1osc2Bwno28xEcxH5qR6/qWJuuI kYBMUlLqxyKrUBkis60ewZI8wvJ9npKRVsxeppiN5g7D/ZpM3pOM6KIz+f3S7KgLMxVQ AJxIWmIjAK/8QUyi9lkp6RE9pNyGXeOTB/wE1hiL+Lh+Kuc8uxW+Y23OJQF88eSdFoFv 6w2A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:date:to:cc:subject:in-reply-to:message-id :references:user-agent:mime-version; bh=tVmhLJLQ5T/y5hQOGuIreG3g5krgYpxchQb5QC76B5s=; b=VA7eHvJSJbX7ZB5fmivLzaPXES69UbKIBoZmJEoMVkBcqg/Y7r2m2bsUmLfrs5LBj7 b3mZlckX4ZcEl9LOPJumh6PuEye/MTaQTs8UHxVzsv845paGo09ILCdN2fruiMfp8NAX 2bz9NtgxC2rnN4qr5HqAeGSYGUFG9KDw1cTAkp1s4RByibiZ6+agMSdiPTTiusO4Jukj u6e0+A/WecTKkawKTVqqv0o5bIyN+X1xEYsFVTNtfZUuqqxCfxyJ37xRdnncppGf4OXA w+JFx8NurxiuMfkkHYzMMa1FcZO8ZGDzJWrN+QtSuNZ8pKJbbEfKXGvIrA3XNNXrMppf QHCQ== X-Gm-Message-State: AEkooutev++Zou2cshML2qdc2XNsAgi8StGHJjvU1cKDPfYg+tQ6yUCJaWL9sXRz6jkihQ== X-Received: by 10.66.0.231 with SMTP id 7mr33475255pah.118.1471790251087; Sun, 21 Aug 2016 07:37:31 -0700 (PDT) Original-Received: from calancha-pc (mail.family-wifi.jp. [180.42.51.5]) by smtp.gmail.com with ESMTPSA id k9sm20541471pfj.63.2016.08.21.07.37.28 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 21 Aug 2016 07:37:30 -0700 (PDT) X-Google-Original-From: Tino Calancha X-X-Sender: calancha@calancha-pc In-Reply-To: X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:122452 Archived-At: 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 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