bug#10598: 24.0.92; run dired-do-async-shell-command on multiple files individually

bug#10598: 24.0.92; run dired-do-async-shell-command on multiple files individually

[Roland Winkler]

Within dired, I want to run a shell command asynchronously on
multiple files individually. Say, most specifically I tried to
watch the postscript files foo.ps and bar.ps with gv.

So I mark foo.ps and bar.ps in the dired buffer and hit
& gv ?

In the buffer *Async Shell Command* this gives me the error message

/bin/bash: -c: line 0: syntax error near unexpected token `;'
/bin/bash: -c: line 0: `gv foo.ps &;gv bar.ps'

I get the same error message with the command dired-do-shell-command using
! gv ? &

I am not the most experienced bash expert. But it seems that bash
doesn't like the combination `&;'. Note also that there is no `&'
for `gv bar.ps'.

In a tty
$ gv foo.ps & gv bar.ps &
works fine for me (giving me what I expect to get within dired).

Am I misunderstanding the docstrings of dired-do-shell-command and
dired-do-async-shell-command (which are a bit confusing for me
concerning how to run a shell command on multiple files) or is there
a bug in the code?


In GNU Emacs 24.0.92.1 (x86_64-unknown-linux-gnu, GTK+ Version 2.20.1)
 of 2011-12-14 on regnitz
Windowing system distributor `The X.Org Foundation', version 11.0.10706000

bug#10598: 24.0.92; run dired-do-async-shell-command on multiple files individually

[Juri Linkov]

> Am I misunderstanding the docstrings of dired-do-shell-command and
> dired-do-async-shell-command (which are a bit confusing for me
> concerning how to run a shell command on multiple files) or is there
> a bug in the code?

I think `dired-shell-stuff-it' should be improved to translate
`gv foo.ps&;gv bar.ps&' into `gv foo.ps&gv bar.ps&' or into
`gv foo.ps;gv bar.ps&' depending on whether you want to run
asynchronously both at the same time or sequentially
(perhaps a new option should define that).

bug#10598: 24.0.92; run dired-do-async-shell-command on multiple files individually

[Roland Winkler]

On Wed Jan 25 2012 Juri Linkov wrote:
> I think `dired-shell-stuff-it' should be improved to translate
> `gv foo.ps&;gv bar.ps&' into `gv foo.ps&gv bar.ps&' or into
> `gv foo.ps;gv bar.ps&' depending on whether you want to run
> asynchronously both at the same time or sequentially
> (perhaps a new option should define that).

Good point! In my case I wanted to run the commands asynchronously
at the same time. Others might want to run the command
asynchronously, but sequentially.

Roland

bug#10598: 24.0.92; run dired-do-async-shell-command on multiple files individually

[Juri Linkov]

>> I think `dired-shell-stuff-it' should be improved to translate
>> `gv foo.ps&;gv bar.ps&' into `gv foo.ps&gv bar.ps&' or into
>> `gv foo.ps;gv bar.ps&' depending on whether you want to run
>> asynchronously both at the same time or sequentially
>> (perhaps a new option should define that).
>
> Good point! In my case I wanted to run the commands asynchronously
> at the same time. Others might want to run the command
> asynchronously, but sequentially.

This can be fixed with the following patch:

=== modified file 'lisp/dired-aux.el'
--- lisp/dired-aux.el	2012-04-17 01:52:00 +0000
+++ lisp/dired-aux.el	2012-07-12 08:51:48 +0000
@@ -542,12 +542,25 @@ (defun dired-read-shell-command (prompt
       (dired-mark-pop-up nil 'shell files
 			 'read-shell-command prompt nil nil))))
 
+(defvar dired-async-each t
+  "How to run `dired-do-async-shell-command' on multiple files individually.
+When non-nil, run asynchronous commands on each file parallelly.
+In shell syntax this means separating the individual commands with `&'.
+When nil, run commands in the background on each file sequentially
+waiting for each command to terminate before running the next command.
+In shell syntax this means separating the individual commands with `;'.")
+
 ;;;###autoload
 (defun dired-do-async-shell-command (command &optional arg file-list)
   "Run a shell command COMMAND on the marked files asynchronously.
 
-Like `dired-do-shell-command' but if COMMAND doesn't end in ampersand,
-adds `* &' surrounded by whitespace and executes the command asynchronously.
+Like `dired-do-shell-command' but adds `&' at the end of COMMAND
+to execute it asynchronously.
+
+When operating on multiple files, the variable `dired-async-each'
+defines whether to run asynchronous commands on each file parallelly
+or sequentially.
+
 The output appears in the buffer `*Async Shell Command*'."
   (interactive
    (let ((files (dired-get-marked-files t current-prefix-arg)))
@@ -556,14 +569,10 @@ (defun dired-do-async-shell-command (com
       (dired-read-shell-command "& on %s: " current-prefix-arg files)
       current-prefix-arg
       files)))
-  (unless (string-match "[*?][ \t]*\\'" command)
-    (setq command (concat command " *")))
   (unless (string-match "&[ \t]*\\'" command)
     (setq command (concat command " &")))
   (dired-do-shell-command command arg file-list))
 
-;; The in-background argument is only needed in Emacs 18 where
-;; shell-command doesn't understand an appended ampersand `&'.
 ;;;###autoload
 (defun dired-do-shell-command (command &optional arg file-list)
   "Run a shell command COMMAND on the marked files.
@@ -655,7 +661,11 @@ (defun dired-shell-stuff-it (command fil
 ;; Might be redefined for smarter things and could then use RAW-ARG
 ;; (coming from interactive P and currently ignored) to decide what to do.
 ;; Smart would be a way to access basename or extension of file names.
-  (let ((stuff-it
+  (let* ((in-background (string-match "[ \t]*&[ \t]*\\'" command))
+	 (command (if in-background
+		      (substring command 0 (match-beginning 0))
+		    command))
+	 (stuff-it
 	 (if (or (string-match dired-star-subst-regexp command)
 		 (string-match dired-quark-subst-regexp command))
 	     (lambda (x)
@@ -665,13 +675,16 @@ (defun dired-shell-stuff-it (command fil
 		   (setq retval (replace-match x t t retval 2)))
 		 retval))
 	   (lambda (x) (concat command dired-mark-separator x)))))
+    (concat
     (if on-each
-	(mapconcat stuff-it (mapcar 'shell-quote-argument file-list) ";")
+	 (mapconcat stuff-it (mapcar 'shell-quote-argument file-list)
+		    (if (and in-background dired-async-each) "&" ";"))
       (let ((files (mapconcat 'shell-quote-argument
 			      file-list dired-mark-separator)))
 	(if (> (length file-list) 1)
 	    (setq files (concat dired-mark-prefix files dired-mark-postfix)))
-	(funcall stuff-it files)))))
+	 (funcall stuff-it files)))
+     (if in-background "&" ""))))
 
 ;; This is an extra function so that it can be redefined by ange-ftp.
 ;;;###autoload

bug#10598: 24.0.92; run dired-do-async-shell-command on multiple files individually

[Roland Winkler]

On Thu Jul 12 2012 Juri Linkov wrote:
> This can be fixed with the following patch:

Thanks for looking into this.

> +(defvar dired-async-each t

Should this possibly be defcustom? I'd consider this a user option.

Roland

bug#10598: 24.0.92; run dired-do-async-shell-command on multiple files individually

[Juri Linkov]

>> +(defvar dired-async-each t
>
> Should this possibly be defcustom? I'd consider this a user option.

I can't imagine anyone wanting to customize it on a permanent basis
because the desirable behavior depends on the type of executed command:
non-interactive programs are better to run sequentially,
but programs with GUI are better to run in parallel.

Ideally this preference should be expressed by syntax
on the command line like using special symbols `*' and `?',
but at the moment I have no idea for such a syntax.

At least with a variable the users can do something like

(define-key dired-mode-map ";"
  (lambda ()
    (interactive)
    (let ((dired-async-each nil))
      (call-interactively 'dired-do-async-shell-command))))

bug#10598: 24.0.92; run dired-do-async-shell-command on multiple files individually

[Roland Winkler]

On Fri Jul 13 2012 Juri Linkov wrote:
> At least with a variable the users can do something like
> 
> (define-key dired-mode-map ";"
>   (lambda ()
>     (interactive)
>     (let ((dired-async-each nil))
>       (call-interactively 'dired-do-async-shell-command))))

Would it make sense to provide such a command for everyone? If
dired-async-each is not even intended to be customizable, I cannot
think of too many other options to use this variable in a meaningful
way. Anyway, in real life I expect that according to Murphy's Law
whatever customization a user chooses for this variable, next time
he or she needs to run dired-do-async-shell-command, the customized
value will be the wrong one.

bug#10598: 24.0.92; run dired-do-async-shell-command on multiple files individually

[Juri Linkov]

> Would it make sense to provide such a command for everyone? If
> dired-async-each is not even intended to be customizable, I cannot
> think of too many other options to use this variable in a meaningful
> way. Anyway, in real life I expect that according to Murphy's Law
> whatever customization a user chooses for this variable, next time
> he or she needs to run dired-do-async-shell-command, the customized
> value will be the wrong one.

A good UI to select the desired behavior before the command invocation
is to use a prefix argument.  A comment in `dired-shell-stuff-it' says:

  ;; Might be redefined for smarter things and could then use RAW-ARG
  ;; (coming from interactive P and currently ignored) to decide what to do.
  ;; Smart would be a way to access basename or extension of file names.

I don't know how it was intended to be implemented a smart way, but
maybe the same could be used to specify how to run dired async commands?

bug#10598: 24.0.92; run dired-do-async-shell-command on multiple files individually

[Roland Winkler]

On Sat Jul 14 2012 Juri Linkov wrote:
> A good UI to select the desired behavior before the command invocation
> is to use a prefix argument.  A comment in `dired-shell-stuff-it'
> says:

...much agreed -- if these had not already established their own
usage of the prefix arg. The command dired-do-async-shell-command
uses the prefix arg like dired-do-shell-command, that is, the prefix
arg is one method (besides the mark) to determine the file list the
shell commands will work on. Changing this established behavior for
dired-do-async-shell-command (while possibly keeping the current
behavior for dired-do-shell-command) is probably confusing.

>   ;; Might be redefined for smarter things and could then use RAW-ARG
>   ;; (coming from interactive P and currently ignored) to decide what to do.
>   ;; Smart would be a way to access basename or extension of file names.
> 
> I don't know how it was intended to be implemented a smart way, but
> maybe the same could be used to specify how to run dired async commands?

I guess that this can only refer to the handling of file arguments,
but the prefix cannot be used (anymore) for a completely different
purpose.

Currently the COMMAND arg knows three special characters, *. ?, and
&. How about a fourth character ";"? More specifically:

 & means: run asynchronously, separating with &
 ; means: run asynchronously, separating with ;

I believe this should not break too much backward compatibility
because ; normally needs to be protected anyway.

Then & becomes the default of dired-do-async-shell-command. Anyway,
& is appended only if it is not yet present.

Or am I missing something? Would this break something?

bug#10598: 24.0.92; run dired-do-async-shell-command on multiple files individually

[Juri Linkov]

> Currently the COMMAND arg knows three special characters, *. ?, and
> &. How about a fourth character ";"? More specifically:
>
>  & means: run asynchronously, separating with &
>  ; means: run asynchronously, separating with ;
>
> I believe this should not break too much backward compatibility
> because ; normally needs to be protected anyway.
>
> Then & becomes the default of dired-do-async-shell-command. Anyway,
> & is appended only if it is not yet present.
>
> Or am I missing something? Would this break something?

Yes, it seems this is what remains to do.
It's implemented in the following patch:

=== modified file 'lisp/dired-aux.el'
--- lisp/dired-aux.el	2012-04-17 01:52:00 +0000
+++ lisp/dired-aux.el	2012-07-15 08:27:04 +0000
@@ -546,8 +546,16 @@ (defun dired-read-shell-command (prompt
 (defun dired-do-async-shell-command (command &optional arg file-list)
   "Run a shell command COMMAND on the marked files asynchronously.
 
-Like `dired-do-shell-command' but if COMMAND doesn't end in ampersand,
-adds `* &' surrounded by whitespace and executes the command asynchronously.
+Like `dired-do-shell-command', but adds `&' at the end of COMMAND
+to execute it asynchronously.
+
+When operating on multiple files, asynchronous commands are executed
+on each file in parallel.  In shell syntax this means separating the
+individual commands with `&'.  However, when COMMAND ends in `;' or `;&'
+then commands are executed in the background on each file sequentially
+waiting for each command to terminate before running the next command.
+In shell syntax this means separating the individual commands with `;'.
+
 The output appears in the buffer `*Async Shell Command*'."
   (interactive
    (let ((files (dired-get-marked-files t current-prefix-arg)))
@@ -556,14 +564,10 @@ (defun dired-do-async-shell-command (com
       (dired-read-shell-command "& on %s: " current-prefix-arg files)
       current-prefix-arg
       files)))
-  (unless (string-match "[*?][ \t]*\\'" command)
-    (setq command (concat command " *")))
   (unless (string-match "&[ \t]*\\'" command)
     (setq command (concat command " &")))
   (dired-do-shell-command command arg file-list))
 
-;; The in-background argument is only needed in Emacs 18 where
-;; shell-command doesn't understand an appended ampersand `&'.
 ;;;###autoload
 (defun dired-do-shell-command (command &optional arg file-list)
   "Run a shell command COMMAND on the marked files.
@@ -655,7 +656,15 @@ (defun dired-shell-stuff-it (command fil
 ;; Might be redefined for smarter things and could then use RAW-ARG
 ;; (coming from interactive P and currently ignored) to decide what to do.
 ;; Smart would be a way to access basename or extension of file names.
-  (let ((stuff-it
+  (let* ((in-background (string-match "[ \t]*&[ \t]*\\'" command))
+	 (command (if in-background
+		      (substring command 0 (match-beginning 0))
+		    command))
+	 (sequentially (string-match "[ \t]*;[ \t]*\\'" command))
+	 (command (if sequentially
+		      (substring command 0 (match-beginning 0))
+		    command))
+	 (stuff-it
 	 (if (or (string-match dired-star-subst-regexp command)
 		 (string-match dired-quark-subst-regexp command))
 	     (lambda (x)
@@ -665,13 +674,16 @@ (defun dired-shell-stuff-it (command fil
 		   (setq retval (replace-match x t t retval 2)))
 		 retval))
 	   (lambda (x) (concat command dired-mark-separator x)))))
+    (concat
     (if on-each
-	(mapconcat stuff-it (mapcar 'shell-quote-argument file-list) ";")
+	 (mapconcat stuff-it (mapcar 'shell-quote-argument file-list)
+		    (if (and in-background (not sequentially)) "&" ";"))
       (let ((files (mapconcat 'shell-quote-argument
 			      file-list dired-mark-separator)))
 	(if (> (length file-list) 1)
 	    (setq files (concat dired-mark-prefix files dired-mark-postfix)))
-	(funcall stuff-it files)))))
+	 (funcall stuff-it files)))
+     (if in-background "&" ""))))
 
 ;; This is an extra function so that it can be redefined by ange-ftp.
 ;;;###autoload

bug#10598: 24.0.92; run dired-do-async-shell-command on multiple files individually

[Roland Winkler]

On Sun Jul 15 2012 Juri Linkov wrote:
> Yes, it seems this is what remains to do.
> It's implemented in the following patch:

Thanks a lot, this looks great to me (and also works for me as expected).

In the slightly modified version of the patch below, I updated also
the docstring for dired-do-shell-command and I unified the indentation.

It would be good if the info node "(emacs)Shell Commands in Dired"
will be updated, too.


--- dired-aux.el~	2012-07-15 01:08:08.000000000 -0500
+++ dired-aux.el	2012-07-17 02:03:33.000000000 -0500
@@ -539,8 +539,17 @@
 (defun dired-do-async-shell-command (command &optional arg file-list)
   "Run a shell command COMMAND on the marked files asynchronously.
 
-Like `dired-do-shell-command' but if COMMAND doesn't end in ampersand,
-adds `* &' surrounded by whitespace and executes the command asynchronously.
+Like `dired-do-shell-command', but adds `&' at the end of COMMAND
+to execute it asynchronously.
+
+When operating on multiple files, asynchronous commands are executed
+in the background on each file in parallel.  In shell syntax this means
+separating the individual commands with `&'.  However, when COMMAND
+ends in `;' or `;&' then commands are executed in the background on each file
+sequentially waiting for each command to terminate before running
+the next command.  In shell syntax this means separating the individual
+commands with `;'.
+
 The output appears in the buffer `*Async Shell Command*'."
   (interactive
    (let ((files (dired-get-marked-files t current-prefix-arg)))
@@ -549,18 +558,14 @@
       (dired-read-shell-command "& on %s: " current-prefix-arg files)
       current-prefix-arg
       files)))
-  (unless (string-match "[*?][ \t]*\\'" command)
-    (setq command (concat command " *")))
   (unless (string-match "&[ \t]*\\'" command)
     (setq command (concat command " &")))
   (dired-do-shell-command command arg file-list))
 
-;; The in-background argument is only needed in Emacs 18 where
-;; shell-command doesn't understand an appended ampersand `&'.
 ;;;###autoload
 (defun dired-do-shell-command (command &optional arg file-list)
   "Run a shell command COMMAND on the marked files.
-If no files are marked or a specific numeric prefix arg is given,
+If no files are marked or a numeric prefix arg is given,
 the next ARG files are used.  Just \\[universal-argument] means the current file.
 The prompt mentions the file(s) or the marker, as appropriate.
 
@@ -582,6 +587,15 @@
 it, write `*\"\"' in place of just `*'.  This is equivalent to just
 `*' in the shell, but avoids Dired's special handling.
 
+If COMMAND ends in `&', `;', or `;&', execute the shell command
+in the background asynchronously.  When operating on multiple files
+and COMMAND ends in `&', the shell command is executed on each file
+in parallel.  In shell syntax this means separating the individual commands
+with `&'.  However, when COMMAND ends in `;' or `;&' then commands
+are executed in the background on each file sequentially
+waiting for each command to terminate before running the next command.
+In shell syntax this means separating the individual commands with `;'.
+
 If COMMAND produces output, it goes to a separate buffer.
 
 This feature does not try to redisplay Dired buffers afterward, as
@@ -592,9 +606,9 @@
 of the Dired buffer, so output files usually are created there
 instead of in a subdir.
 
-In a noninteractive call (from Lisp code), you must specify
-the list of file names explicitly with the FILE-LIST argument, which
-can be produced by `dired-get-marked-files', for example."
+In a noninteractive call, you must specify the list of file names explicitly
+with the FILE-LIST argument, which can be produced by `dired-get-marked-files',
+for example."
 ;;Functions dired-run-shell-command and dired-shell-stuff-it do the
 ;;actual work and can be redefined for customization.
   (interactive
@@ -648,23 +662,34 @@
 ;; Might be redefined for smarter things and could then use RAW-ARG
 ;; (coming from interactive P and currently ignored) to decide what to do.
 ;; Smart would be a way to access basename or extension of file names.
-  (let ((stuff-it
-	 (if (or (string-match dired-star-subst-regexp command)
-		 (string-match dired-quark-subst-regexp command))
-	     (lambda (x)
-	       (let ((retval command))
-		 (while (string-match
-			 "\\(^\\|[ \t]\\)\\([*?]\\)\\([ \t]\\|$\\)" retval)
-		   (setq retval (replace-match x t t retval 2)))
-		 retval))
-	   (lambda (x) (concat command dired-mark-separator x)))))
-    (if on-each
-	(mapconcat stuff-it (mapcar 'shell-quote-argument file-list) ";")
-      (let ((files (mapconcat 'shell-quote-argument
-			      file-list dired-mark-separator)))
-	(if (> (length file-list) 1)
-	    (setq files (concat dired-mark-prefix files dired-mark-postfix)))
-	(funcall stuff-it files)))))
+  (let* ((in-background (string-match "[ \t]*&[ \t]*\\'" command))
+	 (command (if in-background
+		      (substring command 0 (match-beginning 0))
+		    command))
+	 (sequentially (string-match "[ \t]*;[ \t]*\\'" command))
+	 (command (if sequentially
+		      (substring command 0 (match-beginning 0))
+		    command))
+	 (stuff-it
+          (if (or (string-match dired-star-subst-regexp command)
+                  (string-match dired-quark-subst-regexp command))
+              (lambda (x)
+                (let ((retval command))
+                  (while (string-match
+                          "\\(^\\|[ \t]\\)\\([*?]\\)\\([ \t]\\|$\\)" retval)
+                    (setq retval (replace-match x t t retval 2)))
+                  retval))
+            (lambda (x) (concat command dired-mark-separator x)))))
+    (concat
+     (if on-each
+	 (mapconcat stuff-it (mapcar 'shell-quote-argument file-list)
+		    (if (and in-background (not sequentially)) "&" ";"))
+       (let ((files (mapconcat 'shell-quote-argument
+                               file-list dired-mark-separator)))
+         (if (> (length file-list) 1)
+             (setq files (concat dired-mark-prefix files dired-mark-postfix)))
+	 (funcall stuff-it files)))
+     (if in-background "&" ""))))
 
 ;; This is an extra function so that it can be redefined by ange-ftp.
 ;;;###autoload

bug#10598: 24.0.92; run dired-do-async-shell-command on multiple files individually

[Juri Linkov]

> Thanks a lot, this looks great to me (and also works for me as expected).

Thanks, installed and closed.

> It would be good if the info node "(emacs)Shell Commands in Dired"
> will be updated, too.

A NEWS entry for this change has no "---" indication that means
it should be documented in the manual before the next release when
the functionality is stabilized and there are no more code changes.

bug#10598: 24.0.92; run dired-do-async-shell-command on multiple files individually

[Jérémy Compostella]

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

Hi,

This bug was really annoying me tonight so I wrote the attached patch
which fix it by translating `gv foo.ps&;gv bar.ps&' to `gv foo.ps&gv
bar.ps&'.

I do agree it should be better to provide a way to choice between the
sequential or parallel solution but I have no idea how to implement it
in a clean fashion for now.

Anyway, the current behavior looks buggy and I propose to merge the
attached patch in a first time in order to have a acceptable behavior
that is far better than an error.

Best regards,

Jérémy


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Fix dired-do-async-shell-command on multiple files individually (bug#10598) patch --]
[-- Type: text/x-diff, Size: 1186 bytes --]

From aefc85c364f6e51676ffc9dc44deb9a4d43a128c Mon Sep 17 00:00:00 2001
From: Jeremy Compostella <jeremy.compostella@gmail.com>
Date: Wed, 25 Jan 2012 23:38:54 +0100
Subject: [PATCH] Fix dired-do-async-shell-command on multiple files individually (bug#10598)

The `dired-shell-stuff-it' should not insert the ";" shell command
separator when called with a command argument ending with the "&"
character.

Signed-off-by: Jeremy Compostella <jeremy.compostella@gmail.com>
---
 lisp/dired-aux.el |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index 2d05be3..74f0de9 100644
--- a/lisp/dired-aux.el
+++ b/lisp/dired-aux.el
@@ -659,7 +659,8 @@ can be produced by `dired-get-marked-files', for example."
 		 retval))
 	   (lambda (x) (concat command dired-mark-separator x)))))
     (if on-each
-	(mapconcat stuff-it (mapcar 'shell-quote-argument file-list) ";")
+	(mapconcat stuff-it (mapcar 'shell-quote-argument file-list)
+		   (if (string-match "[ \t]*&[ \t]*\\'" command) "" ";"))
       (let ((files (mapconcat 'shell-quote-argument
 			      file-list dired-mark-separator)))
 	(if (> (length file-list) 1)
-- 
1.7.2.5