Always inherit environment when launching external terminal

[apognu]

This was discussed on the closed issue #1317 (from this comment).

#1318 was merged in August and propagated environment to the launched external terminal, but it seemed to propagate the environment VIM was started with if and only if an environment was set somewhere in the configuration and passed to the launch_external_terminal function.

This had the side effect, under some configuration, that the launched external terminal have an empty environment.

This pull request changed the logic when merging VIM environment and passed environment to always include the environment VIM was started with.

--

I am quite unable to determine if this could have side effects or if the fact that the parent environment not being inherited by the external terminal is working as intended, so do not hesitate to tell me if that is the case, for some reason.

[mfussenegger]

I think this was just an oversight because spawn inherits the parent's env by default if it isn't provided - initializing env_formatted to {} prevents that.

Could you try with:

diff --git a/lua/dap/session.lua b/lua/dap/session.lua
index 2a941ea..4deaa0d 100644
--- a/lua/dap/session.lua
+++ b/lua/dap/session.lua
@@ -116,15 +116,18 @@ local function launch_external_terminal(env, terminal, args)
   local handle
   local pid_or_err
   local full_args = {}
   vim.list_extend(full_args, terminal.args or {})
   vim.list_extend(full_args, args)
-  local env_formatted = {}
-  -- Copy environment, prefer vars set by client
-  for k, v in pairs(env and vim.tbl_extend('keep', env, vim.fn.environ()) or {}) do
-    if k:find "^[^=]*$" then -- correct variable?
-      env_formatted[#env_formatted+1] = k.."="..tostring(v)
+  local env_formatted = nil
+  if env then
+    env_formatted = {}
+    -- Copy environment, prefer vars set by client
+    for k, v in pairs(vim.tbl_extend('keep', env, vim.fn.environ())) do
+      if k:find "^[^=]*$" then -- correct variable?
+        env_formatted[#env_formatted+1] = k.."="..tostring(v)
+      end
     end
   end
   local opts = {
     args = full_args,
     detached = true,

This should have the same effect but without copying the full environment.

[apognu]

Indeed, the provided patch has the same effect, the environment is properly propagated to the external terminal and, at least for my usage, fixes the issue!

Do you wish me to edit this PR to reflect this or would you prefer I close it?

[mfussenegger]

Do you wish me to edit this PR to reflect this or would you prefer I close it?

If you don't mind, updating the PR to reflect the proposed change would be great.

[apognu]

Done.

[mfussenegger]

thanks