Cleanup Eshell to rely less on dynamic scoping.

* lisp/eshell/esh-opt.el (eshell-eval-using-options): Don't bind usage-msg,
last-value, and ext-command here.  Bind `args' closer to `body'.
(temp-args, last-value, usage-msg, ext-command, args): Don't defvar.
(eshell--args): Declare new dynamic var.
(eshell-do-opt): Add argument `args'.  Bind our own usage-msg,
last-value, and ext-command.  Pass `args' to `body'.
(eshell-process-args): Bind eshell--args.
(eshell-set-option): Use eshell--args.
* lisp/eshell/eshell.el (eshell): Use derived-mode-p.
* lisp/eshell/esh-var.el (eshell-parse-variable): Use backquote.
(eshell-parse-variable-ref): Remove unused vars `end' and `err'.
(eshell-glob-function): Declare.
* lisp/eshell/esh-util.el: Require cl-lib.
(eshell-read-hosts-file): Avoid add-to-list.
* lisp/eshell/esh-cmd.el (eshell-parse-lisp-argument): Remove unused var
`err'.
* lisp/eshell/em-unix.el (compilation-scroll-output, locate-history-list):
Declare.
(eshell/diff): Remove unused var `err'.
* lisp/eshell/em-rebind.el (eshell-delete-backward-char): Remove unused arg
`killflag'.
* lisp/eshell/em-pred.el (eshell-parse-modifiers): Remove unused var `err'.
* lisp/eshell/em-ls.el (eshell-ls-highlight-alist): Move defvars before
first use.
* lisp/eshell/em-glob.el (eshell-glob-matches, message-shown):
Move declaration before first use.
* lisp/eshell/em-alias.el (eshell-maybe-replace-by-alias): Use backquotes.
* autorevert.el (auto-revert-notify-handler): Use `cl-dolist' since we
rely on cl-return.

13 files changed, 161 insertions, 140 deletions

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index 61e9c9bf708..7984dc214c4 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,5 +1,33 @@
 2013-09-12  Stefan Monnier  <monnier@iro.umontreal.ca>
 
+        Cleanup Eshell to rely less on dynamic scoping.
+        * eshell/esh-opt.el (eshell-eval-using-options): Don't bind usage-msg,
+        last-value, and ext-command here.  Bind `args' closer to `body'.
+        (temp-args, last-value, usage-msg, ext-command, args): Don't defvar.
+        (eshell--args): Declare new dynamic var.
+        (eshell-do-opt): Add argument `args'.  Bind our own usage-msg,
+        last-value, and ext-command.  Pass `args' to `body'.
+        (eshell-process-args): Bind eshell--args.
+        (eshell-set-option): Use eshell--args.
+        * eshell/eshell.el (eshell): Use derived-mode-p.
+        * eshell/esh-var.el (eshell-parse-variable): Use backquote.
+        (eshell-parse-variable-ref): Remove unused vars `end' and `err'.
+        (eshell-glob-function): Declare.
+        * eshell/esh-util.el: Require cl-lib.
+        (eshell-read-hosts-file): Avoid add-to-list.
+        * eshell/esh-cmd.el (eshell-parse-lisp-argument): Remove unused var
+        `err'.
+        * eshell/em-unix.el (compilation-scroll-output, locate-history-list):
+        Declare.
+        (eshell/diff): Remove unused var `err'.
+        * eshell/em-rebind.el (eshell-delete-backward-char): Remove unused arg
+        `killflag'.
+        * eshell/em-pred.el (eshell-parse-modifiers): Remove unused var `err'.
+        * eshell/em-ls.el (eshell-ls-highlight-alist): Move defvars before
+        first use.
+        * eshell/em-glob.el (eshell-glob-matches, message-shown):
+        Move declaration before first use.
+        * eshell/em-alias.el (eshell-maybe-replace-by-alias): Use backquotes.
         * autorevert.el (auto-revert-notify-handler): Use `cl-dolist' since we
         rely on cl-return.
 
diff --git a/lisp/eshell/em-alias.el b/lisp/eshell/em-alias.el
index a46b48c01b3..9a9cc4cd567 100644
--- a/lisp/eshell/em-alias.el
+++ b/lisp/eshell/em-alias.el
@@ -221,18 +221,11 @@ file named by `eshell-aliases-file'.")
     (let ((alias (eshell-lookup-alias command)))
       (if alias
           (throw 'eshell-replace-command
-                 (list
-                  'let
-                  (list
-                   (list 'eshell-command-name
-                         (list 'quote eshell-last-command-name))
-                   (list 'eshell-command-arguments
-                         (list 'quote eshell-last-arguments))
-                   (list 'eshell-prevent-alias-expansion
-                         (list 'quote
-                               (cons command
-                                     eshell-prevent-alias-expansion))))
-                  (eshell-parse-command (nth 1 alias))))))))
+                 `(let ((eshell-command-name ',eshell-last-command-name)
+                        (eshell-command-arguments ',eshell-last-arguments)
+                        (eshell-prevent-alias-expansion
+                         ',(cons command eshell-prevent-alias-expansion)))
+                    ,(eshell-parse-command (nth 1 alias))))))))
 
 (defun eshell-alias-completions (name)
   "Find all possible completions for NAME.
diff --git a/lisp/eshell/em-glob.el b/lisp/eshell/em-glob.el
index a58c7730ded..b5ca8119470 100644
--- a/lisp/eshell/em-glob.el
+++ b/lisp/eshell/em-glob.el
@@ -180,6 +180,8 @@ interpretation."
                 (goto-char (1+ end))))))))))
 
 (defvar eshell-glob-chars-regexp nil)
+(defvar eshell-glob-matches)
+(defvar message-shown)
 
 (defun eshell-glob-regexp (pattern)
   "Convert glob-pattern PATTERN to a regular expression.
@@ -262,9 +264,6 @@ the form:
             (error "No matches found: %s" glob)
           glob))))
 
-(defvar eshell-glob-matches)
-(defvar message-shown)
-
 ;; FIXME does this really need to abuse eshell-glob-matches, message-shown?
 (defun eshell-glob-entries (path globs &optional recurse-p)
   "Glob the entries in PATHS, possibly recursing if RECURSE-P is non-nil."
diff --git a/lisp/eshell/em-ls.el b/lisp/eshell/em-ls.el
index 41db4cd03d1..3dee1adb58a 100644
--- a/lisp/eshell/em-ls.el
+++ b/lisp/eshell/em-ls.el
@@ -268,6 +268,25 @@ scope during the evaluation of TEST-SEXP."
   :type '(repeat (cons function face))
   :group 'eshell-ls)
 
+(defvar block-size)
+(defvar dereference-links)
+(defvar dir-literal)
+(defvar error-func)
+(defvar flush-func)
+(defvar human-readable)
+(defvar ignore-pattern)
+(defvar insert-func)
+(defvar listing-style)
+(defvar numeric-uid-gid)
+(defvar reverse-list)
+(defvar show-all)
+(defvar show-almost-all)
+(defvar show-recursive)
+(defvar show-size)
+(defvar sort-method)
+(defvar ange-cache)
+(defvar dired-flag)
+
 ;;; Functions:
 
 (defun eshell-ls-insert-directory
@@ -315,25 +334,6 @@ instead."
 
 (put 'eshell/ls 'eshell-no-numeric-conversions t)
 
-(defvar block-size)
-(defvar dereference-links)
-(defvar dir-literal)
-(defvar error-func)
-(defvar flush-func)
-(defvar human-readable)
-(defvar ignore-pattern)
-(defvar insert-func)
-(defvar listing-style)
-(defvar numeric-uid-gid)
-(defvar reverse-list)
-(defvar show-all)
-(defvar show-almost-all)
-(defvar show-recursive)
-(defvar show-size)
-(defvar sort-method)
-(defvar ange-cache)
-(defvar dired-flag)
-
 (declare-function eshell-glob-regexp "em-glob" (pattern))
 
 (defun eshell-do-ls (&rest args)
diff --git a/lisp/eshell/em-pred.el b/lisp/eshell/em-pred.el
index 3a7f46ebe83..14d3020530f 100644
--- a/lisp/eshell/em-pred.el
+++ b/lisp/eshell/em-pred.el
@@ -307,7 +307,7 @@ functions.  PRED-FUNCS take a filename and return t if the test
 succeeds; MOD-FUNCS take any string and preform a modification,
 returning the resultant string."
   (let (result negate follow preds mods)
-    (condition-case err
+    (condition-case nil
         (while (not (eobp))
           (let ((char (char-after)))
             (cond
diff --git a/lisp/eshell/em-rebind.el b/lisp/eshell/em-rebind.el
index 341191fc62f..a526d590307 100644
--- a/lisp/eshell/em-rebind.el
+++ b/lisp/eshell/em-rebind.el
@@ -218,7 +218,7 @@ lock it at that."
         (cdar bindings))
       (setq bindings (cdr bindings)))))
 
-(defun eshell-delete-backward-char (n &optional killflag)
+(defun eshell-delete-backward-char (n)
   "Delete the last character, unless it's part of the output."
   (interactive "P")
   (let ((count (prefix-numeric-value n)))
diff --git a/lisp/eshell/em-unix.el b/lisp/eshell/em-unix.el
index af54d875cb0..b9b1c1635a5 100644
--- a/lisp/eshell/em-unix.el
+++ b/lisp/eshell/em-unix.el
@@ -714,6 +714,8 @@ available..."
           (goto-char (point-min))
           (resize-temp-buffer-window))))))
 
+(defvar compilation-scroll-output)
+
 (defun eshell-grep (command args &optional maybe-use-occur)
   "Generic service function for the various grep aliases.
 It calls Emacs's grep utility if the command is not redirecting output,
@@ -989,7 +991,7 @@ Show wall-clock time elapsed during execution of COMMAND.")
             (setq args nil)
           (setcdr (last args 3) nil))
         (with-current-buffer
-            (condition-case err
+            (condition-case nil
                 (diff-no-select
                  old new
                  (nil-blank-string (eshell-flatten-and-stringify args)))
@@ -1014,6 +1016,8 @@ Show wall-clock time elapsed during execution of COMMAND.")
 
 (put 'eshell/diff 'eshell-no-numeric-conversions t)
 
+(defvar locate-history-list)
+
 (defun eshell/locate (&rest args)
   "Alias \"locate\" to call Emacs `locate' function."
   (if (or eshell-plain-locate-behavior
diff --git a/lisp/eshell/esh-arg.el b/lisp/eshell/esh-arg.el
index d7dfd27d8d3..e3a12d5ece5 100644
--- a/lisp/eshell/esh-arg.el
+++ b/lisp/eshell/esh-arg.el
@@ -278,7 +278,7 @@ Point is left at the end of the arguments."
     (eshell-resolve-current-argument)
     eshell-current-argument))
 
-(defsubst eshell-operator (&rest args)
+(defsubst eshell-operator (&rest _args)
   "A stub function that generates an error if a floating operator is found."
   (error "Unhandled operator in input text"))
 
diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index ef8a53f3c0b..c2922983ae2 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -650,7 +650,7 @@ For an external command, it means an exit code of 0."
            (looking-at eshell-lisp-regexp))
       (let* ((here (point))
              (obj
-              (condition-case err
+              (condition-case nil
                   (read (current-buffer))
                 (end-of-file
                  (throw 'eshell-incomplete ?\()))))
diff --git a/lisp/eshell/esh-opt.el b/lisp/eshell/esh-opt.el
index 33625433022..c62cbc7e1dc 100644
--- a/lisp/eshell/esh-opt.el
+++ b/lisp/eshell/esh-opt.el
@@ -28,11 +28,11 @@
 (require 'esh-ext)
 
 ;; Unused.
-;;; (defgroup eshell-opt nil
-;;;   "The options processing code handles command argument parsing for
-;;; Eshell commands implemented in Lisp."
-;;;   :tag "Command options processing"
-;;;   :group 'eshell)
+;; (defgroup eshell-opt nil
+;;   "The options processing code handles command argument parsing for
+;; Eshell commands implemented in Lisp."
+;;   :tag "Command options processing"
+;;   :group 'eshell)
 
 ;;; User Functions:
 
@@ -103,32 +103,25 @@ interned variable `args' (created using a `let' form)."
                macro-args
              (list 'eshell-stringify-list
                    (list 'eshell-flatten-list macro-args)))))
-     (let ,(append (delq nil (mapcar (lambda (opt)
+     (let ,(delq nil (mapcar (lambda (opt)
                                        (and (listp opt) (nth 3 opt)))
                                      (cadr options)))
-                   '(usage-msg last-value ext-command args))
        ;; FIXME: `options' ends up hiding some variable names under `quote',
        ;; which is incompatible with lexical scoping!!
-       (eshell-do-opt ,name ,options (lambda () ,@body-forms)))))
+       (eshell-do-opt ,name ,options (lambda (args) ,@body-forms) temp-args))))
 
 ;;; Internal Functions:
 
-(defvar temp-args)
-(defvar last-value)
-(defvar usage-msg)
-(defvar ext-command)
 ;; Documented part of the interface; see eshell-eval-using-options.
-(defvar args)
+(defvar eshell--args)
 
-(defun eshell-do-opt (name options body-fun)
+(defun eshell-do-opt (name options body-fun args)
   "Helper function for `eshell-eval-using-options'.
 This code doesn't really need to be macro expanded everywhere."
-  (setq args temp-args)
-  (if (setq
-       ext-command
+  (let* (last-value
+         (ext-command
        (catch 'eshell-ext-command
-         (when (setq
-                usage-msg
+            (let ((usage-msg
                 (catch 'eshell-usage
                   (setq last-value nil)
                   (if (and (= (length args) 0)
@@ -136,12 +129,14 @@ This code doesn't really need to be macro expanded everywhere."
                       (throw 'eshell-usage
                              (eshell-show-usage name options)))
                   (setq args (eshell-process-args name args options)
-                        last-value (funcall body-fun))
-                  nil))
-           (error "%s" usage-msg))))
+                        last-value (funcall body-fun args))
+                     nil)))
+              (when usage-msg
+                (error "%s" usage-msg))))))
+    (if ext-command
       (throw 'eshell-external
              (eshell-external-command ext-command args))
-    last-value))
+      last-value)))
 
 (defun eshell-show-usage (name options)
   "Display the usage message for NAME, using OPTIONS."
@@ -197,12 +192,13 @@ will be modified."
   (if (not (nth 3 opt))
       (eshell-show-usage name options)
     (if (eq (nth 2 opt) t)
-        (if (> ai (length args))
+        (if (> ai (length eshell--args))
             (error "%s: missing option argument" name)
-          (set (nth 3 opt) (nth ai args))
+          (set (nth 3 opt) (nth ai eshell--args))
           (if (> ai 0)
-              (setcdr (nthcdr (1- ai) args) (nthcdr (1+ ai) args))
-            (setq args (cdr args))))
+              (setcdr (nthcdr (1- ai) eshell--args)
+                      (nthcdr (1+ ai) eshell--args))
+            (setq eshell--args (cdr eshell--args))))
       (set (nth 3 opt) (or (nth 2 opt) t)))))
 
 (defun eshell-process-option (name switch kind ai options)
@@ -232,14 +228,15 @@ switch is unrecognized."
           (setq extcmd (eshell-search-path (cadr extcmd)))
           (if extcmd
               (throw 'eshell-ext-command extcmd)
-            (if (characterp switch)
-                (error "%s: unrecognized option -%c" name switch)
-              (error "%s: unrecognized option --%s" name switch))))))))
+            (error (if (characterp switch) "%s: unrecognized option -%c"
+                     "%s: unrecognized option --%s")
+                   name switch)))))))
 
 (defun eshell-process-args (name args options)
   "Process the given ARGS using OPTIONS.
 This assumes that symbols have been intern'd by `eshell-eval-using-options'."
-  (let ((ai 0) arg)
+  (let ((ai 0) arg
+        (eshell--args args))
     (while (< ai (length args))
       (setq arg (nth ai args))
       (if (not (and (stringp arg)
diff --git a/lisp/eshell/esh-util.el b/lisp/eshell/esh-util.el
index dd344eb50a2..968d1ebad79 100644
--- a/lisp/eshell/esh-util.el
+++ b/lisp/eshell/esh-util.el
@@ -23,6 +23,8 @@
 
 ;;; Code:
 
+(eval-when-compile (require 'cl-lib))
+
 (defgroup eshell-util nil
   "This is general utility code, meant for use by Eshell itself."
   :tag "General utilities"
@@ -484,12 +486,12 @@ list."
       (while (re-search-forward
               "^\\([^#[:space:]]+\\)\\s-+\\(\\S-+\\)\\(\\s-*\\(\\S-+\\)\\)?" nil t)
         (if (match-string 1)
-            (add-to-list 'hosts (match-string 1)))
+            (cl-pushnew (match-string 1) hosts :test #'equal))
         (if (match-string 2)
-            (add-to-list 'hosts (match-string 2)))
+            (cl-pushnew (match-string 2) hosts :test #'equal))
         (if (match-string 4)
-            (add-to-list 'hosts (match-string 4)))))
-    (sort hosts 'string-lessp)))
+            (cl-pushnew (match-string 4) hosts :test #'equal))))
+    (sort hosts #'string-lessp)))
 
 (defun eshell-read-hosts (file result-var timestamp-var)
   "Read the contents of /etc/passwd for user names."
diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index 188b8165248..75c36a68544 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -395,12 +395,9 @@ process any indices that come after the variable reference."
           indices (and (not (eobp))
                        (eq (char-after) ?\[)
                        (eshell-parse-indices))
-          value (list 'let
-                      (list (list 'indices
-                                  (list 'quote indices)))
-                      value))
+          value `(let ((indices ',indices)) ,value))
     (if get-len
-        (list 'length value)
+        `(length ,value)
       value)))
 
 (defun eshell-parse-variable-ref ()
@@ -414,67 +411,68 @@ Possible options are:
   <LONG-NAME>   disambiguates the length of the name
   {COMMAND}     result of command is variable's value
   (LISP-FORM)   result of Lisp form is variable's value"
-  (let (end)
-    (cond
-     ((eq (char-after) ?{)
-      (let ((end (eshell-find-delimiter ?\{ ?\})))
-        (if (not end)
-            (throw 'eshell-incomplete ?\{)
-          (prog1
-              (list 'eshell-convert
-                    (list 'eshell-command-to-value
-                          (list 'eshell-as-subcommand
-                                (eshell-parse-command
-                                 (cons (1+ (point)) end)))))
-            (goto-char (1+ end))))))
-     ((memq (char-after) '(?\' ?\"))
-      (let ((name (if (eq (char-after) ?\')
-                      (eshell-parse-literal-quote)
-                    (eshell-parse-double-quote))))
-        (if name
+  (cond
+   ((eq (char-after) ?{)
+    (let ((end (eshell-find-delimiter ?\{ ?\})))
+      (if (not end)
+          (throw 'eshell-incomplete ?\{)
+        (prog1
+            (list 'eshell-convert
+                  (list 'eshell-command-to-value
+                        (list 'eshell-as-subcommand
+                              (eshell-parse-command
+                               (cons (1+ (point)) end)))))
+          (goto-char (1+ end))))))
+   ((memq (char-after) '(?\' ?\"))
+    (let ((name (if (eq (char-after) ?\')
+                    (eshell-parse-literal-quote)
+                  (eshell-parse-double-quote))))
+      (if name
           (list 'eshell-get-variable (eval name) 'indices))))
-     ((eq (char-after) ?\<)
-      (let ((end (eshell-find-delimiter ?\< ?\>)))
-        (if (not end)
-            (throw 'eshell-incomplete ?\<)
-          (let* ((temp (make-temp-file temporary-file-directory))
-                 (cmd (concat (buffer-substring (1+ (point)) end)
-                              " > " temp)))
-            (prog1
-                (list
-                 'let (list (list 'eshell-current-handles
-                                  (list 'eshell-create-handles temp
-                                        (list 'quote 'overwrite))))
-                 (list
-                  'progn
-                  (list 'eshell-as-subcommand
-                        (eshell-parse-command cmd))
-                  (list 'ignore
-                        (list 'nconc 'eshell-this-command-hook
-                              (list 'list
-                                    (list 'function
-                                          (list 'lambda nil
-                                                (list 'delete-file temp))))))
-                  (list 'quote temp)))
-              (goto-char (1+ end)))))))
-     ((eq (char-after) ?\()
-      (condition-case err
-          (list 'eshell-command-to-value
-                (list 'eshell-lisp-command
-                      (list 'quote (read (current-buffer)))))
-        (end-of-file
-         (throw 'eshell-incomplete ?\())))
-     ((assoc (char-to-string (char-after))
-             eshell-variable-aliases-list)
-      (forward-char)
-      (list 'eshell-get-variable
-            (char-to-string (char-before)) 'indices))
-     ((looking-at eshell-variable-name-regexp)
-      (prog1
-          (list 'eshell-get-variable (match-string 0) 'indices)
-        (goto-char (match-end 0))))
-     (t
-      (error "Invalid variable reference")))))
+   ((eq (char-after) ?\<)
+    (let ((end (eshell-find-delimiter ?\< ?\>)))
+      (if (not end)
+          (throw 'eshell-incomplete ?\<)
+        (let* ((temp (make-temp-file temporary-file-directory))
+               (cmd (concat (buffer-substring (1+ (point)) end)
+                            " > " temp)))
+          (prog1
+              (list
+               'let (list (list 'eshell-current-handles
+                                (list 'eshell-create-handles temp
+                                      (list 'quote 'overwrite))))
+               (list
+                'progn
+                (list 'eshell-as-subcommand
+                      (eshell-parse-command cmd))
+                (list 'ignore
+                      (list 'nconc 'eshell-this-command-hook
+                            (list 'list
+                                  (list 'function
+                                        (list 'lambda nil
+                                              (list 'delete-file temp))))))
+                (list 'quote temp)))
+            (goto-char (1+ end)))))))
+   ((eq (char-after) ?\()
+    (condition-case nil
+        (list 'eshell-command-to-value
+              (list 'eshell-lisp-command
+                    (list 'quote (read (current-buffer)))))
+      (end-of-file
+       (throw 'eshell-incomplete ?\())))
+   ((assoc (char-to-string (char-after))
+           eshell-variable-aliases-list)
+    (forward-char)
+    (list 'eshell-get-variable
+          (char-to-string (char-before)) 'indices))
+   ((looking-at eshell-variable-name-regexp)
+    (prog1
+        (list 'eshell-get-variable (match-string 0) 'indices)
+      (goto-char (match-end 0))))
+   (t
+    (error "Invalid variable reference"))))
+
+(defvar eshell-glob-function)
 
 (defun eshell-parse-indices ()
   "Parse and return a list of list of indices."
diff --git a/lisp/eshell/eshell.el b/lisp/eshell/eshell.el
index 9bdf8b3eb68..e3f8f0d11bc 100644
--- a/lisp/eshell/eshell.el
+++ b/lisp/eshell/eshell.el
@@ -300,7 +300,7 @@ buffer selected (or created)."
                     (get-buffer-create eshell-buffer-name)))))
     (cl-assert (and buf (buffer-live-p buf)))
     (pop-to-buffer-same-window buf)
-    (unless (eq major-mode 'eshell-mode)
+    (unless (derived-mode-p 'eshell-mode)
       (eshell-mode))
     buf))