bug#40679: 28.0.50; Multiple definitions of explicit-shell-file-name

bug#40679: 28.0.50; Multiple definitions of explicit-shell-file-name

[Basil L. Contovounesios]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: 0001-Define-explicit-shell-file-name-only-in-shell.el.patch --]
[-- Type: text/x-diff, Size: 7407 bytes --]

From 0744d8d7c8a6a4b9cdddec8a8a39abef06559e17 Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob@tcd.ie>
Date: Fri, 17 Apr 2020 10:27:36 +0100
Subject: [PATCH] Define explicit-shell-file-name only in shell.el

For discussion, see the following thread:
https://lists.gnu.org/archive/html/emacs-devel/2020-04/msg00880.html
* doc/emacs/cmdargs.texi (General Variables): Mention
shell-file-name in relation to SHELL.
* doc/emacs/misc.texi (Interactive Shell): Move index entry for
SHELL environment variable from here, where it is not mentioned...
(Single Shell): ...to here, where it is discussed along with
shell-file-name.
* lisp/dired.el (dired-insert-directory): Use shell-file-name
instead of explicit-shell-file-name when a shell is implicitly
requested.
* lisp/obsolete/terminal.el (explicit-shell-file-name):
* lisp/term.el (explicit-shell-file-name): Remove duplicate
defcustoms and load lisp/shell.el instead.
* lisp/shell.el (explicit-shell-file-name): Clarify docstring.
(shell): Simplify.
---
 doc/emacs/cmdargs.texi    |  3 ++-
 doc/emacs/misc.texi       |  2 +-
 lisp/dired.el             |  3 +--
 lisp/obsolete/terminal.el |  7 +------
 lisp/shell.el             | 24 ++++++++++++------------
 lisp/term.el              | 16 +++-------------
 6 files changed, 20 insertions(+), 35 deletions(-)

diff --git a/doc/emacs/cmdargs.texi b/doc/emacs/cmdargs.texi
index 9303b0b8dd..936facbf28 100644
--- a/doc/emacs/cmdargs.texi
+++ b/doc/emacs/cmdargs.texi
@@ -647,7 +647,8 @@ General Variables
 @item SHELL
 @vindex SHELL@r{, environment variable}
 The name of an interpreter used to parse and execute programs run from
-inside Emacs.
+inside Emacs.  This is used to initialize the variable
+@code{shell-file-name} (@pxref{Single Shell}).
 @item SMTPSERVER
 @vindex SMTPSERVER@r{, environment variable}
 The name of the outgoing mail server.  This is used to initialize the
diff --git a/doc/emacs/misc.texi b/doc/emacs/misc.texi
index d097f4ee7d..8e1b61815d 100644
--- a/doc/emacs/misc.texi
+++ b/doc/emacs/misc.texi
@@ -806,6 +806,7 @@ Single Shell
 @file{*Shell Command Output*} buffer.
 
 @vindex shell-file-name
+@cindex @env{SHELL} environment variable
   The above commands use the shell specified by the variable
 @code{shell-file-name}.  Its default value is determined by the
 @env{SHELL} environment variable when Emacs is started.  If the file
@@ -875,7 +876,6 @@ Interactive Shell
 @vindex explicit-shell-file-name
 @cindex environment variables for subshells
 @cindex @env{ESHELL} environment variable
-@cindex @env{SHELL} environment variable
   To specify the shell file name used by @kbd{M-x shell}, customize
 the variable @code{explicit-shell-file-name}.  If this is @code{nil}
 (the default), Emacs uses the environment variable @env{ESHELL} if it
diff --git a/lisp/dired.el b/lisp/dired.el
index 14bbb28db5..071597716c 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -1448,8 +1448,7 @@ dired-insert-directory
                     (script (format "ls %s %s" switches (cdr dir-wildcard)))
                     (remotep (file-remote-p dir))
                     (sh (or (and remotep "/bin/sh")
-                            (and (bound-and-true-p explicit-shell-file-name)
-                                 (executable-find explicit-shell-file-name))
+                            (executable-find shell-file-name)
                             (executable-find "sh")))
                     (switch (if remotep "-c" shell-command-switch)))
                (unless
diff --git a/lisp/obsolete/terminal.el b/lisp/obsolete/terminal.el
index 6ee53af648..3d73c030c3 100644
--- a/lisp/obsolete/terminal.el
+++ b/lisp/obsolete/terminal.el
@@ -44,6 +44,7 @@
 ;;>>   more-processing enabled.
 
 (require 'ehelp)
+(require 'shell)
 
 (defgroup terminal nil
   "Terminal emulator for Emacs."
@@ -1056,12 +1057,6 @@ te-stty-string
 ;; This used to have `new' in it, but that loses outside BSD
 ;; and it's apparently not needed in BSD.
 
-(defcustom explicit-shell-file-name nil
-  "If non-nil, is file name to use for explicitly requested inferior shell."
-  :type '(choice (const :tag "None" nil)
-		 file)
-  :group 'terminal)
-
 ;;;###autoload
 (defun terminal-emulator (buffer program args &optional width height)
   "Under a display-terminal emulator in BUFFER, run PROGRAM on arguments ARGS.
diff --git a/lisp/shell.el b/lisp/shell.el
index 1e2679f723..6688f9dcd9 100644
--- a/lisp/shell.el
+++ b/lisp/shell.el
@@ -268,10 +268,11 @@ shell-dirtrack-verbose
   :group 'shell-directories)
 
 (defcustom explicit-shell-file-name nil
-  "If non-nil, is file name to use for explicitly requested inferior shell.
-When nil, such interactive shell sessions fallback to using either
-the shell specified in $ESHELL or in `shell-file-name'."
-  :type '(choice (const :tag "None" nil) file)
+  "If non-nil, the file name to use for explicitly requested inferior shells.
+When nil, such interactive shell sessions fall back to using the
+shell specified in either the environment variable \"ESHELL\" or
+`shell-file-name'."
+  :type '(choice (const :tag "Default" nil) file)
   :group 'shell)
 
 ;; Note: There are no explicit references to the variable `explicit-csh-args'.
@@ -742,16 +743,15 @@ shell
 
   (with-connection-local-variables
    ;; On remote hosts, the local `shell-file-name' might be useless.
-   (when (file-remote-p default-directory)
-     (if (and (called-interactively-p 'any)
+   (when (and (file-remote-p default-directory)
+              (called-interactively-p 'any)
               (null explicit-shell-file-name)
               (null (getenv "ESHELL")))
-         (set (make-local-variable 'explicit-shell-file-name)
-              (file-local-name
-	       (expand-file-name
-                (read-file-name
-                 "Remote shell path: " default-directory shell-file-name
-                 t shell-file-name))))))
+     (setq-local explicit-shell-file-name
+                 (file-local-name
+                  (expand-file-name
+                   (read-file-name "Remote shell path: " default-directory
+                                   shell-file-name t shell-file-name)))))
 
    ;; Rain or shine, BUFFER must be current by now.
    (unless (comint-check-proc buffer)
diff --git a/lisp/term.el b/lisp/term.el
index b990c83cfc..373d23d2e2 100644
--- a/lisp/term.el
+++ b/lisp/term.el
@@ -302,15 +302,10 @@ term-protocol-version
 
 (eval-when-compile (require 'ange-ftp))
 (eval-when-compile (require 'cl-lib))
-(require 'ring)
-(require 'ehelp)
 (require 'comint) ; Password regexp.
-
-(declare-function ring-empty-p "ring" (ring))
-(declare-function ring-ref "ring" (ring index))
-(declare-function ring-insert-at-beginning "ring" (ring item))
-(declare-function ring-length "ring" (ring))
-(declare-function ring-insert "ring" (ring item))
+(require 'ehelp)
+(require 'ring)
+(require 'shell)
 
 (defgroup term nil
   "General command interpreter in a window."
@@ -394,11 +389,6 @@ term-pager-old-filter
 (defvar-local term-line-mode-buffer-read-only nil
   "The `buffer-read-only' state to set in `term-line-mode'.")
 
-(defcustom explicit-shell-file-name nil
-  "If non-nil, is file name to use for explicitly requested inferior shell."
-  :type '(choice (const nil) file)
-  :group 'term)
-
 (defvar term-prompt-regexp "^"
   "Regexp to recognize prompts in the inferior process.
 Defaults to \"^\", the null string at BOL.
-- 
2.25.1


[-- Attachment #2: Type: text/plain, Size: 956 bytes --]


This is a followup to the following emacs-devel thread:
https://lists.gnu.org/archive/html/emacs-devel/2020-04/msg00880.html

The user option explicit-shell-file-name has been defined in three
different places and under three different custom groups since at least
as far back as 1997:

- lisp/shell.el
- lisp/term.el
- lisp/obsolete/terminal.el

I think there should be only a single definition, and lisp/shell.el was
suggested as the correct place in the aforementioned thread.

I attach a patch which does this and also cleans up a couple of uses of
explicit-shell-file-name.  Tino, was there a particular reason for using
explicit-shell-file-name instead of shell-file-name in
dired-insert-directory[1], or is it safe to swap them?

[1]: Don't assume /bin/sh as the 'sh' location in the local host
e82c4f56e6 2017-08-02 16:50:44 +0900
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=e82c4f56e6f9a6bce4098698b17fa45dcc5bbd25

Thanks,

-- 
Basil

bug#40679: 28.0.50; Multiple definitions of explicit-shell-file-name

[Basil L. Contovounesios]

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> Tino, was there a particular reason for using explicit-shell-file-name
> instead of shell-file-name in dired-insert-directory[1], or is it safe
> to swap them?
>
> [1]: Don't assume /bin/sh as the 'sh' location in the local host
> e82c4f56e6 2017-08-02 16:50:44 +0900
> https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=e82c4f56e6f9a6bce4098698b17fa45dcc5bbd25

Ping.

-- 
Basil

bug#40679: 28.0.50; Multiple definitions of explicit-shell-file-name

[Lars Ingebrigtsen]

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> "Basil L. Contovounesios" <contovob@tcd.ie> writes:
>
>> Tino, was there a particular reason for using explicit-shell-file-name
>> instead of shell-file-name in dired-insert-directory[1], or is it safe
>> to swap them?
>>
>> [1]: Don't assume /bin/sh as the 'sh' location in the local host
>> e82c4f56e6 2017-08-02 16:50:44 +0900
>> https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=e82c4f56e6f9a6bce4098698b17fa45dcc5bbd25
>
> Ping.

Tino, did you find time to have a look at Basil's patch?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#40679: 28.0.50; Multiple definitions of explicit-shell-file-name

[Basil L. Contovounesios]

tags 40679 fixed
close 40679 28.1
quit

No comments/objections in 8 months, so I've now pushed this to master.

Define explicit-shell-file-name only in shell.el
6ecec60966 2020-12-03 17:25:04 +0000
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=6ecec6096697729491ba141e7650ad69de5f034e

-- 
Basil