* bug#22679: 25.0.91; ibuffer-do-shell-command-pipe truncate output @ 2016-02-15 13:21 Tino Calancha 2016-06-10 5:02 ` Glenn Morris 0 siblings, 1 reply; 23+ messages in thread From: Tino Calancha @ 2016-02-15 13:21 UTC (permalink / raw) To: 22679 [-- Attachment #1: Type: text/plain, Size: 456 bytes --] Only the output of the last processed buffer is shown. Maybe better if it is shown the concatenation of the output of all of them. emacs -Q --eval="(mapc (lambda(x) (switch-to-buffer x) (insert x)) '(\"foo\" \"bar\"))" --eval='(ibuffer)' % n ^\(foo\|bar\)$ RET | cat RET g C-x b * ibuffer-shell-output* ;;just show one word In GNU Emacs 25.0.91.1 (x86_64-pc-linux-gnu, GTK+ Version 2.24.29) Repository revision: 23ca48d3d867cfff9f49ef600e2aad7a26c7a870 [-- Attachment #2: Type: text/plain, Size: 2368 bytes --] diff --git a/lisp/ibuf-ext.el b/lisp/ibuf-ext.el index f537561..8b1d4d2 100644 --- a/lisp/ibuf-ext.el +++ b/lisp/ibuf-ext.el @@ -323,9 +323,15 @@ 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 - (get-buffer-create "* ibuffer-shell-output*"))) + (let* ((out-buf (get-buffer-create "* ibuffer-shell-output*")) + (buffers (with-current-buffer "*Ibuffer*" (ibuffer-get-marked-buffers)))) + (let ((string (with-temp-buffer + (insert-buffer-substring buf) + (shell-command-on-region (point-min) (point-max) command nil t) + (buffer-substring-no-properties (point-min) (point-max))))) + (with-current-buffer out-buf + (when (eq buf (car buffers)) (erase-buffer)) + (insert string))))) ;;;###autoload (autoload 'ibuffer-do-shell-command-pipe-replace "ibuf-ext") (define-ibuffer-op shell-command-pipe-replace (command) @@ -345,12 +351,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 - (if buffer-file-name - buffer-file-name - (make-temp-file - (substring (buffer-name) 0 (min 10 (length (buffer-name)))))))))) + (let* ((out-buf (get-buffer-create "*Shell Command Output*")) ; This function and shell-command-pipe + ; should have same out buffer name. + (buffers (with-current-buffer "*Ibuffer*" (ibuffer-get-marked-buffers)))) + (let* ((fname (or buffer-file-name + (let ((file (make-temp-file + (substring (buffer-name) 0 (min 10 (length (buffer-name))))))) + (write-region (point-min) (point-max) file) file))) + (cmd (format "%s %s" command (shell-quote-argument fname))) + (string (shell-command-to-string cmd))) + + (with-current-buffer out-buf + (when (eq buf (car buffers)) (erase-buffer)) + (insert string))))) ;;;###autoload (autoload 'ibuffer-do-eval "ibuf-ext") (define-ibuffer-op eval (form) ^ permalink raw reply related [flat|nested] 23+ messages in thread
* bug#22679: 25.0.91; ibuffer-do-shell-command-pipe truncate output 2016-02-15 13:21 bug#22679: 25.0.91; ibuffer-do-shell-command-pipe truncate output Tino Calancha @ 2016-06-10 5:02 ` Glenn Morris [not found] ` <CAMn5WmYDL0CrDppLc_Vs+EM5CkA_wfdb1z+QCmNj_=v6PiYM-g@mail.gmail.com> 2016-06-11 3:48 ` C. Calancha 0 siblings, 2 replies; 23+ messages in thread From: Glenn Morris @ 2016-06-10 5:02 UTC (permalink / raw) To: Tino Calancha; +Cc: 22679 This seems like it would be a lot simpler if shell-command(-on-region) did not unconditionally erase its output buffer. Although that behaviour is long-standing, it seems unfriendly. It would be easier for callers that wanted that to erase their own output buffers. It's less simple for callers that want to preserve existing output to do so with the current system. ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <CAMn5WmYDL0CrDppLc_Vs+EM5CkA_wfdb1z+QCmNj_=v6PiYM-g@mail.gmail.com>]
* bug#22679: 25.0.91; ibuffer-do-shell-command-pipe truncate output [not found] ` <CAMn5WmYDL0CrDppLc_Vs+EM5CkA_wfdb1z+QCmNj_=v6PiYM-g@mail.gmail.com> @ 2016-06-10 9:08 ` Tino Calancha 2016-07-05 15:58 ` Glenn Morris 0 siblings, 1 reply; 23+ messages in thread From: Tino Calancha @ 2016-06-10 9:08 UTC (permalink / raw) To: 22679, Tino Calancha [-- Attachment #1: Type: text/plain, Size: 13605 bytes --] >This seems like it would be a lot simpler if shell-command(-on-region) >did not unconditionally erase its output buffer. Although that behaviour >is long-standing, it seems unfriendly. It would be easier for callers >that wanted that to erase their own output buffers. It's less simple for >callers that want to preserve existing output to do so with the current >system. Adding an extra optional arg KEEP to shell-command family would do the job straightforward (see below patch). Maybe someone may complaint about one function having 9 (***) arguments (shell-command-on-region) inside a file called 'simple'. (***) I have noticed arg REGION-NONCONTIGUOUS-P is not mentioned in the doc string. ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; From b14efc632dfafbbc61863c060b9840a752704320 Mon Sep 17 00:00:00 2001 From: Tino Calancha <f92capac@gmail.com> Date: Fri, 10 Jun 2016 17:44:48 +0900 Subject: [PATCH 1/2] Allow not erasing output buffer on shell-command * lisp/simple.el (async-shell-command) (shell-command, shell-command-on-region): Added optional arg KEEP (Bug#22679). --- lisp/simple.el | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/lisp/simple.el b/lisp/simple.el index 6c30929..59fa851 100644 --- a/lisp/simple.el +++ b/lisp/simple.el @@ -3187,7 +3187,7 @@ async-shell-command-buffer :group 'shell :version "24.3") -(defun async-shell-command (command &optional output-buffer error-buffer) +(defun async-shell-command (command &optional output-buffer error-buffer keep) "Execute string COMMAND asynchronously in background. Like `shell-command', but adds `&' at the end of COMMAND @@ -3218,9 +3218,9 @@ async-shell-command shell-command-default-error-buffer)) (unless (string-match "&[ \t]*\\'" command) (setq command (concat command " &"))) - (shell-command command output-buffer error-buffer)) + (shell-command command output-buffer error-buffer keep)) -(defun shell-command (command &optional output-buffer error-buffer) +(defun shell-command (command &optional output-buffer error-buffer keep) "Execute string COMMAND in inferior shell; display output, if any. With prefix argument, insert the COMMAND's output at point. @@ -3274,6 +3274,9 @@ shell-command In an interactive call, the variable `shell-command-default-error-buffer' specifies the value of ERROR-BUFFER. +If the optional fourth argument KEEP is non-nil, the output buffer +is not erased before inserting the output. + In Elisp, you will often be better served by calling `call-process' or `start-process' directly, since it offers more control and does not impose the use of a shell (with its need to quote arguments)." @@ -3391,7 +3394,7 @@ shell-command ;; if some text has a non-nil read-only property, ;; which comint sometimes adds for prompts. (let ((inhibit-read-only t)) - (erase-buffer)) + (or keep (erase-buffer))) (display-buffer buffer '(nil (allow-no-window . t))) (setq default-directory directory) (setq proc (start-process "Shell" buffer shell-file-name @@ -3405,7 +3408,7 @@ shell-command )) ;; Otherwise, command is executed synchronously. (shell-command-on-region (point) (point) command - output-buffer nil error-buffer))))))) + output-buffer nil error-buffer nil nil keep))))))) (defun display-message-or-buffer (message &optional buffer-name action frame) "Display MESSAGE in the echo area if possible, otherwise in a pop-up buffer. @@ -3485,7 +3488,7 @@ shell-command-sentinel (defun shell-command-on-region (start end command &optional output-buffer replace error-buffer display-error-buffer - region-noncontiguous-p) + region-noncontiguous-p keep) "Execute string COMMAND in inferior shell with region as input. Normally display output (if any) in temp buffer `*Shell Command Output*'; Prefix arg means replace the region with it. Return the exit code of @@ -3533,7 +3536,10 @@ shell-command-on-region Optional seventh arg DISPLAY-ERROR-BUFFER, if non-nil, means to display the error buffer if there were any errors. When called -interactively, this is t." +interactively, this is t. + +Optional nineth arg KEEP, if non-nil, then the output buffer is +not erased before inserting the output." (interactive (let (string) (unless (mark) (user-error "The mark is not set now, so there is no region")) @@ -3618,7 +3624,9 @@ shell-command-on-region (setq buffer-read-only nil) (if (not output-buffer) (setq default-directory directory)) - (erase-buffer))) + (if keep + (goto-char (point-max)) + (erase-buffer)))) (setq exit-status (call-process-region start end shell-file-name nil (if error-file -- 2.8.1 From 56f57e6321a7e37389329c6f8c54c340d12ee419 Mon Sep 17 00:00:00 2001 From: Tino Calancha <f92capac@gmail.com> Date: Fri, 10 Jun 2016 17:56:30 +0900 Subject: [PATCH 2/2] Do not truncate output (Bug#22679) * lisp/ibuf-ext.el (shell-command-pipe, shell-command-file): --- lisp/ibuf-ext.el | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lisp/ibuf-ext.el b/lisp/ibuf-ext.el index 0baab6b..9dd1eea 100644 --- a/lisp/ibuf-ext.el +++ b/lisp/ibuf-ext.el @@ -325,7 +325,8 @@ shell-command-pipe :modifier-p nil) (shell-command-on-region (point-min) (point-max) command - (get-buffer-create "* ibuffer-shell-output*"))) + (get-buffer-create "* ibuffer-shell-output*") + nil nil nil nil 'keep)) ;;;###autoload (autoload 'ibuffer-do-shell-command-pipe-replace "ibuf-ext") (define-ibuffer-op shell-command-pipe-replace (command) @@ -354,7 +355,7 @@ shell-command-file (buffer-name) 0 (min 10 (length (buffer-name))))))) (write-region nil nil file nil 0) - file)))))) + file)))) nil nil 'keep)) ;;;###autoload (autoload 'ibuffer-do-eval "ibuf-ext") (define-ibuffer-op eval (form) -- 2.8.1 On Fri, Jun 10, 2016 at 6:04 PM, Tino Calancha <f92capac@gmail.com> wrote: > >This seems like it would be a lot simpler if shell-command(-on-region) > >did not unconditionally erase its output buffer. Although that behaviour > >is long-standing, it seems unfriendly. It would be easier for callers > >that wanted that to erase their own output buffers. It's less simple for > >callers that want to preserve existing output to do so with the current > >system. > > Adding an extra optional arg KEEP to shell-command family would do the job > straightforward (see below patch). Maybe someone may complaint about one > function having 9 (***) arguments (shell-command-on-region) inside a file > called > 'simple'. > > (***) I have noticed arg REGION-NONCONTIGUOUS-P is not mentioned in the > doc string. > > > ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; > > From b14efc632dfafbbc61863c060b9840a752704320 Mon Sep 17 00:00:00 2001 > From: Tino Calancha <f92capac@gmail.com> > Date: Fri, 10 Jun 2016 17:44:48 +0900 > Subject: [PATCH 1/2] Allow not erasing output buffer on shell-command > > * lisp/simple.el (async-shell-command) > (shell-command, shell-command-on-region): Added optional > arg KEEP (Bug#22679). > --- > lisp/simple.el | 24 ++++++++++++++++-------- > 1 file changed, 16 insertions(+), 8 deletions(-) > > diff --git a/lisp/simple.el b/lisp/simple.el > index 6c30929..59fa851 100644 > --- a/lisp/simple.el > +++ b/lisp/simple.el > @@ -3187,7 +3187,7 @@ async-shell-command-buffer > :group 'shell > :version "24.3") > > -(defun async-shell-command (command &optional output-buffer error-buffer) > +(defun async-shell-command (command &optional output-buffer error-buffer > keep) > "Execute string COMMAND asynchronously in background. > > Like `shell-command', but adds `&' at the end of COMMAND > @@ -3218,9 +3218,9 @@ async-shell-command > shell-command-default-error-buffer)) > (unless (string-match "&[ \t]*\\'" command) > (setq command (concat command " &"))) > - (shell-command command output-buffer error-buffer)) > + (shell-command command output-buffer error-buffer keep)) > > -(defun shell-command (command &optional output-buffer error-buffer) > +(defun shell-command (command &optional output-buffer error-buffer keep) > "Execute string COMMAND in inferior shell; display output, if any. > With prefix argument, insert the COMMAND's output at point. > > @@ -3274,6 +3274,9 @@ shell-command > In an interactive call, the variable `shell-command-default-error-buffer' > specifies the value of ERROR-BUFFER. > > +If the optional fourth argument KEEP is non-nil, the output buffer > +is not erased before inserting the output. > + > In Elisp, you will often be better served by calling `call-process' or > `start-process' directly, since it offers more control and does not impose > the use of a shell (with its need to quote arguments)." > @@ -3391,7 +3394,7 @@ shell-command > ;; if some text has a non-nil read-only property, > ;; which comint sometimes adds for prompts. > (let ((inhibit-read-only t)) > - (erase-buffer)) > + (or keep (erase-buffer))) > (display-buffer buffer '(nil (allow-no-window . t))) > (setq default-directory directory) > (setq proc (start-process "Shell" buffer shell-file-name > @@ -3405,7 +3408,7 @@ shell-command > )) > ;; Otherwise, command is executed synchronously. > (shell-command-on-region (point) (point) command > - output-buffer nil error-buffer))))))) > + output-buffer nil error-buffer nil nil keep))))))) > > (defun display-message-or-buffer (message &optional buffer-name action > frame) > "Display MESSAGE in the echo area if possible, otherwise in a pop-up > buffer. > @@ -3485,7 +3488,7 @@ shell-command-sentinel > (defun shell-command-on-region (start end command > &optional output-buffer replace > error-buffer display-error-buffer > - region-noncontiguous-p) > + region-noncontiguous-p keep) > "Execute string COMMAND in inferior shell with region as input. > Normally display output (if any) in temp buffer `*Shell Command Output*'; > Prefix arg means replace the region with it. Return the exit code of > @@ -3533,7 +3536,10 @@ shell-command-on-region > > Optional seventh arg DISPLAY-ERROR-BUFFER, if non-nil, means to > display the error buffer if there were any errors. When called > -interactively, this is t." > +interactively, this is t. > + > +Optional nineth arg KEEP, if non-nil, then the output buffer is > +not erased before inserting the output." > (interactive (let (string) > (unless (mark) > (user-error "The mark is not set now, so there is no region")) > @@ -3618,7 +3624,9 @@ shell-command-on-region > (setq buffer-read-only nil) > (if (not output-buffer) > (setq default-directory directory)) > - (erase-buffer))) > + (if keep > + (goto-char (point-max)) > + (erase-buffer)))) > (setq exit-status > (call-process-region start end shell-file-name nil > (if error-file > -- > 2.8.1 > > From 56f57e6321a7e37389329c6f8c54c340d12ee419 Mon Sep 17 00:00:00 2001 > From: Tino Calancha <f92capac@gmail.com> > Date: Fri, 10 Jun 2016 17:56:30 +0900 > Subject: [PATCH 2/2] Do not truncate output (Bug#22679) > > * lisp/ibuf-ext.el (shell-command-pipe, shell-command-file): > --- > lisp/ibuf-ext.el | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/lisp/ibuf-ext.el b/lisp/ibuf-ext.el > index 0baab6b..9dd1eea 100644 > --- a/lisp/ibuf-ext.el > +++ b/lisp/ibuf-ext.el > @@ -325,7 +325,8 @@ shell-command-pipe > :modifier-p nil) > (shell-command-on-region > (point-min) (point-max) command > - (get-buffer-create "* ibuffer-shell-output*"))) > + (get-buffer-create "* ibuffer-shell-output*") > + nil nil nil nil 'keep)) > > ;;;###autoload (autoload 'ibuffer-do-shell-command-pipe-replace > "ibuf-ext") > (define-ibuffer-op shell-command-pipe-replace (command) > @@ -354,7 +355,7 @@ shell-command-file > (buffer-name) 0 > (min 10 (length (buffer-name))))))) > (write-region nil nil file nil 0) > - file)))))) > + file)))) nil nil 'keep)) > > ;;;###autoload (autoload 'ibuffer-do-eval "ibuf-ext") > (define-ibuffer-op eval (form) > -- > 2.8.1 > > > > On Fri, Jun 10, 2016 at 2:02 PM, Glenn Morris <rgm@gnu.org> wrote: > >> >> This seems like it would be a lot simpler if shell-command(-on-region) >> did not unconditionally erase its output buffer. Although that behaviour >> is long-standing, it seems unfriendly. It would be easier for callers >> that wanted that to erase their own output buffers. It's less simple for >> callers that want to preserve existing output to do so with the current >> system. >> > > [-- Attachment #2: Type: text/html, Size: 16848 bytes --] ^ permalink raw reply related [flat|nested] 23+ messages in thread
* bug#22679: 25.0.91; ibuffer-do-shell-command-pipe truncate output 2016-06-10 9:08 ` Tino Calancha @ 2016-07-05 15:58 ` Glenn Morris 2016-07-05 16:27 ` Tino Calancha 0 siblings, 1 reply; 23+ messages in thread From: Glenn Morris @ 2016-07-05 15:58 UTC (permalink / raw) To: Tino Calancha; +Cc: 22679 Tino Calancha wrote: > Adding an extra optional arg KEEP to shell-command family would do the > job straightforward (see below patch). Maybe someone may complaint > about one function having 9 (***) arguments (shell-command-on-region) > inside a file called 'simple'. Yes, I don't know if it is worth adding such an argument. I was really just complaining about how the function works. :) ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#22679: 25.0.91; ibuffer-do-shell-command-pipe truncate output 2016-07-05 15:58 ` Glenn Morris @ 2016-07-05 16:27 ` Tino Calancha 2016-07-09 17:28 ` Glenn Morris 0 siblings, 1 reply; 23+ messages in thread From: Tino Calancha @ 2016-07-05 16:27 UTC (permalink / raw) To: Glenn Morris; +Cc: Tino Calancha, 22679 On Tue, 5 Jul 2016, Glenn Morris wrote: > Yes, I don't know if it is worth adding such an argument. > I was really just complaining about how the function works. :) I like your proposal: it simplifies the code and it reads much better. But i am quite new here as to modify that stuff in simple.el: some people may not understand why to change `shell-command' just to fix one marginal command that just few people use. Of course the priority is to make the command works as it should. If i need to fix this i think i would find less resistance if my patch just modify the offending command, possibly with an ugly code. Once the bug is fixed, then someone with much credits may think to write a more elegant solution. ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#22679: 25.0.91; ibuffer-do-shell-command-pipe truncate output 2016-07-05 16:27 ` Tino Calancha @ 2016-07-09 17:28 ` Glenn Morris 2016-07-13 15:27 ` Stefan Monnier 0 siblings, 1 reply; 23+ messages in thread From: Glenn Morris @ 2016-07-09 17:28 UTC (permalink / raw) To: Tino Calancha; +Cc: 22679 Tino Calancha wrote: > On Tue, 5 Jul 2016, Glenn Morris wrote: > >> Yes, I don't know if it is worth adding such an argument. >> I was really just complaining about how the function works. :) > I like your proposal: it simplifies the code and it reads much better. > But i am quite new here as to modify that stuff in simple.el: > some people may not understand why to change `shell-command' just > to fix one marginal command that just few people use. You could ask on emacs-devel (though that is often unproductive). Frankly my own preference would be to change the function so that it does not erase the output buffer, and change the callers instead; but that may be totally unfeasible. > Of course the priority is to make the command works as it should. > If i need to fix this i think i would find less resistance if my patch > just modify the offending command, possibly with an ugly code. You should feel free to just change ibuffer if that is what you prefer. It will certainly be less hassle for you. ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#22679: 25.0.91; ibuffer-do-shell-command-pipe truncate output 2016-07-09 17:28 ` Glenn Morris @ 2016-07-13 15:27 ` Stefan Monnier 2016-08-19 8:33 ` Tino Calancha 0 siblings, 1 reply; 23+ messages in thread From: Stefan Monnier @ 2016-07-13 15:27 UTC (permalink / raw) To: Glenn Morris; +Cc: Tino Calancha, 22679 > You could ask on emacs-devel (though that is often unproductive). > Frankly my own preference would be to change the function so that it > does not erase the output buffer, and change the callers instead; but > that may be totally unfeasible. shell-command is designed for interactive use. In 90% of the cases, Elisp code that uses shell-command would be just as well, if not better, served by start/call-process. Maybe the better change is to create a new function (partly extracted from shell-command) which would be halfway between shell-command and start-process: i.e. designed for use from Elisp, but specifically tailored to running shell code rather than some other executable. Then use that function in shell-command and ibuffer-do-shell-command-pipe. >> Of course the priority is to make the command works as it should. >> If i need to fix this i think i would find less resistance if my patch >> just modify the offending command, possibly with an ugly code. If the new function is well designed, the new ibuffer-do-shell-command-pipe code shouldn't be ugly. Stefan ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#22679: 25.0.91; ibuffer-do-shell-command-pipe truncate output 2016-07-13 15:27 ` Stefan Monnier @ 2016-08-19 8:33 ` Tino Calancha 2016-08-19 13:52 ` Stefan Monnier 0 siblings, 1 reply; 23+ messages in thread From: Tino Calancha @ 2016-08-19 8:33 UTC (permalink / raw) To: Stefan Monnier; +Cc: Tino Calancha, 22679 On Wed, 13 Jul 2016, Stefan Monnier wrote: > shell-command is designed for interactive use. > > In 90% of the cases, Elisp code that uses shell-command would be just as > well, if not better, served by start/call-process. > > Maybe the better change is to create a new function (partly extracted > from shell-command) which would be halfway between shell-command and > start-process: i.e. designed for use from Elisp, but specifically > tailored to running shell code rather than some other executable. > > Then use that function in shell-command and ibuffer-do-shell-command-pipe. Hi Stefan, thanks for the indications. I would like to apply following patch, which avoid using 'shell-command': ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; From 49a77e617b26044c84f193753f1eb7ec4ccea5d8 Mon Sep 17 00:00:00 2001 From: Tino Calancha <tino.calancha@gmail.com> Date: Fri, 19 Aug 2016 16:46:14 +0900 Subject: [PATCH] ibuffer-do-shell-command-pipe: Do not truncate output Fix Bug#22679 * lisp/ibuf-macs.el (define-ibuffer-op): Added optional args 'before' and 'after'. 'before' is a form to evaluate before the operation. 'after' is a form to evaluate after the operation. * lisp/ibuf-ext.el (ibuffer--after-shell-command-pos): New defvar; store a buffer position where to set the point in the output buffer after operation complete. (ibuffer--before-shell-command): New defun; erase output buffer if 'shell-command-not-erase-buffer' is nil and set 'ibuffer--after-shell-command-pos'. (ibuffer--after-shell-command): New defun; set point in the output buffer after operation complete. (ibuffer-do-shell-command-pipe, ibuffer-do-shell-command-file): Bind 'shell-command-not-erase-buffer' to non-nil while processing the buffers; use 'ibuffer--after-shell-command' to set the point in the output buffer. --- lisp/ibuf-ext.el | 83 +++++++++++++++++++++++++++++++++++++++++++------------ lisp/ibuf-macs.el | 8 +++++- 2 files changed, 72 insertions(+), 19 deletions(-) diff --git a/lisp/ibuf-ext.el b/lisp/ibuf-ext.el index f93957e..333104d 100644 --- a/lisp/ibuf-ext.el +++ b/lisp/ibuf-ext.el @@ -346,14 +346,58 @@ ibuffer-backward-filter-group (ibuffer-backward-filter-group 1)) (ibuffer-forward-line 0)) +;; The value `beg-last-out' in `shell-command-not-erase-buffer' +;; set the point at the beginning of the output of the first +;; buffer processed. +(defvar ibuffer--after-shell-command-pos) + +(defun ibuffer--before-shell-command () + (let ((obuf (get-buffer-create "*Shell Command Output*")) + (sym shell-command-not-erase-buffer) + final-pos) + (when (buffer-live-p obuf) + (with-current-buffer obuf + (unless sym + (setq buffer-read-only nil) + (let ((inhibit-read-only t)) + (erase-buffer))) + (setq final-pos + (cond ((or (not sym) (eq sym 'beg-last-out)) + (point-max)) + ((eq sym 'save-point) + (point)))) + (setq ibuffer--after-shell-command-pos + final-pos))))) + +(defun ibuffer--after-shell-command () + (let* ((obuf (get-buffer-create "*Shell Command Output*")) + (pos ibuffer--after-shell-command-pos) + (win (car (get-buffer-window-list obuf)))) + (setq ibuffer--after-shell-command-pos nil) + (with-current-buffer obuf + (unless pos (setq pos (point-max))) + (goto-char pos) + ;; Set point in the window displaying obuf, if any; otherwise + ;; display buf temporary in selected frame and set the point. + (if win + (set-window-point win pos) + (save-window-excursion + (let ((win (display-buffer obuf '(nil (inhibit-switch-frame . t))))) + (set-window-point win pos))))))) + ;;;###autoload (autoload 'ibuffer-do-shell-command-pipe "ibuf-ext") (define-ibuffer-op shell-command-pipe (command) "Pipe the contents of each marked buffer to shell command COMMAND." (:interactive "sPipe to shell command: " :opstring "Shell command executed on" - :modifier-p nil) - (shell-command-on-region - (point-min) (point-max) command)) + :modifier-p nil + :opstring "Shell command executed on" + :modifier-p nil + :before (funcall #'ibuffer--before-shell-command) + :after (funcall #'ibuffer--after-shell-command)) + (let ((out-buf (get-buffer "*Shell Command Output*"))) + (with-current-buffer out-buf (goto-char (point-max))) + (call-process-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) @@ -363,26 +407,29 @@ 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) command t buf)) ;;;###autoload (autoload 'ibuffer-do-shell-command-file "ibuf-ext") (define-ibuffer-op shell-command-file (command) "Run shell command COMMAND separately on files of marked buffers." (: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)))))) + :opstring "Shell command executed on" + :modifier-p nil + :before (funcall #'ibuffer--before-shell-command) + :after (funcall #'ibuffer--after-shell-command)) + (let ((file (and (not (buffer-modified-p)) + buffer-file-name)) + (out-buf (get-buffer "*Shell Command Output*"))) + (unless 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/ibuf-macs.el b/lisp/ibuf-macs.el index 27e7af9..8bb05ec 100644 --- a/lisp/ibuf-macs.el +++ b/lisp/ibuf-macs.el @@ -169,6 +169,8 @@ ibuffer-save-marks dangerous (opstring "operated on") (active-opstring "Operate on") + before + after complex) &rest body) "Generate a function which operates on a buffer. @@ -198,6 +200,8 @@ ibuffer-save-marks ACTIVE-OPSTRING is a string which will be displayed to the user in a confirmation message, in the form: \"Really ACTIVE-OPSTRING x buffers?\" +BEFORE is a form to evaluate before start the operation. +AFTER is a form to evaluate once the operation is complete. COMPLEX means this function is special; if COMPLEX is nil BODY evaluates once for each marked buffer, MBUF, with MBUF current and saving the point. If COMPLEX is non-nil, BODY evaluates @@ -206,7 +210,7 @@ ibuffer-save-marks marked buffer. BODY is evaluated with `buf' bound to the buffer object. -\(fn OP ARGS DOCUMENTATION (&key INTERACTIVE MARK MODIFIER-P DANGEROUS OPSTRING ACTIVE-OPSTRING COMPLEX) &rest BODY)" +\(fn OP ARGS DOCUMENTATION (&key INTERACTIVE MARK MODIFIER-P DANGEROUS OPSTRING ACTIVE-OPSTRING BEFORE AFTER COMPLEX) &rest BODY)" (declare (indent 2) (doc-string 3)) `(progn (defun ,(intern (concat (if (string-match "^ibuffer-do" (symbol-name op)) @@ -233,11 +237,13 @@ ibuffer-save-marks 'ibuffer-deletion-char) (_ 'ibuffer-marked-char)))) + ,before ; pre-operation form. ,(let* ((finish (append '(progn) (if (eq modifier-p t) '((setq ibuffer-did-modification t)) ()) + (and after `(,after)) ; post-operation form. `((ibuffer-redisplay t) (message ,(concat "Operation finished; " opstring " %s buffers") count)))) (inner-body (if complex -- 2.8.1 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; In GNU Emacs 25.1.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.20.7) of 2016-08-18 built on calancha-pc Repository revision: a4fa31150f186611ad083c3387e3cb2c5d25f991 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* bug#22679: 25.0.91; ibuffer-do-shell-command-pipe truncate output 2016-08-19 8:33 ` Tino Calancha @ 2016-08-19 13:52 ` Stefan Monnier 2016-08-20 3:28 ` Tino Calancha 2016-08-20 10:28 ` Tino Calancha 0 siblings, 2 replies; 23+ messages in thread From: Stefan Monnier @ 2016-08-19 13:52 UTC (permalink / raw) To: Tino Calancha; +Cc: 22679 > - :modifier-p nil) > - (shell-command-on-region > - (point-min) (point-max) command)) > + :modifier-p nil > + :opstring "Shell command executed on" > + :modifier-p nil > + :before (funcall #'ibuffer--before-shell-command) > + :after (funcall #'ibuffer--after-shell-command)) > + (let ((out-buf (get-buffer "*Shell Command Output*"))) > + (with-current-buffer out-buf (goto-char (point-max))) > + (call-process-region (point-min) (point-max) command nil out-buf))) I haven't looked at the rest of your patch but this part looks wrong: the docstring indicates that `command' is expected to be a shell command whereas call-process-region expects an executable. Stefan ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#22679: 25.0.91; ibuffer-do-shell-command-pipe truncate output 2016-08-19 13:52 ` Stefan Monnier @ 2016-08-20 3:28 ` Tino Calancha 2016-08-20 10:28 ` Tino Calancha 1 sibling, 0 replies; 23+ messages in thread From: Tino Calancha @ 2016-08-20 3:28 UTC (permalink / raw) To: Stefan Monnier; +Cc: 22679 On 08/19/2016 10:52 PM, Stefan Monnier wrote: >> - :modifier-p nil) >> - (shell-command-on-region >> - (point-min) (point-max) command)) >> + :modifier-p nil >> + :opstring "Shell command executed on" >> + :modifier-p nil >> + :before (funcall #'ibuffer--before-shell-command) >> + :after (funcall #'ibuffer--after-shell-command)) >> + (let ((out-buf (get-buffer "*Shell Command Output*"))) >> + (with-current-buffer out-buf (goto-char (point-max))) >> + (call-process-region (point-min) (point-max) command nil out-buf))) > I haven't looked at the rest of your patch but this part looks wrong: > the docstring indicates that `command' is expected to be a shell command > whereas call-process-region expects an executable. > > > Stefan Thank you very much. I didn't notice this because i was testing with and executable (cat) as the command. I will start working in a new patch. ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#22679: 25.0.91; ibuffer-do-shell-command-pipe truncate output 2016-08-19 13:52 ` Stefan Monnier 2016-08-20 3:28 ` Tino Calancha @ 2016-08-20 10:28 ` Tino Calancha 2016-08-20 12:46 ` Stefan Monnier 1 sibling, 1 reply; 23+ messages in thread From: Tino Calancha @ 2016-08-20 10:28 UTC (permalink / raw) To: Stefan Monnier; +Cc: Tino Calancha, 22679 On Fri, 19 Aug 2016, Stefan Monnier wrote: >> - :modifier-p nil) >> - (shell-command-on-region >> - (point-min) (point-max) command)) >> + :modifier-p nil >> + :opstring "Shell command executed on" >> + :modifier-p nil >> + :before (funcall #'ibuffer--before-shell-command) >> + :after (funcall #'ibuffer--after-shell-command)) >> + (let ((out-buf (get-buffer "*Shell Command Output*"))) >> + (with-current-buffer out-buf (goto-char (point-max))) >> + (call-process-region (point-min) (point-max) command nil out-buf))) > > I haven't looked at the rest of your patch but this part looks wrong: > the docstring indicates that `command' is expected to be a shell command > whereas call-process-region expects an executable. I have corrected the call to `call-process-region': now it uses 'shell-file-name' as the executable. I have also added one test for this bug that the new patch pass. This test is just for documentation: i don't want to push it to the master branch because it assumes the machine has an 'awk' executable in 'exec-path'. ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; From b3fe9fe795d317d1798a1ad2d116ff52131f6612 Mon Sep 17 00:00:00 2001 From: Tino Calancha <tino.calancha@gmail.com> Date: Sat, 20 Aug 2016 15:46:27 +0900 Subject: [PATCH 1/2] Fix Bug#22679 * lisp/ibuf-macs.el (define-ibuffer-op): Added optional args 'before' and 'after'. 'before' is a form to evaluate before the operation. 'after' is a form to evaluate after the operation. * lisp/ibuf-ext.el (ibuffer--after-shell-command-pos): New defvar; store a buffer position where to set the point in the output buffer after operation complete. (ibuffer--before-shell-command): New defun; erase output buffer if 'shell-command-not-erase-buffer' is nil and set 'ibuffer--after-shell-command-pos'. (ibuffer--after-shell-command): New defun; set point in the output buffer after operation complete. (ibuffer-do-shell-command-pipe, ibuffer-do-shell-command-file): Bind 'shell-command-not-erase-buffer' to non-nil while processing the buffers; use 'ibuffer--after-shell-command' to set the point in the output buffer. --- lisp/ibuf-ext.el | 87 +++++++++++++++++++++++++++++++++++++++++++------------ lisp/ibuf-macs.el | 8 ++++- 2 files changed, 76 insertions(+), 19 deletions(-) diff --git a/lisp/ibuf-ext.el b/lisp/ibuf-ext.el index f93957e..0d79617 100644 --- a/lisp/ibuf-ext.el +++ b/lisp/ibuf-ext.el @@ -346,14 +346,60 @@ ibuffer-backward-filter-group (ibuffer-backward-filter-group 1)) (ibuffer-forward-line 0)) +;; The value `beg-last-out' in `shell-command-not-erase-buffer' +;; set the point at the beginning of the output of the first +;; buffer processed. +(defvar ibuffer--after-shell-command-pos) + +(defun ibuffer--before-shell-command () + (let ((obuf (get-buffer-create "*Shell Command Output*")) + (sym shell-command-not-erase-buffer) + final-pos) + (when (buffer-live-p obuf) + (with-current-buffer obuf + (unless sym + (setq buffer-read-only nil) + (let ((inhibit-read-only t)) + (erase-buffer))) + (setq final-pos + (cond ((or (not sym) (eq sym 'beg-last-out)) + (point-max)) + ((eq sym 'save-point) + (point)))) + (setq ibuffer--after-shell-command-pos + final-pos))))) + +(defun ibuffer--after-shell-command () + (let* ((obuf (get-buffer-create "*Shell Command Output*")) + (pos ibuffer--after-shell-command-pos) + (win (car (get-buffer-window-list obuf)))) + (setq ibuffer--after-shell-command-pos nil) + (with-current-buffer obuf + (unless pos (setq pos (point-max))) + (goto-char pos) + ;; Set point in the window displaying obuf, if any; otherwise + ;; display buf temporary in selected frame and set the point. + (if win + (set-window-point win pos) + (save-window-excursion + (let ((win (display-buffer obuf '(nil (inhibit-switch-frame . t))))) + (set-window-point win pos))))))) + ;;;###autoload (autoload 'ibuffer-do-shell-command-pipe "ibuf-ext") (define-ibuffer-op shell-command-pipe (command) "Pipe the contents of each marked buffer to shell command COMMAND." (:interactive "sPipe to shell command: " :opstring "Shell command executed on" - :modifier-p nil) - (shell-command-on-region - (point-min) (point-max) command)) + :modifier-p nil + :opstring "Shell command executed on" + :modifier-p nil + :before (funcall #'ibuffer--before-shell-command) + :after (funcall #'ibuffer--after-shell-command)) + (let ((out-buf (get-buffer "*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,26 +409,31 @@ 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) "Run shell command COMMAND separately on files of marked buffers." (: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)))))) + :opstring "Shell command executed on" + :modifier-p nil + :before (funcall #'ibuffer--before-shell-command) + :after (funcall #'ibuffer--after-shell-command)) + (let ((file (and (not (buffer-modified-p)) + buffer-file-name)) + (out-buf (get-buffer "*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/ibuf-macs.el b/lisp/ibuf-macs.el index 27e7af9..8bb05ec 100644 --- a/lisp/ibuf-macs.el +++ b/lisp/ibuf-macs.el @@ -169,6 +169,8 @@ ibuffer-save-marks dangerous (opstring "operated on") (active-opstring "Operate on") + before + after complex) &rest body) "Generate a function which operates on a buffer. @@ -198,6 +200,8 @@ ibuffer-save-marks ACTIVE-OPSTRING is a string which will be displayed to the user in a confirmation message, in the form: \"Really ACTIVE-OPSTRING x buffers?\" +BEFORE is a form to evaluate before start the operation. +AFTER is a form to evaluate once the operation is complete. COMPLEX means this function is special; if COMPLEX is nil BODY evaluates once for each marked buffer, MBUF, with MBUF current and saving the point. If COMPLEX is non-nil, BODY evaluates @@ -206,7 +210,7 @@ ibuffer-save-marks marked buffer. BODY is evaluated with `buf' bound to the buffer object. -\(fn OP ARGS DOCUMENTATION (&key INTERACTIVE MARK MODIFIER-P DANGEROUS OPSTRING ACTIVE-OPSTRING COMPLEX) &rest BODY)" +\(fn OP ARGS DOCUMENTATION (&key INTERACTIVE MARK MODIFIER-P DANGEROUS OPSTRING ACTIVE-OPSTRING BEFORE AFTER COMPLEX) &rest BODY)" (declare (indent 2) (doc-string 3)) `(progn (defun ,(intern (concat (if (string-match "^ibuffer-do" (symbol-name op)) @@ -233,11 +237,13 @@ ibuffer-save-marks 'ibuffer-deletion-char) (_ 'ibuffer-marked-char)))) + ,before ; pre-operation form. ,(let* ((finish (append '(progn) (if (eq modifier-p t) '((setq ibuffer-did-modification t)) ()) + (and after `(,after)) ; post-operation form. `((ibuffer-redisplay t) (message ,(concat "Operation finished; " opstring " %s buffers") count)))) (inner-body (if complex -- 2.8.1 From 156c63d38a78555fbdd8e04038ad96898a904019 Mon Sep 17 00:00:00 2001 From: Tino Calancha <tino.calancha@gmail.com> Date: Sat, 20 Aug 2016 18:47:04 +0900 Subject: [PATCH 2/2] Add test for Bug#22679 * test/lisp/ibuffer-tests.el (ibuffer-test-bug22679): --- test/lisp/ibuffer-tests.el | 58 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/test/lisp/ibuffer-tests.el b/test/lisp/ibuffer-tests.el index de281c0..5442bd2 100644 --- a/test/lisp/ibuffer-tests.el +++ b/test/lisp/ibuffer-tests.el @@ -30,5 +30,63 @@ (symbol-function 'ibuffer-mark-unsaved-buffers)))) +(ert-deftest ibuffer-test-bug22679 () + "Test for http://debbugs.gnu.org/22679 ." + :expected-result :failed + (let* ((nums (generate-new-buffer "nums")) + (letters (generate-new-buffer "letters")) + (nums-file "/tmp/nums-file") + (letters-file "/tmp/letters-file") + (str-nums "1 2 3 4 5\n6 7 8\n") + (str-letters "a b c d e\nf g h\n") + (str1 "1 2 3\n6 7 8\n") + (str2 "a b c\nf g h\n") + (buffer (get-buffer-create "*Shell Command Output*")) + (command "awk '{print $1 FS $2 FS $3}'") + (check-input-buffers (lambda () + (with-current-buffer nums + (should (string= (buffer-string) str1))) + (with-current-buffer letters + (should (string= (buffer-string) str2))))) + (check-output-buffer (lambda () + (with-current-buffer buffer + (should (or (string= (buffer-string) (concat str1 str2)) + (string= (buffer-string) (concat str2 str1))))))) + ;; Erase output buffer before each test. + (shell-command-not-erase-buffer nil) + ;; Don't ask for confimation to replace buffer content. + (ibuffer-expert t)) + (with-current-buffer nums (insert str-nums)) + (with-current-buffer letters (insert str-letters)) + (with-temp-file nums-file (insert str-nums)) + (with-temp-file letters-file (insert str-letters)) + (unwind-protect + (save-current-buffer + (mapc 'find-file (list nums-file letters-file)) + (ibuffer) + ;; Test ibuffer-do-shell-command-pipe[-replace] + (mapc (lambda (x) + (ibuffer-mark-by-name-regexp (format "\\`%s\\'" x))) + (mapcar 'buffer-name (list nums letters))) + (ibuffer-do-shell-command-pipe command) + (funcall check-output-buffer) + (ibuffer-do-shell-command-pipe-replace command) + (funcall check-input-buffers) + ;; Test ibuffer-do-shell-command-file + (ibuffer-unmark-all-marks) + (mapc (lambda (x) + (ibuffer-mark-by-name-regexp (format "\\`%s\\'" x))) + (mapcar 'buffer-name (list (get-file-buffer nums-file) + (get-file-buffer letters-file)))) + (ibuffer-do-shell-command-file command) + (funcall check-output-buffer)) + ;; Clean up. + (switch-to-buffer (current-buffer)) + (mapc 'kill-buffer (list nums + letters + (get-file-buffer nums-file) + (get-file-buffer letters-file))) + (mapc 'delete-file (list nums-file letters-file))))) + (provide 'ibuffer-tests) ;; ibuffer-tests.el ends here -- 2.8.1 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; In GNU Emacs 25.1.50.14 (x86_64-pc-linux-gnu, GTK+ Version 3.20.7) of 2016-08-20 built on calancha-pc Repository revision: a4ba426d25bd6a5cbe11d81b82a789b8a2c948ed ^ permalink raw reply related [flat|nested] 23+ messages in thread
* bug#22679: 25.0.91; ibuffer-do-shell-command-pipe truncate output 2016-08-20 10:28 ` Tino Calancha @ 2016-08-20 12:46 ` Stefan Monnier 2016-08-21 14:37 ` Tino Calancha 0 siblings, 1 reply; 23+ messages in thread From: Stefan Monnier @ 2016-08-20 12:46 UTC (permalink / raw) To: Tino Calancha; +Cc: 22679 > I have corrected the call to `call-process-region': now it uses > 'shell-file-name' as the executable. 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). 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)? Stefan ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#22679: 25.0.91; ibuffer-do-shell-command-pipe truncate output 2016-08-20 12:46 ` Stefan Monnier @ 2016-08-21 14:37 ` Tino Calancha 2016-08-22 16:06 ` Stefan Monnier 0 siblings, 1 reply; 23+ messages in thread From: Tino Calancha @ 2016-08-21 14:37 UTC (permalink / raw) To: Stefan Monnier; +Cc: Tino Calancha, 22679 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 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* bug#22679: 25.0.91; ibuffer-do-shell-command-pipe truncate output 2016-08-21 14:37 ` Tino Calancha @ 2016-08-22 16:06 ` Stefan Monnier 2016-08-23 15:08 ` Tino Calancha 0 siblings, 1 reply; 23+ messages in thread From: Stefan Monnier @ 2016-08-22 16:06 UTC (permalink / raw) To: Tino Calancha; +Cc: 22679 >> 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. 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. >> 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'. 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 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: I think we should first aim at a simple and clean fix, yes. > - (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))) 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))) -- Stefan ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#22679: 25.0.91; ibuffer-do-shell-command-pipe truncate output 2016-08-22 16:06 ` Stefan Monnier @ 2016-08-23 15:08 ` Tino Calancha 2016-08-24 17:05 ` Stefan Monnier 0 siblings, 1 reply; 23+ messages in thread From: Tino Calancha @ 2016-08-23 15:08 UTC (permalink / raw) To: Stefan Monnier; +Cc: Tino Calancha, 22679 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 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* bug#22679: 25.0.91; ibuffer-do-shell-command-pipe truncate output 2016-08-23 15:08 ` Tino Calancha @ 2016-08-24 17:05 ` Stefan Monnier 2016-08-25 9:39 ` Tino Calancha 2017-01-27 6:26 ` Tino Calancha 0 siblings, 2 replies; 23+ messages in thread From: Stefan Monnier @ 2016-08-24 17:05 UTC (permalink / raw) To: Tino Calancha; +Cc: 22679 > - (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))) Oh, indeed, we already have `call-process-shell-command'. Great. Doesn't look simple enough, tho: you dropped the shell-quote-argument. > +(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)) Indentation looks wrong, but other than that, looks OK. Thanks. Stefan ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#22679: 25.0.91; ibuffer-do-shell-command-pipe truncate output 2016-08-24 17:05 ` Stefan Monnier @ 2016-08-25 9:39 ` Tino Calancha 2016-08-25 12:36 ` Stefan Monnier 2017-01-27 6:26 ` Tino Calancha 1 sibling, 1 reply; 23+ messages in thread From: Tino Calancha @ 2016-08-25 9:39 UTC (permalink / raw) To: Stefan Monnier; +Cc: Tino Calancha, 22679 > Indentation looks wrong, but other than that, looks OK. Thanks. Do you thing we should support PROGRAM being nil? For instance in, * lisp/emacs-lisp/chart.el (chart-space-usage) it first inserts a command in the buffer, and then calls `call-process-region' with 6 arguments. My previous implementation cannot handle that, i.e., fails if PROGRAM is nil. In fact, that task maybe fit better with `call-process'/ `call-process-shell-command': i would just not write the command in the buffer, and pass it as argument to `call-process'. Following patch handle COMMAND being nil. What implementation do you prefer? ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; From c572c4a4e625648016dea192c3234dfc2128ce46 Mon Sep 17 00:00:00 2001 From: Tino Calancha <tino.calancha@gmail.com> Date: Thu, 25 Aug 2016 18:27:42 +0900 Subject: [PATCH] * lisp/subr.el (call-shell-region): Handle COMMAND being nil --- lisp/subr.el | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lisp/subr.el b/lisp/subr.el index 96917b9..7b7f204 100644 --- a/lisp/subr.el +++ b/lisp/subr.el @@ -3097,9 +3097,12 @@ call-shell-region 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)) + (apply #'call-process-region + (append (list start end + shell-file-name delete buffer nil) + (if command + (list shell-command-switch command) + '())))) ;;;; Lisp macros to do various things temporarily. -- 2.9.3 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ^ permalink raw reply related [flat|nested] 23+ messages in thread
* bug#22679: 25.0.91; ibuffer-do-shell-command-pipe truncate output 2016-08-25 9:39 ` Tino Calancha @ 2016-08-25 12:36 ` Stefan Monnier 2016-08-25 13:26 ` Tino Calancha 0 siblings, 1 reply; 23+ messages in thread From: Stefan Monnier @ 2016-08-25 12:36 UTC (permalink / raw) To: Tino Calancha; +Cc: 22679 > Do you thing we should support PROGRAM being nil? No. > * lisp/emacs-lisp/chart.el (chart-space-usage) > it first inserts a command in the buffer, and then calls > `call-process-region' with 6 arguments. Then it can keep using call-process-region if it so prefers. > Following patch handle COMMAND being nil. What implementation do > you prefer? The more straightforward (and restrictive) one. Stefan ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#22679: 25.0.91; ibuffer-do-shell-command-pipe truncate output 2016-08-25 12:36 ` Stefan Monnier @ 2016-08-25 13:26 ` Tino Calancha 0 siblings, 0 replies; 23+ messages in thread From: Tino Calancha @ 2016-08-25 13:26 UTC (permalink / raw) To: Stefan Monnier; +Cc: Tino Calancha, 22679 >> Do you thing we should support PROGRAM being nil? > > No. > >> * lisp/emacs-lisp/chart.el (chart-space-usage) >> it first inserts a command in the buffer, and then calls >> `call-process-region' with 6 arguments. > > Then it can keep using call-process-region if it so prefers. > >> Following patch handle COMMAND being nil. What implementation do >> you prefer? > > The more straightforward (and restrictive) one. Cool. Thanks for your expert advices. I appreciate them a lot. I have pushed to master the one you like. ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#22679: 25.0.91; ibuffer-do-shell-command-pipe truncate output 2016-08-24 17:05 ` Stefan Monnier 2016-08-25 9:39 ` Tino Calancha @ 2017-01-27 6:26 ` Tino Calancha 2017-02-03 4:25 ` Tino Calancha 1 sibling, 1 reply; 23+ messages in thread From: Tino Calancha @ 2017-01-27 6:26 UTC (permalink / raw) To: Stefan Monnier; +Cc: 22679, tino.calancha Stefan Monnier <monnier@iro.umontreal.ca> writes: >> - (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))) > > Doesn't look simple enough, tho: you dropped the shell-quote-argument. OK, i will use it. The following patch is divided in two parts. 1) First one fix this bug: avoid truncation of the output, i.e., the output from all processed buffers is kept. 2) Erase *Shell Command Output* before `ibuffer-do-shell-command-pipe', `ibuffer-do-shell-command-pipe-replace' and `ibuffer-do-shell-command-file' if shell-command-dont-erase-buffer is nil. How do you think? ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; From d8b9c18d1693e30a07b2559aae89459a4fce0a4a Mon Sep 17 00:00:00 2001 From: Tino Calancha <tino.calancha@gmail.com> Date: Fri, 27 Jan 2017 15:22:41 +0900 Subject: [PATCH 1/2] Ibuffer: Don't truncate shell command output * lisp/ibuf-ext.el (ibuffer-do-shell-command-pipe) (ibuffer-do-shell-command-pipe-replace) Use 'call-shell-region' (Bug#22679). (ibuffer-do-shell-command-file): Use call-process-shell-command. If FILE, the file that the buffer object is visiting, exists and the buffer is up-to-date, then use FILE instead of creating a temporary file (Bug#22679). --- lisp/ibuf-ext.el | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/lisp/ibuf-ext.el b/lisp/ibuf-ext.el index 058eaecb36..00cbf051d2 100644 --- a/lisp/ibuf-ext.el +++ b/lisp/ibuf-ext.el @@ -512,8 +512,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) @@ -523,9 +525,8 @@ 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-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) @@ -533,16 +534,23 @@ 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*"))) + (unless (and file (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 + (shell-quote-argument file)) + nil out-buf nil))) + ;;;###autoload (autoload 'ibuffer-do-eval "ibuf-ext") (define-ibuffer-op eval (form) -- 2.11.0 From 905a263a86340ad739b9e8816d36f596ae00b65b Mon Sep 17 00:00:00 2001 From: Tino Calancha <tino.calancha@gmail.com> Date: Fri, 27 Jan 2017 15:22:53 +0900 Subject: [PATCH 2/2] Ibuffer: Erase output buffer before shell commands * lisp/ibuf-macs.el (define-ibuffer-op): Add keyword arguments BEFORE and AFTER; they are forms to run before/after BODY. * lisp/ibuf-ext.el (ibuffer--maybe-erase-shell-cmd-output): New defsubst; if shell-command-dont-erase-buffer is nil, then erase shell command output buffer. (ibuffer-do-shell-command-pipe, ibuffer-do-shell-command-file): Use it. --- lisp/ibuf-ext.el | 9 ++++++++- lisp/ibuf-macs.el | 8 +++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/lisp/ibuf-ext.el b/lisp/ibuf-ext.el index 00cbf051d2..04a3d84f58 100644 --- a/lisp/ibuf-ext.el +++ b/lisp/ibuf-ext.el @@ -506,11 +506,18 @@ ibuffer-backward-filter-group (ibuffer-backward-filter-group 1)) (ibuffer-forward-line 0)) +(defsubst ibuffer--maybe-erase-shell-cmd-output (&optional buffer) + (let ((buf (or buffer (get-buffer "*Shell Command Output*")))) + (when (and (buffer-live-p buf) + (not shell-command-dont-erase-buffer)) + (with-current-buffer buf (erase-buffer))))) + ;;;###autoload (autoload 'ibuffer-do-shell-command-pipe "ibuf-ext") (define-ibuffer-op shell-command-pipe (command) "Pipe the contents of each marked buffer to shell command COMMAND." (:interactive "sPipe to shell command: " :opstring "Shell command executed on" + :before (ibuffer--maybe-erase-shell-cmd-output) :modifier-p nil) (let ((out-buf (get-buffer-create "*Shell Command Output*"))) (with-current-buffer out-buf (goto-char (point-max))) @@ -533,6 +540,7 @@ shell-command-file "Run shell command COMMAND separately on files of marked buffers." (:interactive "sShell command on buffer's file: " :opstring "Shell command executed on" + :before (ibuffer--maybe-erase-shell-cmd-output) :modifier-p nil) (let ((file (and (not (buffer-modified-p)) buffer-file-name)) @@ -551,7 +559,6 @@ shell-command-file (shell-quote-argument file)) nil out-buf nil))) - ;;;###autoload (autoload 'ibuffer-do-eval "ibuf-ext") (define-ibuffer-op eval (form) "Evaluate FORM in each of the buffers. diff --git a/lisp/ibuf-macs.el b/lisp/ibuf-macs.el index 05e568efeb..1ee157aac7 100644 --- a/lisp/ibuf-macs.el +++ b/lisp/ibuf-macs.el @@ -169,6 +169,8 @@ ibuffer-save-marks dangerous (opstring "operated on") (active-opstring "Operate on") + before + after complex) &rest body) "Generate a function which operates on a buffer. @@ -198,6 +200,8 @@ ibuffer-save-marks ACTIVE-OPSTRING is a string which will be displayed to the user in a confirmation message, in the form: \"Really ACTIVE-OPSTRING x buffers?\" +BEFORE is a form to evaluate before start the operation. +AFTER is a form to evaluate once the operation is complete. COMPLEX means this function is special; if COMPLEX is nil BODY evaluates once for each marked buffer, MBUF, with MBUF current and saving the point. If COMPLEX is non-nil, BODY evaluates @@ -206,7 +210,7 @@ ibuffer-save-marks marked buffer. BODY is evaluated with `buf' bound to the buffer object. -\(fn OP ARGS DOCUMENTATION (&key INTERACTIVE MARK MODIFIER-P DANGEROUS OPSTRING ACTIVE-OPSTRING COMPLEX) &rest BODY)" +\(fn OP ARGS DOCUMENTATION (&key INTERACTIVE MARK MODIFIER-P DANGEROUS OPSTRING ACTIVE-OPSTRING BEFORE AFTER COMPLEX) &rest BODY)" (declare (indent 2) (doc-string 3)) `(progn (defun ,(intern (concat (if (string-match "^ibuffer-do" (symbol-name op)) @@ -233,11 +237,13 @@ ibuffer-save-marks 'ibuffer-deletion-char) (_ 'ibuffer-marked-char)))) + ,before ; pre-operation form. ,(let* ((finish (append '(progn) (if (eq modifier-p t) '((setq ibuffer-did-modification t)) ()) + (and after `(,after)) ; post-operation form. `((ibuffer-redisplay t) (message ,(concat "Operation finished; " opstring " %s buffers") count)))) (inner-body (if complex -- 2.11.0 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; In GNU Emacs 26.0.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.22.6) of 2017-01-27 Repository revision: 7cb7a582f44db94292709d35f4f5474f891f03b0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* bug#22679: 25.0.91; ibuffer-do-shell-command-pipe truncate output 2017-01-27 6:26 ` Tino Calancha @ 2017-02-03 4:25 ` Tino Calancha 2017-02-09 9:24 ` Tino Calancha 0 siblings, 1 reply; 23+ messages in thread From: Tino Calancha @ 2017-02-03 4:25 UTC (permalink / raw) To: 22679; +Cc: Stefan Monnier, Tino Calancha Tino Calancha <tino.calancha@gmail.com> writes: >> Doesn't look simple enough, tho: you dropped the shell-quote-argument. > OK, i will use it. > The following patch is divided in two parts. > 1) First one fix this bug: avoid truncation of the output, i.e., the > output from all processed buffers is kept. > > 2) Erase *Shell Command Output* before `ibuffer-do-shell-command-pipe', > `ibuffer-do-shell-command-pipe-replace' and > `ibuffer-do-shell-command-file' > if shell-command-dont-erase-buffer is nil. I have updated 2): BEFORE form must be executed once, right before the operation; previous patch run it inside the loop. Following is the updated patch. I'd like to push it into master branch in a few days if there is no objections. ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; From f01155c426e83fd69ec0b9ecdd697bc973b78197 Mon Sep 17 00:00:00 2001 From: Tino Calancha <tino.calancha@gmail.com> Date: Fri, 3 Feb 2017 12:37:25 +0900 Subject: [PATCH 1/2] Ibuffer: Don't truncate shell command output * lisp/ibuf-ext.el (ibuffer-do-shell-command-pipe) (ibuffer-do-shell-command-pipe-replace) Use 'call-shell-region' (Bug#22679). (ibuffer-do-shell-command-file): Use call-process-shell-command. If FILE, the file that the buffer object is visiting, exists and the buffer is up-to-date, then use FILE instead of creating a temporary file (Bug#22679). --- lisp/ibuf-ext.el | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/lisp/ibuf-ext.el b/lisp/ibuf-ext.el index 058eaecb36..00cbf051d2 100644 --- a/lisp/ibuf-ext.el +++ b/lisp/ibuf-ext.el @@ -512,8 +512,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) @@ -523,9 +525,8 @@ 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-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) @@ -533,16 +534,23 @@ 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*"))) + (unless (and file (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 + (shell-quote-argument file)) + nil out-buf nil))) + ;;;###autoload (autoload 'ibuffer-do-eval "ibuf-ext") (define-ibuffer-op eval (form) -- 2.11.0 From 196c1dc5984c3b0f6842201ef86bc34b71d4a440 Mon Sep 17 00:00:00 2001 From: Tino Calancha <tino.calancha@gmail.com> Date: Fri, 3 Feb 2017 13:10:08 +0900 Subject: [PATCH 2/2] Ibuffer: Erase output buffer before shell commands * lisp/ibuf-macs.el (define-ibuffer-op): Add keyword arguments BEFORE and AFTER; they are forms to run before/after the operation. * lisp/ibuf-ext.el (ibuffer--maybe-erase-shell-cmd-output): New defun; if shell-command-dont-erase-buffer is nil, then erase shell command output buffer. (ibuffer-do-shell-command-pipe, ibuffer-do-shell-command-file): Use it. --- lisp/ibuf-ext.el | 10 +++++++++- lisp/ibuf-macs.el | 10 ++++++++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/lisp/ibuf-ext.el b/lisp/ibuf-ext.el index 00cbf051d2..2a68f777d9 100644 --- a/lisp/ibuf-ext.el +++ b/lisp/ibuf-ext.el @@ -506,11 +506,19 @@ ibuffer-backward-filter-group (ibuffer-backward-filter-group 1)) (ibuffer-forward-line 0)) +(defun ibuffer--maybe-erase-shell-cmd-output () + (let ((buf (get-buffer "*Shell Command Output*"))) + (when (and (buffer-live-p buf) + (not shell-command-dont-erase-buffer) + (not (zerop (buffer-size buf)))) + (with-current-buffer buf (erase-buffer))))) + ;;;###autoload (autoload 'ibuffer-do-shell-command-pipe "ibuf-ext") (define-ibuffer-op shell-command-pipe (command) "Pipe the contents of each marked buffer to shell command COMMAND." (:interactive "sPipe to shell command: " :opstring "Shell command executed on" + :before (ibuffer--maybe-erase-shell-cmd-output) :modifier-p nil) (let ((out-buf (get-buffer-create "*Shell Command Output*"))) (with-current-buffer out-buf (goto-char (point-max))) @@ -533,6 +541,7 @@ shell-command-file "Run shell command COMMAND separately on files of marked buffers." (:interactive "sShell command on buffer's file: " :opstring "Shell command executed on" + :before (ibuffer--maybe-erase-shell-cmd-output) :modifier-p nil) (let ((file (and (not (buffer-modified-p)) buffer-file-name)) @@ -551,7 +560,6 @@ shell-command-file (shell-quote-argument file)) nil out-buf nil))) - ;;;###autoload (autoload 'ibuffer-do-eval "ibuf-ext") (define-ibuffer-op eval (form) "Evaluate FORM in each of the buffers. diff --git a/lisp/ibuf-macs.el b/lisp/ibuf-macs.el index 05e568efeb..8457599899 100644 --- a/lisp/ibuf-macs.el +++ b/lisp/ibuf-macs.el @@ -169,6 +169,8 @@ ibuffer-save-marks dangerous (opstring "operated on") (active-opstring "Operate on") + before + after complex) &rest body) "Generate a function which operates on a buffer. @@ -198,6 +200,8 @@ ibuffer-save-marks ACTIVE-OPSTRING is a string which will be displayed to the user in a confirmation message, in the form: \"Really ACTIVE-OPSTRING x buffers?\" +BEFORE is a form to evaluate before start the operation. +AFTER is a form to evaluate once the operation is complete. COMPLEX means this function is special; if COMPLEX is nil BODY evaluates once for each marked buffer, MBUF, with MBUF current and saving the point. If COMPLEX is non-nil, BODY evaluates @@ -206,7 +210,7 @@ ibuffer-save-marks marked buffer. BODY is evaluated with `buf' bound to the buffer object. -\(fn OP ARGS DOCUMENTATION (&key INTERACTIVE MARK MODIFIER-P DANGEROUS OPSTRING ACTIVE-OPSTRING COMPLEX) &rest BODY)" +\(fn OP ARGS DOCUMENTATION (&key INTERACTIVE MARK MODIFIER-P DANGEROUS OPSTRING ACTIVE-OPSTRING BEFORE AFTER COMPLEX) &rest BODY)" (declare (indent 2) (doc-string 3)) `(progn (defun ,(intern (concat (if (string-match "^ibuffer-do" (symbol-name op)) @@ -238,6 +242,7 @@ ibuffer-save-marks (if (eq modifier-p t) '((setq ibuffer-did-modification t)) ()) + (and after `(,after)) ; post-operation form. `((ibuffer-redisplay t) (message ,(concat "Operation finished; " opstring " %s buffers") count)))) (inner-body (if complex @@ -247,7 +252,8 @@ ibuffer-save-marks (save-excursion ,@body)) t))) - (body `(let ((count + (body `(let ((_before ,before) ; pre-operation form. + (count (,(pcase mark (:deletion 'ibuffer-map-deletion-lines) -- 2.11.0 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; In GNU Emacs 26.0.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.22.6) of 2017-02-02 Repository revision: ce88155d83ba84e84321ed69a39c82f40117dd1f ^ permalink raw reply related [flat|nested] 23+ messages in thread
* bug#22679: 25.0.91; ibuffer-do-shell-command-pipe truncate output 2017-02-03 4:25 ` Tino Calancha @ 2017-02-09 9:24 ` Tino Calancha 0 siblings, 0 replies; 23+ messages in thread From: Tino Calancha @ 2017-02-09 9:24 UTC (permalink / raw) To: 22679-done Tino Calancha <tino.calancha@gmail.com> writes: > Following is the updated patch. I'd like to push it into master > branch in a few days if there is no objections. Pushed fix to master branch as commits: 1e23bf5c513fafb9d14a8e07232101515386a912 d9fd1d32632816aa7833bcfcc116a0a01a53a4b7 ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#22679: 25.0.91; ibuffer-do-shell-command-pipe truncate output 2016-06-10 5:02 ` Glenn Morris [not found] ` <CAMn5WmYDL0CrDppLc_Vs+EM5CkA_wfdb1z+QCmNj_=v6PiYM-g@mail.gmail.com> @ 2016-06-11 3:48 ` C. Calancha 1 sibling, 0 replies; 23+ messages in thread From: C. Calancha @ 2016-06-11 3:48 UTC (permalink / raw) To: 22679; +Cc: Tino Calancha Note: Sorry, i made previous patch over the Emacs master branch: In GNU Emacs 25.1.50 (x86_64-pc-linux-gnu, GTK+ Version 3.20.6) Repository revision: f9af5eddc835bbed2ca100838f8f294901b60c2d ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2017-02-09 9:24 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-15 13:21 bug#22679: 25.0.91; ibuffer-do-shell-command-pipe truncate output Tino Calancha
2016-06-10 5:02 ` Glenn Morris
[not found] ` <CAMn5WmYDL0CrDppLc_Vs+EM5CkA_wfdb1z+QCmNj_=v6PiYM-g@mail.gmail.com>
2016-06-10 9:08 ` Tino Calancha
2016-07-05 15:58 ` Glenn Morris
2016-07-05 16:27 ` Tino Calancha
2016-07-09 17:28 ` Glenn Morris
2016-07-13 15:27 ` Stefan Monnier
2016-08-19 8:33 ` Tino Calancha
2016-08-19 13:52 ` Stefan Monnier
2016-08-20 3:28 ` Tino Calancha
2016-08-20 10:28 ` Tino Calancha
2016-08-20 12:46 ` Stefan Monnier
2016-08-21 14:37 ` Tino Calancha
2016-08-22 16:06 ` Stefan Monnier
2016-08-23 15:08 ` Tino Calancha
2016-08-24 17:05 ` Stefan Monnier
2016-08-25 9:39 ` Tino Calancha
2016-08-25 12:36 ` Stefan Monnier
2016-08-25 13:26 ` Tino Calancha
2017-01-27 6:26 ` Tino Calancha
2017-02-03 4:25 ` Tino Calancha
2017-02-09 9:24 ` Tino Calancha
2016-06-11 3:48 ` C. Calancha
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).