Repository: microsoft/terminal
File: src/cascadia/TerminalApp/TerminalPage.cpp (C++)

Code before:

            if constexpr (Feature_VtPassthroughMode::IsEnabled())
            {
                valueSet.Insert(L"passthroughMode", Windows::Foundation::PropertyValue::CreateBoolean(settings.VtPassthrough()));
            }

            connection.Initialize(valueSet);
        }

        else
        {
            // profile is guaranteed to exist here
            auto guidWString = Utils::GuidToString(profile.Guid());

            StringMap envMap{};
            if (_settings.GlobalSettings().ReloadEnvironmentVariables())
            {
                til::env environment;
                environment.regenerate();

                for (const auto& [key, value] : environment.as_map())
                {
                    envMap.Insert(key, value);
                }
            }
            envMap.Insert(L"WT_PROFILE_ID", guidWString);
            envMap.Insert(L"WSLENV", L"WT_PROFILE_ID");

            // Update the path to be relative to whatever our CWD is.
            //
            // Refer to the examples in
            // https://en.cppreference.com/w/cpp/filesystem/path/append
            //
            // We need to do this here, to ensure we tell the ConptyConnection
            // the correct starting path. If we're being invoked from another
            // terminal instance (e.g. wt -w 0 -d .), then we have switched our
            // CWD to the provided path. We should treat the StartingDirectory
            // as relative to the current CWD.
            //
            // The connection must be informed of the current CWD on
            // construction, because the connection might not spawn the child
            // process until later, on another thread, after we've already
            // restored the CWD to its original value.
            auto newWorkingDirectory{ settings.StartingDirectory() };
            if (newWorkingDirectory.size() == 0 || newWorkingDirectory.size() == 1 &&
                                                       !(newWorkingDirectory[0] == L'~' || newWorkingDirectory[0] == L'/'))
            { // We only want to resolve the new WD against the CWD if it doesn't look like a Linux path (see GH#592)
                auto cwdString{ wil::GetCurrentDirectoryW<std::wstring>() };
                std::filesystem::path cwd{ cwdString };
                cwd /= settings.StartingDirectory().c_str();
                newWorkingDirectory = winrt::hstring{ cwd.wstring() };

Review comment (refactor):
I love the simplicity of this, but I am a little bit concerned about what happens after we hand this environment block off.

We produce a new env block here, painstakingly inserting it into envMap... and then ConptyConnection uses `EnvironmentVariableMapW` to _generate a new map based on the current process environment_ and inserts the contents of `envMap` from here into it.

In brief... what we will get is a hybrid of Terminal's spawning environment and the current environment. As well, we will probably end up parsing the entire env twice.

Should we move this down to ConptyConnection (worm the boolean down in through the valueset for all I mind, ha) and make sure we do not call `UpdateEnvironmentMapW`?

Code after:
            if constexpr (Feature_VtPassthroughMode::IsEnabled())
            {
                valueSet.Insert(L"passthroughMode", Windows::Foundation::PropertyValue::CreateBoolean(settings.VtPassthrough()));
            }

            connection.Initialize(valueSet);
        }

        else
        {
            // profile is guaranteed to exist here
            auto guidWString = Utils::GuidToString(profile.Guid());

            StringMap envMap{};
            envMap.Insert(L"WT_PROFILE_ID", guidWString);
            envMap.Insert(L"WSLENV", L"WT_PROFILE_ID");

            // Update the path to be relative to whatever our CWD is.
            //
            // Refer to the examples in
            // https://en.cppreference.com/w/cpp/filesystem/path/append
            //
            // We need to do this here, to ensure we tell the ConptyConnection
            // the correct starting path. If we're being invoked from another
            // terminal instance (e.g. wt -w 0 -d .), then we have switched our
            // CWD to the provided path. We should treat the StartingDirectory
            // as relative to the current CWD.
            //
            // The connection must be informed of the current CWD on
            // construction, because the connection might not spawn the child
            // process until later, on another thread, after we've already
            // restored the CWD to its original value.
            auto newWorkingDirectory{ settings.StartingDirectory() };
            if (newWorkingDirectory.size() == 0 || newWorkingDirectory.size() == 1 &&
                                                       !(newWorkingDirectory[0] == L'~' || newWorkingDirectory[0] == L'/'))
            { // We only want to resolve the new WD against the CWD if it doesn't look like a Linux path (see GH#592)
                auto cwdString{ wil::GetCurrentDirectoryW<std::wstring>() };
                std::filesystem::path cwd{ cwdString };
                cwd /= settings.StartingDirectory().c_str();
                newWorkingDirectory = winrt::hstring{ cwd.wstring() };
            }

            auto conhostConn = TerminalConnection::ConptyConnection();
            auto valueSet = TerminalConnection::ConptyConnection::CreateSettings(settings.Commandline(),
                                                                                 newWorkingDirectory,
                                                                                 settings.StartingTitle(),
                                                                                 envMap.GetView(),
                                                                                 settings.InitialRows(),
                                                                                 settings.InitialCols(),
                                                                                 winrt::guid());