bug#9907: 24.0.90; eshell:for command destructivly modifies list variables

[Stefan Monnier]

>>> Sorry 'bout that.  Should be fixed now, but please confirm.
>> It is working fine now, thanks.
> It was pointed out on emacs-devel that eshell pipes no longer work
> (it's because I just closed this bug...):

> emacs -Q -f eshell
> ls | grep foo

> -> Wrong type argument: symbolp, (quote nil)
>    and a stale grep process in M-x list-processes

I just installed the patch below which seems to fix this problem.


        Stefan


=== modified file 'lisp/ChangeLog'
--- lisp/ChangeLog      2011-11-18 14:34:39 +0000
+++ lisp/ChangeLog      2011-11-18 14:48:12 +0000
@@ -1,5 +1,10 @@
 2011-11-18  Stefan Monnier  <monnier@iro.umontreal.ca>
 
+       * eshell/esh-cmd.el (eshell-do-eval): Handle `setq' (bug#9907).
+       (eshell-rewrite-for-command): Remove workaround.
+       (eshell-do-pipelines, eshell-do-pipelines-synchronously)
+       (eshell-do-eval, eshell-exec-lisp): Avoid gratuitous setq.
+
        * files-x.el (modify-file-local-variable): Obey commenting conventions.
 
 2011-11-17  Glenn Morris  <rgm@gnu.org>

=== modified file 'lisp/eshell/esh-cmd.el'
--- lisp/eshell/esh-cmd.el      2011-11-14 23:59:56 +0000
+++ lisp/eshell/esh-cmd.el      2011-11-18 14:41:23 +0000
@@ -480,25 +480,20 @@
       (let ((body (car (last terms))))
        (setcdr (last terms 2) nil)
        `(let ((for-items
-                ;; Apparently, eshell-do-eval only works for immutable
-                ;; let-bindings, i.e. we cannot use `setq' on `for-items'.
-                ;; Instead we store the list in the car of a cons-cell (which
-                ;; acts as a ref-cell) so we can setcar instead of setq.
-                (list
                  (append
                   ,@(mapcar
                      (lambda (elem)
                        (if (listp elem)
                            elem
                          `(list ,elem)))
-                     (cdr (cddr terms))))))
+                    (cdr (cddr terms)))))
                (eshell-command-body '(nil))
                (eshell-test-body '(nil)))
-           (while (consp (car for-items))
-             (let ((,(intern (cadr terms)) (caar for-items)))
+           (while (consp for-items)
+             (let ((,(intern (cadr terms)) (car for-items)))
                (eshell-protect
                 ,(eshell-invokify-arg body t)))
-             (setcar for-items (cdar for-items)))
+             (setq for-items (cdr for-items)))
            (eshell-close-handles
             eshell-last-command-status
             (list 'quote eshell-last-command-result))))))
@@ -766,9 +761,8 @@
     `(eshell-copy-handles
       (progn
        ,(when (cdr pipeline)
-          `(let (nextproc)
-              (setq nextproc
-                    (eshell-do-pipelines (quote ,(cdr pipeline)) t))
+          `(let ((nextproc
+                  (eshell-do-pipelines (quote ,(cdr pipeline)) t)))
               (eshell-set-output-handle ,eshell-output-handle
                                         'append nextproc)
               (eshell-set-output-handle ,eshell-error-handle
@@ -796,10 +790,9 @@
 Output of each command is passed as input to the next one in the pipeline.
 This is used on systems where `start-process' is not supported."
   (when (setq pipeline (cadr pipeline))
-    `(let (result)
+    `(progn
        ,(when (cdr pipeline)
-          `(let (output-marker)
-             (setq output-marker ,(point-marker))
+          `(let ((output-marker ,(point-marker)))
              (eshell-set-output-handle ,eshell-output-handle
                                        'append output-marker)
              (eshell-set-output-handle ,eshell-error-handle
@@ -819,13 +812,13 @@
             `(progn
                (setq eshell-current-handles tail-handles)
                (setq eshell-in-pipeline-p nil)))
-       (setq result ,(car pipeline))
+       (let ((result ,(car pipeline)))
        ;; tailproc gets the result of the last successful process in
        ;; the pipeline.
        (setq tailproc (or result tailproc))
        ,(if (cdr pipeline)
             `(eshell-do-pipelines-synchronously (quote ,(cdr pipeline))))
-       result)))
+         result))))
 
 (defalias 'eshell-process-identity 'identity)
 
@@ -890,8 +883,7 @@
         (eshell-print "errors\n"))
      (if eshell-debug-command
         (eshell-print "commands\n")))
-    ((or (string= (car args) "-h")
-        (string= (car args) "--help"))
+    ((member (car args) '("-h" "--help"))
      (eshell-print "usage: eshell-debug [kinds]
 
 This command is used to aid in debugging problems related to Eshell
@@ -1091,6 +1083,11 @@
          (eshell-manipulate "handling special form"
            (setcar args `(eshell-do-eval ',(car args) ,synchronous-p))))
        (eval form))
+       ((eq (car form) 'setq)
+       (if (cddr args) (error "Unsupported form (setq X1 E1 X2 E2..)"))
+        (eshell-manipulate "evaluating arguments to setq"
+          (setcar (cdr args) (eshell-do-eval (cadr args) synchronous-p)))
+       (list 'quote (eval form)))
        (t
        (if (and args (not (memq (car form) '(run-hooks))))
            (eshell-manipulate
@@ -1127,11 +1124,12 @@
          ;; Thus, aliases can even contain references to asynchronous
          ;; sub-commands, and things will still work out as they
          ;; should.
-         (let (result new-form)
-           (if (setq new-form
+         (let* (result
+                 (new-form
                      (catch 'eshell-replace-command
                        (ignore
-                        (setq result (eval form)))))
+                     (setq result (eval form))))))
+           (if new-form
                (progn
                  (eshell-manipulate "substituting replacement form"
                    (setcar form (car new-form))
@@ -1247,14 +1245,12 @@
 PRINTER and ERRPRINT are functions to use for printing regular
 messages, and errors.  FORM-P should be non-nil if FUNC-OR-FORM
 represent a lisp form; ARGS will be ignored in that case."
-  (let (result)
     (eshell-condition-case err
-       (progn
-         (setq result
+      (let ((result
                (save-current-buffer
                  (if form-p
                      (eval func-or-form)
-                   (apply func-or-form args))))
+                 (apply func-or-form args)))))
          (and result (funcall printer result))
          result)
       (error
@@ -1265,7 +1261,7 @@
             (let ((func-doc (eldoc-get-fnsym-args-string func-or-form)))
               (setq msg (format "usage: %s" func-doc))))
         (funcall errprint msg))
-       nil))))
+     nil)))
 
 (defsubst eshell-apply* (printer errprint func args)
   "Call FUNC, with ARGS, trapping errors and return them as output.

=== modified file 'lisp/eshell/esh-util.el'
--- lisp/eshell/esh-util.el     2011-03-10 07:16:04 +0000
+++ lisp/eshell/esh-util.el     2011-11-18 14:14:57 +0000
@@ -140,14 +140,13 @@
 (defmacro eshell-condition-case (tag form &rest handlers)
   "If `eshell-handle-errors' is non-nil, this is `condition-case'.
 Otherwise, evaluates FORM with no error handling."
+  (declare (indent 2))
   (if eshell-handle-errors
       `(condition-case ,tag
           ,form
         ,@handlers)
     form))
 
-(put 'eshell-condition-case 'lisp-indent-function 2)
-
 (defun eshell-find-delimiter
   (open close &optional bound reverse-p backslash-p)
   "From point, find the CLOSE delimiter corresponding to OPEN.
@@ -275,14 +274,14 @@
     text))
 
 (defmacro eshell-for (for-var for-list &rest forms)
-  "Iterate through a list"
+  "Iterate through a list."
+  (declare (indent 2))
   `(let ((list-iter ,for-list))
      (while list-iter
        (let ((,for-var (car list-iter)))
         ,@forms)
        (setq list-iter (cdr list-iter)))))
 
-(put 'eshell-for 'lisp-indent-function 2)
 
 (make-obsolete 'eshell-for 'dolist "24.1")