From 6c7aa8159f8072ff3afdcb7dfa6852665f7ab58d Mon Sep 17 00:00:00 2001
From: Wez Furlong <wez@wezfurlong.org>
Date: Sun, 20 Aug 2023 10:38:24 -0700
Subject: [PATCH] SHELL cleanup

* Allow set_environment_variables to set the SHELL.
  Previously, we'd always set it to the shell from the password
  database.  Now we do that by default, but if set_environment_variables
  has been used, we'll preserve that value for the environment
  of the to-be-spawned process

* Clarify the docs:
  * Remove the confusing version dependent sections that started
    with old behavior and rewrite in terms of the behavior that
    has been true for the past year
  * I've heard from a few people that they tried to change COMSPEC
    on Windows and pain ensued.  Try to nudge them to read the
    next paragraph that tells them what they are actually supposed
    to change.

refs: https://github.com/wez/wezterm/issues/4098
refs: https://github.com/wez/wezterm/issues/4168
closes: https://github.com/wez/wezterm/issues/3816
refs: https://github.com/wez/wezterm/issues/3317
---
 docs/config/launch.md | 38 +++++++++++++++++++++-----------------
 pty/src/cmdbuilder.rs | 20 ++++++++++++--------
 2 files changed, 33 insertions(+), 25 deletions(-)

diff --git a/docs/config/launch.md b/docs/config/launch.md
index a029a6b2d41..3e5d66df450 100644
--- a/docs/config/launch.md
+++ b/docs/config/launch.md
@@ -6,28 +6,31 @@ Your shell is determined by the following rules:
 
 === "On Posix Systems"
 
-    1. The value of the `$SHELL` environment variable is used if it is set
-    2. Otherwise, it will resolve your current uid and try to look up your
-    shell from the password database.
+    The shell configured for the current user in the password database will be
+    used.  The value of the `$SHELL` environment variable is **_deliberately
+    ignored_** in order for wezterm to continue to be functional without
+    restarting after the user changes their shell.
 
-    `wezterm` will spawn the shell and pass `-l` as an argument to request
-    a login shell.  A login shell generally loads additional startup files
-    and sets up more environment than a non-login shell.
+    wezterm will set the `$SHELL` environment variable to the shell that it
+    resolved from the password database. If you want to control the value of
+    `$SHELL` in your spawned processes, use
+    [set_environment_variables](lua/config/set_environment_variables.md) to
+    define the value you prefer.
 
-    {{since('20210502-154244-3f7122cb', inline=True)}}: instead of passing `-l` to the shell, wezterm
-    will spawn the shell as `-$SHELL` to invoke it as a login shell.
+    The shell will be spawned as `-<SHELL>` (with a `-` prefixed to its *ARGV0*)
+    to invoke it as a login shell.  A login shell generally loads additional
+    startup files and sets up more environment than a non-login shell.
 
-    Note: if you have recently changed your shell using `chsh` and you
-    have `$SHELL` set in the environment, you will need to sign out and
-    sign back in again for the environment to pick up your new `$SHELL`
-    value.
-
-    {{since('20220903-194523-3bb1ed61', inline=True)}}: wezterm will now always resolve the shell via the
-    password database.
+    Older versions of wezterm (circa 2022 and earlier) used slightly
+    different logic to determine the default program and invoke it.
 
 === "On Windows Systems"
 
     1. The value of the `%COMSPEC%` environment variable is used if it is set.
+       **It is not recommended to change COMSPEC**, keep reading this
+       page of the documentation to learn how to make wezterm
+       run a different program.
+
     2. Otherwise, `cmd.exe`
 
 ## Changing the default program
@@ -97,8 +100,9 @@ used.
 
 ## Passing Environment variables to the spawned program
 
-The `set_environment_variables` configuration setting can be used to
-add environment variables to the environment of the spawned program.
+The [set_environment_variables](lua/config/set_environment_variables.md)
+configuration setting can be used to add environment variables to the
+environment of the spawned program.
 
 The behavior is to take the environment of the `wezterm` process
 and then set the specified variables for the spawned process.
diff --git a/pty/src/cmdbuilder.rs b/pty/src/cmdbuilder.rs
index 69531edf759..58eba26a870 100644
--- a/pty/src/cmdbuilder.rs
+++ b/pty/src/cmdbuilder.rs
@@ -85,14 +85,18 @@ fn get_base_env() -> BTreeMap<OsString, EnvEntry> {
 
     #[cfg(unix)]
     {
-        env.insert(
-            EnvEntry::map_key("SHELL".into()),
-            EnvEntry {
-                is_from_base_env: true,
-                preferred_key: "SHELL".into(),
-                value: get_shell().into(),
-            },
-        );
+        let key = EnvEntry::map_key("SHELL".into());
+        // Only set the value of SHELL if it isn't already set
+        if !env.contains_key(&key) {
+            env.insert(
+                EnvEntry::map_key("SHELL".into()),
+                EnvEntry {
+                    is_from_base_env: true,
+                    preferred_key: "SHELL".into(),
+                    value: get_shell().into(),
+                },
+            );
+        }
     }
 
     #[cfg(windows)]