Consolidate Eshell module loading/unloading code

This also adds the ability to suppress module loading/unloading
messages, which will be necessary to support running Eshell scripts as
batch scripts.

* lisp/eshell/esh-mode.el (eshell-mode): Move module
loading/initialization to...

* lisp/eshell/esh-module.el (eshell-load-modules)
(eshell-initialize-modules): ... here.
(eshell-module-loading-messages): New option.
(eshell-module--feature-name): Improve docstring.
(eshell-unload-modules): Display a real warning if unable to unload a
module.

* test/lisp/eshell/eshell-tests-helpers.el (with-temp-eshell)
(eshell-command-result-equal):
* test/lisp/eshell/eshell-tests-unload.el (load-eshell): Silence Eshell
loading messages.

4 files changed, 72 insertions, 41 deletions

diff --git a/lisp/eshell/esh-mode.el b/lisp/eshell/esh-mode.el
index c4ae55afe3f..7290c29b008 100644
--- a/lisp/eshell/esh-mode.el
+++ b/lisp/eshell/esh-mode.el
@@ -372,36 +372,15 @@ and the hook `eshell-exit-hook'."
   ;; strong R2L character.
   (setq bidi-paragraph-direction 'left-to-right)
 
-  ;; load extension modules into memory.  This will cause any global
-  ;; variables they define to be visible, since some of the core
-  ;; modules sometimes take advantage of their functionality if used.
-  (dolist (module eshell-modules-list)
-    (let ((module-fullname (symbol-name module))
-          module-shortname)
-      (if (string-match "^eshell-\\(.*\\)" module-fullname)
-          (setq module-shortname
-                (concat "em-" (match-string 1 module-fullname))))
-      (unless module-shortname
-        (error "Invalid Eshell module name: %s" module-fullname))
-      (unless (featurep (intern module-shortname))
-        (condition-case nil
-            (load module-shortname)
-          (error (lwarn 'eshell :error
-                        "Unable to load module `%s' (defined in `eshell-modules-list')"
-                        module-fullname))))))
+  ;; Load extension modules into memory.
+  (eshell-load-modules eshell-modules-list)
 
   (unless (file-exists-p eshell-directory-name)
     (eshell-make-private-directory eshell-directory-name t))
 
-  ;; Load core Eshell modules, then extension modules, for this session.
-  (dolist (module (append (eshell-subgroups 'eshell) eshell-modules-list))
-    (let ((load-hook (intern-soft (format "%s-load-hook" module)))
-          (initfunc (intern-soft (format "%s-initialize" module))))
-      (when (and load-hook (boundp load-hook))
-        (if (memq initfunc (symbol-value load-hook)) (setq initfunc nil))
-        (run-hooks load-hook))
-      ;; So we don't need the -initialize functions on the hooks (bug#5375).
-      (and initfunc (fboundp initfunc) (funcall initfunc))))
+  ;; Initialize core Eshell modules, then extension modules, for this session.
+  (eshell-initialize-modules (eshell-subgroups 'eshell))
+  (eshell-initialize-modules eshell-modules-list)
 
   (if eshell-send-direct-to-subprocesses
       (add-hook 'pre-command-hook #'eshell-intercept-commands t t))
diff --git a/lisp/eshell/esh-module.el b/lisp/eshell/esh-module.el
index fbd5ae4b9b8..88221638e16 100644
--- a/lisp/eshell/esh-module.el
+++ b/lisp/eshell/esh-module.el
@@ -49,6 +49,12 @@ customizing the variable `eshell-modules-list'."
   :group 'eshell-module)
 (make-obsolete-variable 'eshell-module-unload-hook nil "30.1")
 
+(defcustom eshell-module-loading-messages t
+  "If non-nil, display messages when loading/unloading Eshell modules."
+  :type 'boolean
+  :group 'eshell-module
+  :version "30.1")
+
 (defcustom eshell-modules-list
   '(eshell-alias
     eshell-banner
@@ -87,7 +93,9 @@ Changes will only take effect in future Eshell buffers."
 ;;; Code:
 
 (defsubst eshell-module--feature-name (module &optional kind)
-  "Get the feature name for the specified Eshell MODULE."
+  "Get the feature name for the specified Eshell MODULE.
+KIND can be either `core' for a core module or `extension' for an
+extension module; if nil, KIND defaults to `extension'."
   (let ((module-name (symbol-name module))
         (prefix (cond ((eq kind 'core) "esh-")
                       ((memq kind '(extension nil)) "em-")
@@ -102,17 +110,57 @@ The MODULE should be a symbol corresponding to that module's
 customization group.  Example: `eshell-cmpl' for that module."
   (memq module eshell-modules-list))
 
-(defun eshell-unload-modules (modules &optional kind)
-  "Try to unload the specified Eshell MODULES."
+(defun eshell-load-modules (modules)
+  "Load Eshell MODULES into memory.
+This will cause any global variables they define to be visible so
+that other modules can take advantage of their functionality if
+desired."
+  (let ((verbose eshell-module-loading-messages))
+    (dolist (module modules)
+      (let ((module-feature-name (eshell-module--feature-name module)))
+        (unless (featurep (intern module-feature-name))
+          (when verbose (message "Loading %s..." module))
+          (condition-case-unless-debug nil
+              (progn
+                (load module-feature-name nil t)
+                (when verbose (message "Loading %s...done" module)))
+            (error (when verbose (message "Loading %s...failed" module))
+                   (lwarn 'eshell :error
+                          "Unable to load Eshell module `%s'"
+                          module))))))))
+
+(defun eshell-initialize-modules (modules)
+  "Initialize Eshell MODULES.
+This calls `MODULE-load-hook' and `MODULE-initialize' for each
+MODULE, if they're defined."
   (dolist (module modules)
-    (let ((module-feature (intern (eshell-module--feature-name module kind))))
-      (when (featurep module-feature)
-        (message "Unloading %s..." (symbol-name module))
-        (condition-case-unless-debug _
-            (progn
-              (unload-feature module-feature)
-              (message "Unloading %s...done" (symbol-name module)))
-          (error (message "Unloading %s...failed" (symbol-name module))))))))
+    (let ((load-hook (intern-soft (format "%s-load-hook" module)))
+          (initfunc (intern-soft (format "%s-initialize" module))))
+      (when (and load-hook (boundp load-hook))
+        (if (memq initfunc (symbol-value load-hook)) (setq initfunc nil))
+        (run-hooks load-hook))
+      ;; So we don't need the -initialize functions on the hooks (bug#5375).
+      (and initfunc (fboundp initfunc) (funcall initfunc)))))
+
+(defun eshell-unload-modules (modules &optional kind)
+  "Try to unload the specified Eshell MODULES.
+KIND can be either `core' for core modules or `extension' for
+extension modules; if nil, KIND defaults to `extension'."
+  ;; We're about to unload this module, but we need to remember whether
+  ;; to print messages.
+  (let ((verbose eshell-module-loading-messages))
+    (dolist (module modules)
+      (let ((module-feature (intern (eshell-module--feature-name module kind))))
+        (when (featurep module-feature)
+          (when verbose (message "Unloading %s..." module))
+          (condition-case-unless-debug nil
+              (progn
+                (unload-feature module-feature)
+                (when verbose (message "Unloading %s...done" module)))
+            (error (when verbose (message "Unloading %s...failed" module))
+                   (lwarn 'eshell :error
+                          "Unable to unload Eshell module `%s'"
+                          module))))))))
 
 (defun eshell-unload-extension-modules ()
   "Try to unload all currently-loaded Eshell extension modules."
diff --git a/test/lisp/eshell/eshell-tests-helpers.el b/test/lisp/eshell/eshell-tests-helpers.el
index 652146fefcc..3f1c55f420d 100644
--- a/test/lisp/eshell/eshell-tests-helpers.el
+++ b/test/lisp/eshell/eshell-tests-helpers.el
@@ -63,6 +63,7 @@ beginning of the test file."
               (eshell-debug-command (cons 'process eshell-debug-command))
               (eshell-history-file-name nil)
               (eshell-last-dir-ring-file-name nil)
+              (eshell-module-loading-messages nil)
               (eshell-buffer (eshell t)))
          (unwind-protect
              (with-current-buffer eshell-buffer
@@ -183,10 +184,11 @@ inserting the command."
 (defun eshell-command-result-equal (command result)
   "Execute COMMAND non-interactively and compare it to RESULT."
   (ert-info (#'eshell-get-debug-logs :prefix "Command logs: ")
-    (should (eshell-command-result--equal
-             command
-             (eshell-test-command-result command)
-             result))))
+    (let ((eshell-module-loading-messages nil))
+      (should (eshell-command-result--equal
+               command
+               (eshell-test-command-result command)
+               result)))))
 
 (provide 'eshell-tests-helpers)
 
diff --git a/test/lisp/eshell/eshell-tests-unload.el b/test/lisp/eshell/eshell-tests-unload.el
index bf8291ba47a..479090e8f8c 100644
--- a/test/lisp/eshell/eshell-tests-unload.el
+++ b/test/lisp/eshell/eshell-tests-unload.el
@@ -33,6 +33,7 @@
 (defvar eshell-history-file-name)
 (defvar eshell-last-dir-ring-file-name)
 (defvar eshell-modules-list)
+(defvar eshell-module-loading-messages)
 
 (declare-function eshell-module--feature-name "esh-module"
                   (module &optional kind))
@@ -51,6 +52,7 @@ See `unload-eshell'.")
              (process-environment (cons "HISTFILE" process-environment))
              (eshell-history-file-name nil)
              (eshell-last-dir-ring-file-name nil)
+             (eshell-module-loading-messages nil)
              (eshell-buffer (eshell t)))
         (let (kill-buffer-query-functions)
           (kill-buffer eshell-buffer))))))