* bug#61501: 30.0.50; [PATCH] Support unloading Eshell
@ 2023-02-14 3:52 Jim Porter
2023-02-14 14:34 ` Eli Zaretskii
0 siblings, 1 reply; 7+ messages in thread
From: Jim Porter @ 2023-02-14 3:52 UTC (permalink / raw)
To: 61501
[-- Attachment #1: Type: text/plain, Size: 659 bytes --]
Currently, it's not possible to call "(unload-feature 'eshell)" after
using Eshell. You'll just get an error that some of Eshell's extension
modules depend on eshell.elc. This is a relatively minor issue, but I'm
hoping to make some improvements to how extension modules get loaded, so
this is a prelude to that.
In addition to making it *possible* to unload Eshell, I also fixed an
issue where the unload hooks weren't named correctly. They were of the
form 'eshell-hist-unload-hook', but the file's name is em-hist.el. I
moved these to 'em-hist-unload-function' and similar ('-function'
because the '-hook' version is obsolete, as I understand it).
[-- Attachment #2: 0001-Don-t-require-eshell-in-other-Eshell-files.patch --]
[-- Type: text/plain, Size: 6586 bytes --]
From 522f31164da4186af9d15b5608c721ca377ba3c0 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 12 Feb 2023 23:25:40 -0800
Subject: [PATCH 1/2] ; Don't require 'eshell' in other Eshell files
This isn't necessary and just makes unloading Eshell harder.
* lisp/eshell/em-banner.el:
* lisp/eshell/em-basic.el:
* lisp/eshell/em-cmpl.el:
* lisp/eshell/em-glob.el:
* lisp/eshell/em-prompt.el:
* lisp/eshell/em-rebind.el:
* lisp/eshell/em-smart.el:
* lisp/eshell/em-term.el:
* lisp/eshell/em-tramp.el:
* lisp/eshell/em-xtra.el:
Stop requiring 'eshell', and instead require specific subcomponents.
* lisp/eshell/em-hist.el: Stop requiring 'eshell' and 'em-pred'
(extension modules shouldn't require each other so they can be
independent).
(eshell-hist-parse-modifier): Ensure this can only be called when
'em-pred' is in use, and declare the relevant function.
* lisp/eshell/eshell.el (eshell-non-interactive-p): Move from here...
* lisp/eshell/esh-mode.el (eshell-non-interactive-p): ... to here.
---
lisp/eshell/em-banner.el | 1 -
lisp/eshell/em-basic.el | 5 +++--
lisp/eshell/em-cmpl.el | 4 +---
lisp/eshell/em-glob.el | 3 ++-
lisp/eshell/em-hist.el | 4 ++--
lisp/eshell/em-prompt.el | 2 --
lisp/eshell/em-rebind.el | 1 -
lisp/eshell/em-smart.el | 1 -
lisp/eshell/em-term.el | 1 -
lisp/eshell/em-tramp.el | 3 +--
lisp/eshell/em-xtra.el | 2 --
lisp/eshell/esh-mode.el | 5 +++++
lisp/eshell/eshell.el | 5 +----
13 files changed, 15 insertions(+), 22 deletions(-)
diff --git a/lisp/eshell/em-banner.el b/lisp/eshell/em-banner.el
index 8bc497bdeb3..2bee50b80a4 100644
--- a/lisp/eshell/em-banner.el
+++ b/lisp/eshell/em-banner.el
@@ -43,7 +43,6 @@
(require 'esh-util)
(require 'esh-mode)
-(require 'eshell)
;;;###autoload
(progn
diff --git a/lisp/eshell/em-basic.el b/lisp/eshell/em-basic.el
index bfff3bdf56e..016afe811b2 100644
--- a/lisp/eshell/em-basic.el
+++ b/lisp/eshell/em-basic.el
@@ -53,9 +53,10 @@
;;; Code:
-(require 'esh-util)
-(require 'eshell)
+(require 'esh-cmd)
+(require 'esh-io)
(require 'esh-opt)
+(require 'esh-util)
;;;###autoload
(progn
diff --git a/lisp/eshell/em-cmpl.el b/lisp/eshell/em-cmpl.el
index 2439f1ed804..af8ac4278f1 100644
--- a/lisp/eshell/em-cmpl.el
+++ b/lisp/eshell/em-cmpl.el
@@ -74,9 +74,7 @@
(require 'esh-util)
(require 'em-dirs)
-(eval-when-compile
- (require 'cl-lib)
- (require 'eshell))
+(eval-when-compile (require 'cl-lib))
;;;###autoload
(progn
diff --git a/lisp/eshell/em-glob.el b/lisp/eshell/em-glob.el
index 716f5c32b87..c7360fb246e 100644
--- a/lisp/eshell/em-glob.el
+++ b/lisp/eshell/em-glob.el
@@ -49,8 +49,9 @@
;;; Code:
+(require 'esh-arg)
+(require 'esh-module)
(require 'esh-util)
-(eval-when-compile (require 'eshell))
;;;###autoload
(progn
diff --git a/lisp/eshell/em-hist.el b/lisp/eshell/em-hist.el
index 6e0e471d910..4796df1604e 100644
--- a/lisp/eshell/em-hist.el
+++ b/lisp/eshell/em-hist.el
@@ -59,8 +59,6 @@
(require 'ring)
(require 'esh-opt)
(require 'esh-mode)
-(require 'em-pred)
-(require 'eshell)
;;;###autoload
(progn
@@ -769,6 +767,8 @@ eshell-hist-parse-word-designator
(defun eshell-hist-parse-modifier (hist reference)
"Parse a history modifier beginning for HIST in REFERENCE."
+ (cl-assert (eshell-using-module 'em-pred))
+ (declare-function eshell-parse-modifiers "em-pred" ())
(let ((here (point)))
(insert reference)
(prog1
diff --git a/lisp/eshell/em-prompt.el b/lisp/eshell/em-prompt.el
index b3a0fadf618..9f9e58e83d7 100644
--- a/lisp/eshell/em-prompt.el
+++ b/lisp/eshell/em-prompt.el
@@ -27,8 +27,6 @@
;;; Code:
(require 'esh-mode)
-(eval-when-compile (require 'eshell))
-
(require 'text-property-search)
;;;###autoload
diff --git a/lisp/eshell/em-rebind.el b/lisp/eshell/em-rebind.el
index f147d432300..75a2848a9d5 100644
--- a/lisp/eshell/em-rebind.el
+++ b/lisp/eshell/em-rebind.el
@@ -24,7 +24,6 @@
;;; Code:
(require 'esh-mode)
-(eval-when-compile (require 'eshell))
;;;###autoload
(progn
diff --git a/lisp/eshell/em-smart.el b/lisp/eshell/em-smart.el
index ca04c429785..154ff760212 100644
--- a/lisp/eshell/em-smart.el
+++ b/lisp/eshell/em-smart.el
@@ -69,7 +69,6 @@
;;; Code:
(require 'esh-mode)
-(eval-when-compile (require 'eshell))
;;;###autoload
(progn
diff --git a/lisp/eshell/em-term.el b/lisp/eshell/em-term.el
index a4d777e4a0d..ab26da857b7 100644
--- a/lisp/eshell/em-term.el
+++ b/lisp/eshell/em-term.el
@@ -34,7 +34,6 @@
(require 'cl-lib)
(require 'esh-util)
(require 'esh-ext)
-(eval-when-compile (require 'eshell))
(require 'term)
;;;###autoload
diff --git a/lisp/eshell/em-tramp.el b/lisp/eshell/em-tramp.el
index 13dd62e1617..94eb9797033 100644
--- a/lisp/eshell/em-tramp.el
+++ b/lisp/eshell/em-tramp.el
@@ -29,8 +29,7 @@
(require 'esh-cmd)
(eval-when-compile
- (require 'esh-mode)
- (require 'eshell))
+ (require 'esh-mode))
(require 'tramp)
diff --git a/lisp/eshell/em-xtra.el b/lisp/eshell/em-xtra.el
index defaa7b2887..45c3ea3c0fc 100644
--- a/lisp/eshell/em-xtra.el
+++ b/lisp/eshell/em-xtra.el
@@ -25,8 +25,6 @@
(require 'cl-lib)
(require 'esh-util)
-(eval-when-compile
- (require 'eshell))
;; There are no items in this custom group, but eshell modules (ab)use
;; custom groups.
diff --git a/lisp/eshell/esh-mode.el b/lisp/eshell/esh-mode.el
index 1b8f5ff8018..e0af3579edf 100644
--- a/lisp/eshell/esh-mode.el
+++ b/lisp/eshell/esh-mode.el
@@ -200,6 +200,11 @@ eshell-directory-name
(defvar eshell-first-time-p t
"A variable which is non-nil the first time Eshell is loaded.")
+(defvar eshell-non-interactive-p nil
+ "A variable which is non-nil when Eshell is not running interactively.
+Modules should use this variable so that they don't clutter
+non-interactive sessions, such as when using `eshell-command'.")
+
;; Internal Variables:
;; these are only set to nil initially for the sake of the
diff --git a/lisp/eshell/eshell.el b/lisp/eshell/eshell.el
index 7a7ece5cb7c..0bfc0413cbf 100644
--- a/lisp/eshell/eshell.el
+++ b/lisp/eshell/eshell.el
@@ -267,10 +267,7 @@ eshell-command-mode
(define-obsolete-function-alias 'eshell-return-exits-minibuffer
#'eshell-command-mode "28.1")
-(defvar eshell-non-interactive-p nil
- "A variable which is non-nil when Eshell is not running interactively.
-Modules should use this variable so that they don't clutter
-non-interactive sessions, such as when using `eshell-command'.")
+(defvar eshell-non-interactive-p) ; Defined in esh-mode.el.
(declare-function eshell-add-input-to-history "em-hist" (input))
--
2.25.1
[-- Attachment #3: 0002-Allow-unloading-Eshell.patch --]
[-- Type: text/plain, Size: 16233 bytes --]
From aa1260aac01b3e798a1a1a6a609a18b841d30c53 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 12 Feb 2023 23:25:59 -0800
Subject: [PATCH 2/2] Allow unloading Eshell
* lisp/eshell/em-extpipe.el (eshell-extpipe):
* lisp/eshell/esh-opt.el (eshell-opt): New groups. Eshell uses these
to identify modules to unload.
* lisp/eshell/em-hist.el (eshell-hist-unload-hook):
* lisp/eshell/em-ls.el (eshell-ls-unload-hook):
* lisp/eshell/em-smart.el (eshell-smart-unload-hook):
* lisp/eshell/eshell.el (eshell-unload-hook): Make obsolete and move
to...
* lisp/eshell/em-smart.el (em-smart-unload-function):
* lisp/eshell/em-hist.el (em-hist-unload-function):
* lisp/eshell/em-ls.el (eshell-ls-unload-function):
* lisp/eshell/eshell.el (eshell-unload-function): ... these.
* lisp/eshell/esh-mode.el (eshell-mode-unload-hook):
* lisp/eshell/esh-module.el (eshell-module-unload-hook): Make
obsolete.
* lisp/eshell/em-ls (eshell-ls-enable-in-dired,
eshell-ls-disable-in-dired): New functions...
(eshell-ls-use-in-dired): ... use them.
* lisp/eshell/esh-module.el (eshell-module--feature-name,
eshell-unload-modules): New functions.
(eshell-unload-extension-modules): Use 'eshell-unload-modules'.
* lisp/eshell/eshell.el (eshell-unload-all-modules): Remove.
* test/lisp/eshell/eshell-tests-unload.el: New file.
* doc/misc/eshell.texi (Bugs and ideas): Remove item about unloading
Eshell not working.
---
doc/misc/eshell.texi | 2 -
lisp/eshell/em-extpipe.el | 15 ++++
lisp/eshell/em-hist.el | 4 +
lisp/eshell/em-ls.el | 31 ++++----
lisp/eshell/em-smart.el | 4 +
lisp/eshell/esh-mode.el | 1 +
lisp/eshell/esh-module.el | 32 ++++++--
lisp/eshell/esh-opt.el | 5 ++
lisp/eshell/eshell.el | 33 +++------
test/lisp/eshell/eshell-tests-unload.el | 99 +++++++++++++++++++++++++
10 files changed, 179 insertions(+), 47 deletions(-)
create mode 100644 test/lisp/eshell/eshell-tests-unload.el
diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index e51e2cf799b..1c33c04f647 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -2189,8 +2189,6 @@ Bugs and ideas
cause it to track the bottom of the output; but backspace no longer
scrolls back.
-@item It's not possible to fully @code{unload-feature} Eshell
-
@item Menu support was removed, but never put back
@item If an interactive process is currently running, @kbd{M-!} doesn't work
diff --git a/lisp/eshell/em-extpipe.el b/lisp/eshell/em-extpipe.el
index 9078c44ed9f..5c9a0a85934 100644
--- a/lisp/eshell/em-extpipe.el
+++ b/lisp/eshell/em-extpipe.el
@@ -36,6 +36,21 @@
(eval-when-compile (require 'files-x))
+;;;###autoload
+(progn
+(defgroup eshell-extpipe nil
+ "Native shell pipelines.
+
+This module lets you construct pipelines that use your operating
+system's shell instead of Eshell's own pipelining support. This
+is especially relevant when executing commands on a remote
+machine using Eshell's Tramp integration: using the remote
+shell's pipelining avoids copying the data which will flow
+through the pipeline to local Emacs buffers and then right back
+again."
+ :tag "External pipelines"
+ :group 'eshell-module))
+
;;; Functions:
(defun eshell-extpipe-initialize () ;Called from `eshell-mode' via intern-soft!
diff --git a/lisp/eshell/em-hist.el b/lisp/eshell/em-hist.el
index 4796df1604e..2c199ec160f 100644
--- a/lisp/eshell/em-hist.el
+++ b/lisp/eshell/em-hist.el
@@ -80,6 +80,7 @@ eshell-hist-unload-hook
(remove-hook 'kill-emacs-hook 'eshell-save-some-history)))
"A hook that gets run when `eshell-hist' is unloaded."
:type 'hook)
+(make-obsolete-variable 'eshell-hist-unload-hook nil "30.1")
(defcustom eshell-history-file-name
(expand-file-name "history" eshell-directory-name)
@@ -1037,6 +1038,9 @@ eshell-isearch-return
(isearch-done)
(eshell-send-input))
+(defun em-hist-unload-function ()
+ (remove-hook 'kill-emacs-hook 'eshell-save-some-history))
+
(provide 'em-hist)
;; Local Variables:
diff --git a/lisp/eshell/em-ls.el b/lisp/eshell/em-ls.el
index 7e2a7578ef9..56c5f262789 100644
--- a/lisp/eshell/em-ls.el
+++ b/lisp/eshell/em-ls.el
@@ -62,24 +62,27 @@ eshell-ls-dired-initial-args
This is useful for enabling human-readable format (-h), for example."
:type '(repeat :tag "Arguments" string))
+(defun eshell-ls-enable-in-dired ()
+ "Use `eshell-ls' to read directories in Dired."
+ (require 'dired)
+ (advice-add 'insert-directory :around #'eshell-ls--insert-directory)
+ (advice-add 'dired :around #'eshell-ls--dired))
+
+(defun eshell-ls-disable-in-dired ()
+ "Stop using `eshell-ls' to read directories in Dired."
+ (advice-remove 'insert-directory #'eshell-ls--insert-directory)
+ (advice-remove 'dired #'eshell-ls--dired))
+
(defcustom eshell-ls-use-in-dired nil
"If non-nil, use `eshell-ls' to read directories in Dired.
Changing this without using customize has no effect."
:set (lambda (symbol value)
- (cond (value
- (require 'dired)
- (advice-add 'insert-directory :around
- #'eshell-ls--insert-directory)
- (advice-add 'dired :around #'eshell-ls--dired))
- (t
- (advice-remove 'insert-directory
- #'eshell-ls--insert-directory)
- (advice-remove 'dired #'eshell-ls--dired)))
+ (if value
+ (eshell-ls-enable-in-dired)
+ (eshell-ls-disable-in-dired))
(set symbol value))
:type 'boolean
:require 'em-ls)
-(add-hook 'eshell-ls-unload-hook #'eshell-ls-unload-function)
-
(defcustom eshell-ls-default-blocksize 1024
"The default blocksize to use when display file sizes with -s."
@@ -954,10 +957,8 @@ eshell-ls-decorated-name
(car file)))))
(car file))
-(defun eshell-ls-unload-function ()
- (advice-remove 'insert-directory #'eshell-ls--insert-directory)
- (advice-remove 'dired #'eshell-ls--dired)
- nil)
+(defun em-ls-unload-function ()
+ (eshell-ls-disable-in-dired))
(provide 'em-ls)
diff --git a/lisp/eshell/em-smart.el b/lisp/eshell/em-smart.el
index 154ff760212..d8b7fadc2c2 100644
--- a/lisp/eshell/em-smart.el
+++ b/lisp/eshell/em-smart.el
@@ -99,6 +99,7 @@ eshell-smart-unload-hook
"A hook that gets run when `eshell-smart' is unloaded."
:type 'hook
:group 'eshell-smart)
+(make-obsolete-variable 'eshell-smart-unload-hook nil "30.1")
(defcustom eshell-review-quick-commands nil
"If t, always review commands.
@@ -321,6 +322,9 @@ eshell-smart-display-move
(if clear
(remove-hook 'pre-command-hook 'eshell-smart-display-move t))))
+(defun em-smart-unload-hook ()
+ (remove-hook 'window-configuration-change-hook #'eshell-refresh-windows))
+
(provide 'em-smart)
;; Local Variables:
diff --git a/lisp/eshell/esh-mode.el b/lisp/eshell/esh-mode.el
index e0af3579edf..b3cde472713 100644
--- a/lisp/eshell/esh-mode.el
+++ b/lisp/eshell/esh-mode.el
@@ -79,6 +79,7 @@ eshell-mode
(defcustom eshell-mode-unload-hook nil
"A hook that gets run when `eshell-mode' is unloaded."
:type 'hook)
+(make-obsolete-variable 'eshell-mode-unload-hook nil "30.1")
(defcustom eshell-mode-hook nil
"A hook that gets run when `eshell-mode' is entered."
diff --git a/lisp/eshell/esh-module.el b/lisp/eshell/esh-module.el
index 651e793ad98..7fc74d38796 100644
--- a/lisp/eshell/esh-module.el
+++ b/lisp/eshell/esh-module.el
@@ -47,6 +47,7 @@ eshell-module-unload-hook
"A hook run when `eshell-module' is unloaded."
:type 'hook
:group 'eshell-module)
+(make-obsolete-variable 'eshell-module-unload-hook nil "30.1")
(defcustom eshell-modules-list
'(eshell-alias
@@ -85,20 +86,37 @@ eshell-modules-list
;;; Code:
+(defsubst eshell-module--feature-name (module &optional kind)
+ "Get the feature name for the specified Eshell MODULE."
+ (let ((module-name (symbol-name module))
+ (prefix (cond ((eq kind 'core) "esh-")
+ ((memq kind '(extension nil)) "em-")
+ (t (error "unknown module kind %s" kind)))))
+ (if (string-match "^eshell-\\(.*\\)" module-name)
+ (concat prefix (match-string 1 module-name))
+ (error "Invalid Eshell module name: %s" module))))
+
(defsubst eshell-using-module (module)
"Return non-nil if a certain Eshell MODULE is in use.
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."
+ (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))))))))
+
(defun eshell-unload-extension-modules ()
- "Unload any memory resident extension modules."
- (dolist (module (eshell-subgroups 'eshell-module))
- (if (featurep module)
- (ignore-errors
- (message "Unloading %s..." (symbol-name module))
- (unload-feature module)
- (message "Unloading %s...done" (symbol-name module))))))
+ "Try to unload all currently-loaded Eshell extension modules."
+ (eshell-unload-modules (eshell-subgroups 'eshell-module)))
(provide 'esh-module)
;;; esh-module.el ends here
diff --git a/lisp/eshell/esh-opt.el b/lisp/eshell/esh-opt.el
index 9253f9a4a7d..09c19767a19 100644
--- a/lisp/eshell/esh-opt.el
+++ b/lisp/eshell/esh-opt.el
@@ -29,6 +29,11 @@
;; defined in esh-util.
(require 'esh-util)
+(defgroup eshell-opt nil
+ "Functions for argument parsing in Eshell commands."
+ :tag "Option parsing"
+ :group 'eshell)
+
(defmacro eshell-eval-using-options (name macro-args options &rest body-forms)
"Process NAME's MACRO-ARGS using a set of command line OPTIONS.
After doing so, stores settings in local symbols as declared by OPTIONS;
diff --git a/lisp/eshell/eshell.el b/lisp/eshell/eshell.el
index 0bfc0413cbf..7d2c0335db2 100644
--- a/lisp/eshell/eshell.el
+++ b/lisp/eshell/eshell.el
@@ -199,10 +199,11 @@ eshell-load-hook
:type 'hook
:group 'eshell)
-(defcustom eshell-unload-hook '(eshell-unload-all-modules)
+(defcustom eshell-unload-hook nil
"A hook run when Eshell is unloaded from memory."
:type 'hook
:group 'eshell)
+(make-obsolete-variable 'eshell-unload-hook nil "30.1")
(defcustom eshell-buffer-name "*eshell*"
"The basename used for Eshell buffers.
@@ -370,28 +371,14 @@ eshell-command-result
(set status-var eshell-last-command-status))
(cadr result))))))
-;;; Code:
-
-(defun eshell-unload-all-modules ()
- "Unload all modules that were loaded by Eshell, if possible.
-If the user has require'd in any of the modules, or customized a
-variable with a :require tag (such as `eshell-prefer-to-shell'), it
-will be impossible to unload Eshell completely without restarting
-Emacs."
- ;; if the user set `eshell-prefer-to-shell' to t, but never loaded
- ;; Eshell, then `eshell-subgroups' will be unbound
- (when (fboundp 'eshell-subgroups)
- (dolist (module (eshell-subgroups 'eshell))
- ;; this really only unloads as many modules as possible,
- ;; since other `require' references (such as by customizing
- ;; `eshell-prefer-to-shell' to a non-nil value) might make it
- ;; impossible to unload Eshell completely
- (if (featurep module)
- (ignore-errors
- (message "Unloading %s..." (symbol-name module))
- (unload-feature module)
- (message "Unloading %s...done" (symbol-name module)))))
- (message "Unloading eshell...done")))
+(defun eshell-unload-function ()
+ (eshell-unload-extension-modules)
+ ;; Wait to unload core modules until after `eshell' has finished
+ ;; unloading. `eshell' depends on several of them, so they can't be
+ ;; unloaded immediately.
+ (run-at-time 0 nil #'eshell-unload-modules
+ (reverse (eshell-subgroups 'eshell)) 'core)
+ nil)
(run-hooks 'eshell-load-hook)
diff --git a/test/lisp/eshell/eshell-tests-unload.el b/test/lisp/eshell/eshell-tests-unload.el
new file mode 100644
index 00000000000..cdd58efef18
--- /dev/null
+++ b/test/lisp/eshell/eshell-tests-unload.el
@@ -0,0 +1,99 @@
+;;; eshell-tests-unload.el --- test unloading Eshell -*- lexical-binding:t -*-
+
+;; Copyright (C) 2023 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs. If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; Tests for unloading Eshell.
+
+;;; Code:
+
+(require 'ert)
+(require 'ert-x)
+
+;; In order to test unloading Eshell, don't require any of its files
+;; at the top level. This means we need to explicitly declare some of
+;; the variables and functions we'll use.
+(defvar eshell-directory-name)
+(defvar eshell-history-file-name)
+(defvar eshell-last-dir-ring-file-name)
+(defvar eshell-modules-list)
+
+(declare-function eshell-module--feature-name "esh-module"
+ (module &optional kind))
+(declare-function eshell-subgroups "esh-util" (groupsym))
+
+(defvar max-unload-time 5
+ "The maximum amount of time to wait to unload Eshell modules, in seconds.
+See `unload-eshell'.")
+
+(defun load-eshell ()
+ "Load Eshell by calling the `eshell' function and immediately closing it."
+ (save-current-buffer
+ (ert-with-temp-directory eshell-directory-name
+ (let* (;; We want no history file, so prevent Eshell from falling
+ ;; back on $HISTFILE.
+ (process-environment (cons "HISTFILE" process-environment))
+ (eshell-history-file-name nil)
+ (eshell-last-dir-ring-file-name nil)
+ (eshell-buffer (eshell t)))
+ (let (kill-buffer-query-functions)
+ (kill-buffer eshell-buffer))))))
+
+(defun unload-eshell ()
+ "Unload Eshell, waiting until the core modules are unloaded as well."
+ (let ((debug-on-error t)
+ (inhibit-message t))
+ (unload-feature 'eshell)
+ ;; We unload core modules are unloaded from a timer, since they
+ ;; need to wait until after `eshell' itself is unloaded. Wait for
+ ;; this to finish.
+ (let ((start (current-time)))
+ (while (featurep 'esh-arg)
+ (when (> (float-time (time-since start))
+ max-unload-time)
+ (error "timed out waiting to unload Eshell modules"))
+ (sit-for 0.1)))))
+
+;;; Tests:
+
+(ert-deftest eshell-test-unload/default ()
+ "Test unloading Eshell with the default list of extension modules."
+ (load-eshell)
+ (unload-eshell))
+
+(ert-deftest eshell-test-unload/no-modules ()
+ "Test unloading Eshell with no extension modules."
+ (require 'esh-module)
+ (let (eshell-modules-list)
+ (load-eshell))
+ (dolist (module (eshell-subgroups 'eshell-module))
+ (should-not (featurep (intern (eshell-module--feature-name module)))))
+ (unload-eshell))
+
+(ert-deftest eshell-test-unload/all-modules ()
+ "Test unloading Eshell with every extension module."
+ (require 'esh-module)
+ (let ((eshell-modules-list (eshell-subgroups 'eshell-module)))
+ (load-eshell))
+ (dolist (module (eshell-subgroups 'eshell-module))
+ (should (featurep (intern (eshell-module--feature-name module)))))
+ (unload-eshell))
+
+(provide 'eshell-tests-unload)
+;;; eshell-tests-unload.el ends here
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* bug#61501: 30.0.50; [PATCH] Support unloading Eshell
2023-02-14 3:52 bug#61501: 30.0.50; [PATCH] Support unloading Eshell Jim Porter
@ 2023-02-14 14:34 ` Eli Zaretskii
2023-02-15 0:56 ` Jim Porter
0 siblings, 1 reply; 7+ messages in thread
From: Eli Zaretskii @ 2023-02-14 14:34 UTC (permalink / raw)
To: Jim Porter; +Cc: 61501
> Date: Mon, 13 Feb 2023 19:52:13 -0800
> From: Jim Porter <jporterbugs@gmail.com>
>
> Currently, it's not possible to call "(unload-feature 'eshell)" after
> using Eshell. You'll just get an error that some of Eshell's extension
> modules depend on eshell.elc. This is a relatively minor issue, but I'm
> hoping to make some improvements to how extension modules get loaded, so
> this is a prelude to that.
>
> In addition to making it *possible* to unload Eshell, I also fixed an
> issue where the unload hooks weren't named correctly. They were of the
> form 'eshell-hist-unload-hook', but the file's name is em-hist.el. I
> moved these to 'em-hist-unload-function' and similar ('-function'
> because the '-hook' version is obsolete, as I understand it).
Thanks, good to hear. Please add a NEWS entry to call out this
change.
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#61501: 30.0.50; [PATCH] Support unloading Eshell
2023-02-14 14:34 ` Eli Zaretskii
@ 2023-02-15 0:56 ` Jim Porter
2023-02-15 12:59 ` Eli Zaretskii
0 siblings, 1 reply; 7+ messages in thread
From: Jim Porter @ 2023-02-15 0:56 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 61501
[-- Attachment #1: Type: text/plain, Size: 147 bytes --]
On 2/14/2023 6:34 AM, Eli Zaretskii wrote:
> Thanks, good to hear. Please add a NEWS entry to call out this
> change.
Thanks. How does this look?
[-- Attachment #2: 0002-Allow-unloading-Eshell.patch --]
[-- Type: text/plain, Size: 16888 bytes --]
From 743ea9facc76151a50a928e6265154b112604f27 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 12 Feb 2023 23:25:59 -0800
Subject: [PATCH 2/2] Allow unloading Eshell
* lisp/eshell/em-extpipe.el (eshell-extpipe):
* lisp/eshell/esh-opt.el (eshell-opt): New groups. Eshell uses these
to identify modules to unload.
* lisp/eshell/em-hist.el (eshell-hist-unload-hook):
* lisp/eshell/em-ls.el (eshell-ls-unload-hook):
* lisp/eshell/em-smart.el (eshell-smart-unload-hook):
* lisp/eshell/eshell.el (eshell-unload-hook): Make obsolete and move
to...
* lisp/eshell/em-smart.el (em-smart-unload-function):
* lisp/eshell/em-hist.el (em-hist-unload-function):
* lisp/eshell/em-ls.el (em-ls-unload-function):
* lisp/eshell/eshell.el (eshell-unload-function): ... these.
* lisp/eshell/esh-mode.el (eshell-mode-unload-hook):
* lisp/eshell/esh-module.el (eshell-module-unload-hook): Make
obsolete.
* lisp/eshell/em-ls (eshell-ls-enable-in-dired,
eshell-ls-disable-in-dired): New functions...
(eshell-ls-use-in-dired): ... use them.
* lisp/eshell/esh-module.el (eshell-module--feature-name,
eshell-unload-modules): New functions.
(eshell-unload-extension-modules): Use 'eshell-unload-modules'.
* lisp/eshell/eshell.el (eshell-unload-all-modules): Remove.
* test/lisp/eshell/eshell-tests-unload.el: New file.
* doc/misc/eshell.texi (Bugs and ideas): Remove item about unloading
Eshell not working.
* etc/NEWS: Announce the obsoletion of the old hooks.
---
doc/misc/eshell.texi | 2 -
etc/NEWS | 6 ++
lisp/eshell/em-extpipe.el | 15 ++++
lisp/eshell/em-hist.el | 4 +
lisp/eshell/em-ls.el | 31 ++++----
lisp/eshell/em-smart.el | 4 +
lisp/eshell/esh-mode.el | 1 +
lisp/eshell/esh-module.el | 32 ++++++--
lisp/eshell/esh-opt.el | 5 ++
lisp/eshell/eshell.el | 33 +++------
test/lisp/eshell/eshell-tests-unload.el | 99 +++++++++++++++++++++++++
11 files changed, 185 insertions(+), 47 deletions(-)
create mode 100644 test/lisp/eshell/eshell-tests-unload.el
diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index e51e2cf799b..1c33c04f647 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -2189,8 +2189,6 @@ Bugs and ideas
cause it to track the bottom of the output; but backspace no longer
scrolls back.
-@item It's not possible to fully @code{unload-feature} Eshell
-
@item Menu support was removed, but never put back
@item If an interactive process is currently running, @kbd{M-!} doesn't work
diff --git a/etc/NEWS b/etc/NEWS
index 2d63593ff17..a51e9d9006c 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -196,6 +196,12 @@ their customization options.
This user option has been obsoleted in Emacs 27, use
'remote-file-name-inhibit-cache' instead.
+---
+** User options 'eshell-NAME-unload-hook' are now obsolete.
+These hooks were named incorrectly, and so they never actually ran
+when unloading the correspending feature. Instead, you should use
+hooks named after the feature name, like 'esh-mode-unload-hook'.
+
\f
* Lisp Changes in Emacs 30.1
diff --git a/lisp/eshell/em-extpipe.el b/lisp/eshell/em-extpipe.el
index 9078c44ed9f..5c9a0a85934 100644
--- a/lisp/eshell/em-extpipe.el
+++ b/lisp/eshell/em-extpipe.el
@@ -36,6 +36,21 @@
(eval-when-compile (require 'files-x))
+;;;###autoload
+(progn
+(defgroup eshell-extpipe nil
+ "Native shell pipelines.
+
+This module lets you construct pipelines that use your operating
+system's shell instead of Eshell's own pipelining support. This
+is especially relevant when executing commands on a remote
+machine using Eshell's Tramp integration: using the remote
+shell's pipelining avoids copying the data which will flow
+through the pipeline to local Emacs buffers and then right back
+again."
+ :tag "External pipelines"
+ :group 'eshell-module))
+
;;; Functions:
(defun eshell-extpipe-initialize () ;Called from `eshell-mode' via intern-soft!
diff --git a/lisp/eshell/em-hist.el b/lisp/eshell/em-hist.el
index 4796df1604e..2c199ec160f 100644
--- a/lisp/eshell/em-hist.el
+++ b/lisp/eshell/em-hist.el
@@ -80,6 +80,7 @@ eshell-hist-unload-hook
(remove-hook 'kill-emacs-hook 'eshell-save-some-history)))
"A hook that gets run when `eshell-hist' is unloaded."
:type 'hook)
+(make-obsolete-variable 'eshell-hist-unload-hook nil "30.1")
(defcustom eshell-history-file-name
(expand-file-name "history" eshell-directory-name)
@@ -1037,6 +1038,9 @@ eshell-isearch-return
(isearch-done)
(eshell-send-input))
+(defun em-hist-unload-function ()
+ (remove-hook 'kill-emacs-hook 'eshell-save-some-history))
+
(provide 'em-hist)
;; Local Variables:
diff --git a/lisp/eshell/em-ls.el b/lisp/eshell/em-ls.el
index 7e2a7578ef9..56c5f262789 100644
--- a/lisp/eshell/em-ls.el
+++ b/lisp/eshell/em-ls.el
@@ -62,24 +62,27 @@ eshell-ls-dired-initial-args
This is useful for enabling human-readable format (-h), for example."
:type '(repeat :tag "Arguments" string))
+(defun eshell-ls-enable-in-dired ()
+ "Use `eshell-ls' to read directories in Dired."
+ (require 'dired)
+ (advice-add 'insert-directory :around #'eshell-ls--insert-directory)
+ (advice-add 'dired :around #'eshell-ls--dired))
+
+(defun eshell-ls-disable-in-dired ()
+ "Stop using `eshell-ls' to read directories in Dired."
+ (advice-remove 'insert-directory #'eshell-ls--insert-directory)
+ (advice-remove 'dired #'eshell-ls--dired))
+
(defcustom eshell-ls-use-in-dired nil
"If non-nil, use `eshell-ls' to read directories in Dired.
Changing this without using customize has no effect."
:set (lambda (symbol value)
- (cond (value
- (require 'dired)
- (advice-add 'insert-directory :around
- #'eshell-ls--insert-directory)
- (advice-add 'dired :around #'eshell-ls--dired))
- (t
- (advice-remove 'insert-directory
- #'eshell-ls--insert-directory)
- (advice-remove 'dired #'eshell-ls--dired)))
+ (if value
+ (eshell-ls-enable-in-dired)
+ (eshell-ls-disable-in-dired))
(set symbol value))
:type 'boolean
:require 'em-ls)
-(add-hook 'eshell-ls-unload-hook #'eshell-ls-unload-function)
-
(defcustom eshell-ls-default-blocksize 1024
"The default blocksize to use when display file sizes with -s."
@@ -954,10 +957,8 @@ eshell-ls-decorated-name
(car file)))))
(car file))
-(defun eshell-ls-unload-function ()
- (advice-remove 'insert-directory #'eshell-ls--insert-directory)
- (advice-remove 'dired #'eshell-ls--dired)
- nil)
+(defun em-ls-unload-function ()
+ (eshell-ls-disable-in-dired))
(provide 'em-ls)
diff --git a/lisp/eshell/em-smart.el b/lisp/eshell/em-smart.el
index 154ff760212..d8b7fadc2c2 100644
--- a/lisp/eshell/em-smart.el
+++ b/lisp/eshell/em-smart.el
@@ -99,6 +99,7 @@ eshell-smart-unload-hook
"A hook that gets run when `eshell-smart' is unloaded."
:type 'hook
:group 'eshell-smart)
+(make-obsolete-variable 'eshell-smart-unload-hook nil "30.1")
(defcustom eshell-review-quick-commands nil
"If t, always review commands.
@@ -321,6 +322,9 @@ eshell-smart-display-move
(if clear
(remove-hook 'pre-command-hook 'eshell-smart-display-move t))))
+(defun em-smart-unload-hook ()
+ (remove-hook 'window-configuration-change-hook #'eshell-refresh-windows))
+
(provide 'em-smart)
;; Local Variables:
diff --git a/lisp/eshell/esh-mode.el b/lisp/eshell/esh-mode.el
index e0af3579edf..b3cde472713 100644
--- a/lisp/eshell/esh-mode.el
+++ b/lisp/eshell/esh-mode.el
@@ -79,6 +79,7 @@ eshell-mode
(defcustom eshell-mode-unload-hook nil
"A hook that gets run when `eshell-mode' is unloaded."
:type 'hook)
+(make-obsolete-variable 'eshell-mode-unload-hook nil "30.1")
(defcustom eshell-mode-hook nil
"A hook that gets run when `eshell-mode' is entered."
diff --git a/lisp/eshell/esh-module.el b/lisp/eshell/esh-module.el
index 651e793ad98..7fc74d38796 100644
--- a/lisp/eshell/esh-module.el
+++ b/lisp/eshell/esh-module.el
@@ -47,6 +47,7 @@ eshell-module-unload-hook
"A hook run when `eshell-module' is unloaded."
:type 'hook
:group 'eshell-module)
+(make-obsolete-variable 'eshell-module-unload-hook nil "30.1")
(defcustom eshell-modules-list
'(eshell-alias
@@ -85,20 +86,37 @@ eshell-modules-list
;;; Code:
+(defsubst eshell-module--feature-name (module &optional kind)
+ "Get the feature name for the specified Eshell MODULE."
+ (let ((module-name (symbol-name module))
+ (prefix (cond ((eq kind 'core) "esh-")
+ ((memq kind '(extension nil)) "em-")
+ (t (error "unknown module kind %s" kind)))))
+ (if (string-match "^eshell-\\(.*\\)" module-name)
+ (concat prefix (match-string 1 module-name))
+ (error "Invalid Eshell module name: %s" module))))
+
(defsubst eshell-using-module (module)
"Return non-nil if a certain Eshell MODULE is in use.
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."
+ (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))))))))
+
(defun eshell-unload-extension-modules ()
- "Unload any memory resident extension modules."
- (dolist (module (eshell-subgroups 'eshell-module))
- (if (featurep module)
- (ignore-errors
- (message "Unloading %s..." (symbol-name module))
- (unload-feature module)
- (message "Unloading %s...done" (symbol-name module))))))
+ "Try to unload all currently-loaded Eshell extension modules."
+ (eshell-unload-modules (eshell-subgroups 'eshell-module)))
(provide 'esh-module)
;;; esh-module.el ends here
diff --git a/lisp/eshell/esh-opt.el b/lisp/eshell/esh-opt.el
index 9253f9a4a7d..09c19767a19 100644
--- a/lisp/eshell/esh-opt.el
+++ b/lisp/eshell/esh-opt.el
@@ -29,6 +29,11 @@
;; defined in esh-util.
(require 'esh-util)
+(defgroup eshell-opt nil
+ "Functions for argument parsing in Eshell commands."
+ :tag "Option parsing"
+ :group 'eshell)
+
(defmacro eshell-eval-using-options (name macro-args options &rest body-forms)
"Process NAME's MACRO-ARGS using a set of command line OPTIONS.
After doing so, stores settings in local symbols as declared by OPTIONS;
diff --git a/lisp/eshell/eshell.el b/lisp/eshell/eshell.el
index 0bfc0413cbf..7d2c0335db2 100644
--- a/lisp/eshell/eshell.el
+++ b/lisp/eshell/eshell.el
@@ -199,10 +199,11 @@ eshell-load-hook
:type 'hook
:group 'eshell)
-(defcustom eshell-unload-hook '(eshell-unload-all-modules)
+(defcustom eshell-unload-hook nil
"A hook run when Eshell is unloaded from memory."
:type 'hook
:group 'eshell)
+(make-obsolete-variable 'eshell-unload-hook nil "30.1")
(defcustom eshell-buffer-name "*eshell*"
"The basename used for Eshell buffers.
@@ -370,28 +371,14 @@ eshell-command-result
(set status-var eshell-last-command-status))
(cadr result))))))
-;;; Code:
-
-(defun eshell-unload-all-modules ()
- "Unload all modules that were loaded by Eshell, if possible.
-If the user has require'd in any of the modules, or customized a
-variable with a :require tag (such as `eshell-prefer-to-shell'), it
-will be impossible to unload Eshell completely without restarting
-Emacs."
- ;; if the user set `eshell-prefer-to-shell' to t, but never loaded
- ;; Eshell, then `eshell-subgroups' will be unbound
- (when (fboundp 'eshell-subgroups)
- (dolist (module (eshell-subgroups 'eshell))
- ;; this really only unloads as many modules as possible,
- ;; since other `require' references (such as by customizing
- ;; `eshell-prefer-to-shell' to a non-nil value) might make it
- ;; impossible to unload Eshell completely
- (if (featurep module)
- (ignore-errors
- (message "Unloading %s..." (symbol-name module))
- (unload-feature module)
- (message "Unloading %s...done" (symbol-name module)))))
- (message "Unloading eshell...done")))
+(defun eshell-unload-function ()
+ (eshell-unload-extension-modules)
+ ;; Wait to unload core modules until after `eshell' has finished
+ ;; unloading. `eshell' depends on several of them, so they can't be
+ ;; unloaded immediately.
+ (run-at-time 0 nil #'eshell-unload-modules
+ (reverse (eshell-subgroups 'eshell)) 'core)
+ nil)
(run-hooks 'eshell-load-hook)
diff --git a/test/lisp/eshell/eshell-tests-unload.el b/test/lisp/eshell/eshell-tests-unload.el
new file mode 100644
index 00000000000..cdd58efef18
--- /dev/null
+++ b/test/lisp/eshell/eshell-tests-unload.el
@@ -0,0 +1,99 @@
+;;; eshell-tests-unload.el --- test unloading Eshell -*- lexical-binding:t -*-
+
+;; Copyright (C) 2023 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs. If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; Tests for unloading Eshell.
+
+;;; Code:
+
+(require 'ert)
+(require 'ert-x)
+
+;; In order to test unloading Eshell, don't require any of its files
+;; at the top level. This means we need to explicitly declare some of
+;; the variables and functions we'll use.
+(defvar eshell-directory-name)
+(defvar eshell-history-file-name)
+(defvar eshell-last-dir-ring-file-name)
+(defvar eshell-modules-list)
+
+(declare-function eshell-module--feature-name "esh-module"
+ (module &optional kind))
+(declare-function eshell-subgroups "esh-util" (groupsym))
+
+(defvar max-unload-time 5
+ "The maximum amount of time to wait to unload Eshell modules, in seconds.
+See `unload-eshell'.")
+
+(defun load-eshell ()
+ "Load Eshell by calling the `eshell' function and immediately closing it."
+ (save-current-buffer
+ (ert-with-temp-directory eshell-directory-name
+ (let* (;; We want no history file, so prevent Eshell from falling
+ ;; back on $HISTFILE.
+ (process-environment (cons "HISTFILE" process-environment))
+ (eshell-history-file-name nil)
+ (eshell-last-dir-ring-file-name nil)
+ (eshell-buffer (eshell t)))
+ (let (kill-buffer-query-functions)
+ (kill-buffer eshell-buffer))))))
+
+(defun unload-eshell ()
+ "Unload Eshell, waiting until the core modules are unloaded as well."
+ (let ((debug-on-error t)
+ (inhibit-message t))
+ (unload-feature 'eshell)
+ ;; We unload core modules are unloaded from a timer, since they
+ ;; need to wait until after `eshell' itself is unloaded. Wait for
+ ;; this to finish.
+ (let ((start (current-time)))
+ (while (featurep 'esh-arg)
+ (when (> (float-time (time-since start))
+ max-unload-time)
+ (error "timed out waiting to unload Eshell modules"))
+ (sit-for 0.1)))))
+
+;;; Tests:
+
+(ert-deftest eshell-test-unload/default ()
+ "Test unloading Eshell with the default list of extension modules."
+ (load-eshell)
+ (unload-eshell))
+
+(ert-deftest eshell-test-unload/no-modules ()
+ "Test unloading Eshell with no extension modules."
+ (require 'esh-module)
+ (let (eshell-modules-list)
+ (load-eshell))
+ (dolist (module (eshell-subgroups 'eshell-module))
+ (should-not (featurep (intern (eshell-module--feature-name module)))))
+ (unload-eshell))
+
+(ert-deftest eshell-test-unload/all-modules ()
+ "Test unloading Eshell with every extension module."
+ (require 'esh-module)
+ (let ((eshell-modules-list (eshell-subgroups 'eshell-module)))
+ (load-eshell))
+ (dolist (module (eshell-subgroups 'eshell-module))
+ (should (featurep (intern (eshell-module--feature-name module)))))
+ (unload-eshell))
+
+(provide 'eshell-tests-unload)
+;;; eshell-tests-unload.el ends here
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* bug#61501: 30.0.50; [PATCH] Support unloading Eshell
2023-02-15 0:56 ` Jim Porter
@ 2023-02-15 12:59 ` Eli Zaretskii
2023-02-15 17:31 ` Jim Porter
0 siblings, 1 reply; 7+ messages in thread
From: Eli Zaretskii @ 2023-02-15 12:59 UTC (permalink / raw)
To: Jim Porter; +Cc: 61501
> Date: Tue, 14 Feb 2023 16:56:54 -0800
> Cc: 61501@debbugs.gnu.org
> From: Jim Porter <jporterbugs@gmail.com>
>
> On 2/14/2023 6:34 AM, Eli Zaretskii wrote:
> > Thanks, good to hear. Please add a NEWS entry to call out this
> > change.
>
> Thanks. How does this look?
It's okay, but I actually thought about telling that unloading Eshell
now works...
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#61501: 30.0.50; [PATCH] Support unloading Eshell
2023-02-15 12:59 ` Eli Zaretskii
@ 2023-02-15 17:31 ` Jim Porter
2023-02-15 18:06 ` Eli Zaretskii
0 siblings, 1 reply; 7+ messages in thread
From: Jim Porter @ 2023-02-15 17:31 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 61501
[-- Attachment #1: Type: text/plain, Size: 745 bytes --]
On 2/15/2023 4:59 AM, Eli Zaretskii wrote:
>> Date: Tue, 14 Feb 2023 16:56:54 -0800
>> Cc: 61501@debbugs.gnu.org
>> From: Jim Porter <jporterbugs@gmail.com>
>>
>> On 2/14/2023 6:34 AM, Eli Zaretskii wrote:
>>> Thanks, good to hear. Please add a NEWS entry to call out this
>>> change.
>>
>> Thanks. How does this look?
>
> It's okay, but I actually thought about telling that unloading Eshell
> now works...
Oh, I see. How about this?
I could also try to merge the two news entries. However, since the
previous one that I added about hook names describes an incompatible
change, my gut feeling is to keep them separate, or just remove the
previous entry (I'm not sure anyone uses the old names since they didn't
work in the first place).
[-- Attachment #2: 0002-Allow-unloading-Eshell.patch --]
[-- Type: text/plain, Size: 17304 bytes --]
From 1456b83734e956bd3fb0415715e148e3dd3f8497 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 12 Feb 2023 23:25:59 -0800
Subject: [PATCH 2/2] Allow unloading Eshell
* lisp/eshell/em-extpipe.el (eshell-extpipe):
* lisp/eshell/esh-opt.el (eshell-opt): New groups. Eshell uses these
to identify modules to unload.
* lisp/eshell/em-hist.el (eshell-hist-unload-hook):
* lisp/eshell/em-ls.el (eshell-ls-unload-hook):
* lisp/eshell/em-smart.el (eshell-smart-unload-hook):
* lisp/eshell/eshell.el (eshell-unload-hook): Make obsolete and move
to...
* lisp/eshell/em-smart.el (em-smart-unload-function):
* lisp/eshell/em-hist.el (em-hist-unload-function):
* lisp/eshell/em-ls.el (em-ls-unload-function):
* lisp/eshell/eshell.el (eshell-unload-function): ... these.
* lisp/eshell/esh-mode.el (eshell-mode-unload-hook):
* lisp/eshell/esh-module.el (eshell-module-unload-hook): Make
obsolete.
* lisp/eshell/em-ls (eshell-ls-enable-in-dired,
eshell-ls-disable-in-dired): New functions...
(eshell-ls-use-in-dired): ... use them.
* lisp/eshell/esh-module.el (eshell-module--feature-name,
eshell-unload-modules): New functions.
(eshell-unload-extension-modules): Use 'eshell-unload-modules'.
* lisp/eshell/eshell.el (eshell-unload-all-modules): Remove.
* test/lisp/eshell/eshell-tests-unload.el: New file.
* doc/misc/eshell.texi (Bugs and ideas): Remove item about unloading
Eshell not working.
* etc/NEWS: Announce this change.
---
doc/misc/eshell.texi | 2 -
etc/NEWS | 11 +++
lisp/eshell/em-extpipe.el | 15 ++++
lisp/eshell/em-hist.el | 4 +
lisp/eshell/em-ls.el | 31 ++++----
lisp/eshell/em-smart.el | 4 +
lisp/eshell/esh-mode.el | 1 +
lisp/eshell/esh-module.el | 32 ++++++--
lisp/eshell/esh-opt.el | 5 ++
lisp/eshell/eshell.el | 33 +++------
test/lisp/eshell/eshell-tests-unload.el | 99 +++++++++++++++++++++++++
11 files changed, 190 insertions(+), 47 deletions(-)
create mode 100644 test/lisp/eshell/eshell-tests-unload.el
diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index e51e2cf799b..1c33c04f647 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -2189,8 +2189,6 @@ Bugs and ideas
cause it to track the bottom of the output; but backspace no longer
scrolls back.
-@item It's not possible to fully @code{unload-feature} Eshell
-
@item Menu support was removed, but never put back
@item If an interactive process is currently running, @kbd{M-!} doesn't work
diff --git a/etc/NEWS b/etc/NEWS
index 2d63593ff17..8eff74e00d3 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -131,6 +131,11 @@ this to your configuration:
(keymap-set eshell-mode-map "<home>" #'eshell-bol-ignoring-prompt)
+---
+*** You can now properly unload Eshell.
+Calling "(unload-feature 'eshell)" no longer signals an error, and now
+correctly unloads Eshell and all of its modules.
+
+++
*** 'eshell-read-aliases-list' is now an interactive command.
After manually editing 'eshell-aliases-file', you can use this command
@@ -196,6 +201,12 @@ their customization options.
This user option has been obsoleted in Emacs 27, use
'remote-file-name-inhibit-cache' instead.
+---
+** User options 'eshell-NAME-unload-hook' are now obsolete.
+These hooks were named incorrectly, and so they never actually ran
+when unloading the correspending feature. Instead, you should use
+hooks named after the feature name, like 'esh-mode-unload-hook'.
+
\f
* Lisp Changes in Emacs 30.1
diff --git a/lisp/eshell/em-extpipe.el b/lisp/eshell/em-extpipe.el
index 9078c44ed9f..5c9a0a85934 100644
--- a/lisp/eshell/em-extpipe.el
+++ b/lisp/eshell/em-extpipe.el
@@ -36,6 +36,21 @@
(eval-when-compile (require 'files-x))
+;;;###autoload
+(progn
+(defgroup eshell-extpipe nil
+ "Native shell pipelines.
+
+This module lets you construct pipelines that use your operating
+system's shell instead of Eshell's own pipelining support. This
+is especially relevant when executing commands on a remote
+machine using Eshell's Tramp integration: using the remote
+shell's pipelining avoids copying the data which will flow
+through the pipeline to local Emacs buffers and then right back
+again."
+ :tag "External pipelines"
+ :group 'eshell-module))
+
;;; Functions:
(defun eshell-extpipe-initialize () ;Called from `eshell-mode' via intern-soft!
diff --git a/lisp/eshell/em-hist.el b/lisp/eshell/em-hist.el
index 4796df1604e..2c199ec160f 100644
--- a/lisp/eshell/em-hist.el
+++ b/lisp/eshell/em-hist.el
@@ -80,6 +80,7 @@ eshell-hist-unload-hook
(remove-hook 'kill-emacs-hook 'eshell-save-some-history)))
"A hook that gets run when `eshell-hist' is unloaded."
:type 'hook)
+(make-obsolete-variable 'eshell-hist-unload-hook nil "30.1")
(defcustom eshell-history-file-name
(expand-file-name "history" eshell-directory-name)
@@ -1037,6 +1038,9 @@ eshell-isearch-return
(isearch-done)
(eshell-send-input))
+(defun em-hist-unload-function ()
+ (remove-hook 'kill-emacs-hook 'eshell-save-some-history))
+
(provide 'em-hist)
;; Local Variables:
diff --git a/lisp/eshell/em-ls.el b/lisp/eshell/em-ls.el
index 7e2a7578ef9..56c5f262789 100644
--- a/lisp/eshell/em-ls.el
+++ b/lisp/eshell/em-ls.el
@@ -62,24 +62,27 @@ eshell-ls-dired-initial-args
This is useful for enabling human-readable format (-h), for example."
:type '(repeat :tag "Arguments" string))
+(defun eshell-ls-enable-in-dired ()
+ "Use `eshell-ls' to read directories in Dired."
+ (require 'dired)
+ (advice-add 'insert-directory :around #'eshell-ls--insert-directory)
+ (advice-add 'dired :around #'eshell-ls--dired))
+
+(defun eshell-ls-disable-in-dired ()
+ "Stop using `eshell-ls' to read directories in Dired."
+ (advice-remove 'insert-directory #'eshell-ls--insert-directory)
+ (advice-remove 'dired #'eshell-ls--dired))
+
(defcustom eshell-ls-use-in-dired nil
"If non-nil, use `eshell-ls' to read directories in Dired.
Changing this without using customize has no effect."
:set (lambda (symbol value)
- (cond (value
- (require 'dired)
- (advice-add 'insert-directory :around
- #'eshell-ls--insert-directory)
- (advice-add 'dired :around #'eshell-ls--dired))
- (t
- (advice-remove 'insert-directory
- #'eshell-ls--insert-directory)
- (advice-remove 'dired #'eshell-ls--dired)))
+ (if value
+ (eshell-ls-enable-in-dired)
+ (eshell-ls-disable-in-dired))
(set symbol value))
:type 'boolean
:require 'em-ls)
-(add-hook 'eshell-ls-unload-hook #'eshell-ls-unload-function)
-
(defcustom eshell-ls-default-blocksize 1024
"The default blocksize to use when display file sizes with -s."
@@ -954,10 +957,8 @@ eshell-ls-decorated-name
(car file)))))
(car file))
-(defun eshell-ls-unload-function ()
- (advice-remove 'insert-directory #'eshell-ls--insert-directory)
- (advice-remove 'dired #'eshell-ls--dired)
- nil)
+(defun em-ls-unload-function ()
+ (eshell-ls-disable-in-dired))
(provide 'em-ls)
diff --git a/lisp/eshell/em-smart.el b/lisp/eshell/em-smart.el
index 154ff760212..d8b7fadc2c2 100644
--- a/lisp/eshell/em-smart.el
+++ b/lisp/eshell/em-smart.el
@@ -99,6 +99,7 @@ eshell-smart-unload-hook
"A hook that gets run when `eshell-smart' is unloaded."
:type 'hook
:group 'eshell-smart)
+(make-obsolete-variable 'eshell-smart-unload-hook nil "30.1")
(defcustom eshell-review-quick-commands nil
"If t, always review commands.
@@ -321,6 +322,9 @@ eshell-smart-display-move
(if clear
(remove-hook 'pre-command-hook 'eshell-smart-display-move t))))
+(defun em-smart-unload-hook ()
+ (remove-hook 'window-configuration-change-hook #'eshell-refresh-windows))
+
(provide 'em-smart)
;; Local Variables:
diff --git a/lisp/eshell/esh-mode.el b/lisp/eshell/esh-mode.el
index e0af3579edf..b3cde472713 100644
--- a/lisp/eshell/esh-mode.el
+++ b/lisp/eshell/esh-mode.el
@@ -79,6 +79,7 @@ eshell-mode
(defcustom eshell-mode-unload-hook nil
"A hook that gets run when `eshell-mode' is unloaded."
:type 'hook)
+(make-obsolete-variable 'eshell-mode-unload-hook nil "30.1")
(defcustom eshell-mode-hook nil
"A hook that gets run when `eshell-mode' is entered."
diff --git a/lisp/eshell/esh-module.el b/lisp/eshell/esh-module.el
index 651e793ad98..7fc74d38796 100644
--- a/lisp/eshell/esh-module.el
+++ b/lisp/eshell/esh-module.el
@@ -47,6 +47,7 @@ eshell-module-unload-hook
"A hook run when `eshell-module' is unloaded."
:type 'hook
:group 'eshell-module)
+(make-obsolete-variable 'eshell-module-unload-hook nil "30.1")
(defcustom eshell-modules-list
'(eshell-alias
@@ -85,20 +86,37 @@ eshell-modules-list
;;; Code:
+(defsubst eshell-module--feature-name (module &optional kind)
+ "Get the feature name for the specified Eshell MODULE."
+ (let ((module-name (symbol-name module))
+ (prefix (cond ((eq kind 'core) "esh-")
+ ((memq kind '(extension nil)) "em-")
+ (t (error "unknown module kind %s" kind)))))
+ (if (string-match "^eshell-\\(.*\\)" module-name)
+ (concat prefix (match-string 1 module-name))
+ (error "Invalid Eshell module name: %s" module))))
+
(defsubst eshell-using-module (module)
"Return non-nil if a certain Eshell MODULE is in use.
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."
+ (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))))))))
+
(defun eshell-unload-extension-modules ()
- "Unload any memory resident extension modules."
- (dolist (module (eshell-subgroups 'eshell-module))
- (if (featurep module)
- (ignore-errors
- (message "Unloading %s..." (symbol-name module))
- (unload-feature module)
- (message "Unloading %s...done" (symbol-name module))))))
+ "Try to unload all currently-loaded Eshell extension modules."
+ (eshell-unload-modules (eshell-subgroups 'eshell-module)))
(provide 'esh-module)
;;; esh-module.el ends here
diff --git a/lisp/eshell/esh-opt.el b/lisp/eshell/esh-opt.el
index 9253f9a4a7d..09c19767a19 100644
--- a/lisp/eshell/esh-opt.el
+++ b/lisp/eshell/esh-opt.el
@@ -29,6 +29,11 @@
;; defined in esh-util.
(require 'esh-util)
+(defgroup eshell-opt nil
+ "Functions for argument parsing in Eshell commands."
+ :tag "Option parsing"
+ :group 'eshell)
+
(defmacro eshell-eval-using-options (name macro-args options &rest body-forms)
"Process NAME's MACRO-ARGS using a set of command line OPTIONS.
After doing so, stores settings in local symbols as declared by OPTIONS;
diff --git a/lisp/eshell/eshell.el b/lisp/eshell/eshell.el
index 0bfc0413cbf..7d2c0335db2 100644
--- a/lisp/eshell/eshell.el
+++ b/lisp/eshell/eshell.el
@@ -199,10 +199,11 @@ eshell-load-hook
:type 'hook
:group 'eshell)
-(defcustom eshell-unload-hook '(eshell-unload-all-modules)
+(defcustom eshell-unload-hook nil
"A hook run when Eshell is unloaded from memory."
:type 'hook
:group 'eshell)
+(make-obsolete-variable 'eshell-unload-hook nil "30.1")
(defcustom eshell-buffer-name "*eshell*"
"The basename used for Eshell buffers.
@@ -370,28 +371,14 @@ eshell-command-result
(set status-var eshell-last-command-status))
(cadr result))))))
-;;; Code:
-
-(defun eshell-unload-all-modules ()
- "Unload all modules that were loaded by Eshell, if possible.
-If the user has require'd in any of the modules, or customized a
-variable with a :require tag (such as `eshell-prefer-to-shell'), it
-will be impossible to unload Eshell completely without restarting
-Emacs."
- ;; if the user set `eshell-prefer-to-shell' to t, but never loaded
- ;; Eshell, then `eshell-subgroups' will be unbound
- (when (fboundp 'eshell-subgroups)
- (dolist (module (eshell-subgroups 'eshell))
- ;; this really only unloads as many modules as possible,
- ;; since other `require' references (such as by customizing
- ;; `eshell-prefer-to-shell' to a non-nil value) might make it
- ;; impossible to unload Eshell completely
- (if (featurep module)
- (ignore-errors
- (message "Unloading %s..." (symbol-name module))
- (unload-feature module)
- (message "Unloading %s...done" (symbol-name module)))))
- (message "Unloading eshell...done")))
+(defun eshell-unload-function ()
+ (eshell-unload-extension-modules)
+ ;; Wait to unload core modules until after `eshell' has finished
+ ;; unloading. `eshell' depends on several of them, so they can't be
+ ;; unloaded immediately.
+ (run-at-time 0 nil #'eshell-unload-modules
+ (reverse (eshell-subgroups 'eshell)) 'core)
+ nil)
(run-hooks 'eshell-load-hook)
diff --git a/test/lisp/eshell/eshell-tests-unload.el b/test/lisp/eshell/eshell-tests-unload.el
new file mode 100644
index 00000000000..cdd58efef18
--- /dev/null
+++ b/test/lisp/eshell/eshell-tests-unload.el
@@ -0,0 +1,99 @@
+;;; eshell-tests-unload.el --- test unloading Eshell -*- lexical-binding:t -*-
+
+;; Copyright (C) 2023 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs. If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; Tests for unloading Eshell.
+
+;;; Code:
+
+(require 'ert)
+(require 'ert-x)
+
+;; In order to test unloading Eshell, don't require any of its files
+;; at the top level. This means we need to explicitly declare some of
+;; the variables and functions we'll use.
+(defvar eshell-directory-name)
+(defvar eshell-history-file-name)
+(defvar eshell-last-dir-ring-file-name)
+(defvar eshell-modules-list)
+
+(declare-function eshell-module--feature-name "esh-module"
+ (module &optional kind))
+(declare-function eshell-subgroups "esh-util" (groupsym))
+
+(defvar max-unload-time 5
+ "The maximum amount of time to wait to unload Eshell modules, in seconds.
+See `unload-eshell'.")
+
+(defun load-eshell ()
+ "Load Eshell by calling the `eshell' function and immediately closing it."
+ (save-current-buffer
+ (ert-with-temp-directory eshell-directory-name
+ (let* (;; We want no history file, so prevent Eshell from falling
+ ;; back on $HISTFILE.
+ (process-environment (cons "HISTFILE" process-environment))
+ (eshell-history-file-name nil)
+ (eshell-last-dir-ring-file-name nil)
+ (eshell-buffer (eshell t)))
+ (let (kill-buffer-query-functions)
+ (kill-buffer eshell-buffer))))))
+
+(defun unload-eshell ()
+ "Unload Eshell, waiting until the core modules are unloaded as well."
+ (let ((debug-on-error t)
+ (inhibit-message t))
+ (unload-feature 'eshell)
+ ;; We unload core modules are unloaded from a timer, since they
+ ;; need to wait until after `eshell' itself is unloaded. Wait for
+ ;; this to finish.
+ (let ((start (current-time)))
+ (while (featurep 'esh-arg)
+ (when (> (float-time (time-since start))
+ max-unload-time)
+ (error "timed out waiting to unload Eshell modules"))
+ (sit-for 0.1)))))
+
+;;; Tests:
+
+(ert-deftest eshell-test-unload/default ()
+ "Test unloading Eshell with the default list of extension modules."
+ (load-eshell)
+ (unload-eshell))
+
+(ert-deftest eshell-test-unload/no-modules ()
+ "Test unloading Eshell with no extension modules."
+ (require 'esh-module)
+ (let (eshell-modules-list)
+ (load-eshell))
+ (dolist (module (eshell-subgroups 'eshell-module))
+ (should-not (featurep (intern (eshell-module--feature-name module)))))
+ (unload-eshell))
+
+(ert-deftest eshell-test-unload/all-modules ()
+ "Test unloading Eshell with every extension module."
+ (require 'esh-module)
+ (let ((eshell-modules-list (eshell-subgroups 'eshell-module)))
+ (load-eshell))
+ (dolist (module (eshell-subgroups 'eshell-module))
+ (should (featurep (intern (eshell-module--feature-name module)))))
+ (unload-eshell))
+
+(provide 'eshell-tests-unload)
+;;; eshell-tests-unload.el ends here
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* bug#61501: 30.0.50; [PATCH] Support unloading Eshell
2023-02-15 17:31 ` Jim Porter
@ 2023-02-15 18:06 ` Eli Zaretskii
2023-02-16 1:34 ` Jim Porter
0 siblings, 1 reply; 7+ messages in thread
From: Eli Zaretskii @ 2023-02-15 18:06 UTC (permalink / raw)
To: Jim Porter; +Cc: 61501
> Date: Wed, 15 Feb 2023 09:31:57 -0800
> Cc: 61501@debbugs.gnu.org
> From: Jim Porter <jporterbugs@gmail.com>
>
> On 2/15/2023 4:59 AM, Eli Zaretskii wrote:
> >> Date: Tue, 14 Feb 2023 16:56:54 -0800
> >> Cc: 61501@debbugs.gnu.org
> >> From: Jim Porter <jporterbugs@gmail.com>
> >>
> >> On 2/14/2023 6:34 AM, Eli Zaretskii wrote:
> >>> Thanks, good to hear. Please add a NEWS entry to call out this
> >>> change.
> >>
> >> Thanks. How does this look?
> >
> > It's okay, but I actually thought about telling that unloading Eshell
> > now works...
>
> Oh, I see. How about this?
Perfect, thanks.
> I could also try to merge the two news entries.
I see no need: they are different aspects of the same changeset.
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#61501: 30.0.50; [PATCH] Support unloading Eshell
2023-02-15 18:06 ` Eli Zaretskii
@ 2023-02-16 1:34 ` Jim Porter
0 siblings, 0 replies; 7+ messages in thread
From: Jim Porter @ 2023-02-16 1:34 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 61501-done
On 2/15/2023 10:06 AM, Eli Zaretskii wrote:
> Perfect, thanks.
Thanks. Merged to master as 8051be9ac2. Closing this bug now.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-02-16 1:34 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-14 3:52 bug#61501: 30.0.50; [PATCH] Support unloading Eshell Jim Porter
2023-02-14 14:34 ` Eli Zaretskii
2023-02-15 0:56 ` Jim Porter
2023-02-15 12:59 ` Eli Zaretskii
2023-02-15 17:31 ` Jim Porter
2023-02-15 18:06 ` Eli Zaretskii
2023-02-16 1:34 ` Jim Porter
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).