Terminal: Feature Request: Terminal should listen for the WM_SETTINGCHANGE for environment variable updates

[rkeithhill]

Summary of the new feature/enhancement

When I install an app that updates the path or I update the path manually, it isn't enough to kill an individual tab in the Terminal and start a new one. The new tab still inherits the old, unchanged environment variable values. This means I have to kill/restart the Terminal and lose all my tabs.

Proposed technical implementation details (optional)

Add a Windows message handler for WM_SETTINGCHANGE and update the Terminal process's environment block so that any new tabs get the updated environment variables.

[ysc3839]

Related discussion in ConEmu. https://github.com/Maximus5/ConEmu/issues/468

[EdinaLewis]

Opening a new tab should definitely inherit from the current system environment.

[zadjii-msft]

<This was a kind of unfocused train of thought, though I'm happy with the conclusion at the bottom. leaving my own notes so I can remember whenever I get back to this>

Huh, this might be a bit trickier than I thought. WM_SETTINGCHANGE doesn't tell you which variable changed, only that the "Environment" changed. If we call GetEnvironmentStrings, we're still going to just get our launch environment variables.

We could call CreateEnvironmentBlock with nullptr as the hToken to get a fresh system environment block, but that's probably wrong, since we want the user's environment block. That involves also calling LogonUser. That's presumably doable.

Though, if the user launched the Terminal from one process with some extra variables set, and _expected_ it to inherit variables from the parent process, then we probably shouldn't blow away the env vars for the first tab that's created. I suppose we could only do this refreshing of the environment block when we do get a WM_SETTINGCHANGE. Subsequent tabs would all use the user's default env block, not the one inherited from the parent. That's a little funky - it sorta creates scenarios that aren't totally expected, because _sometimes_ the parent's values would persist to subsequent tabs.

I guess the most unified solution would be to just use a fresh environment block for all connections created after startup. That would at least be consistent behavior. We'd probably want to wait for #4023 to merge first, and set some internal flag to use a fresh env block, only after all the startup actions are processed. Then we wouldn't even need to use WM_SETTINGCHANGE, we'd just assume that you wanted a fresh one.

[yitzhaks]

Opening a new tab should definitely inherit from the current system environment.

nit: I would suggest the current user environment. explorer.exe does this correctly when launching new processes, so I hope it's not too complicated.

[DHowett-MSFT]

Make sure when we reload this, we also reload settings. That'll probably fix the keyboard issue in 4735.

[KalleOlaviNiemitalo]

We could call CreateEnvironmentBlock with nullptr as the hToken to get a fresh system environment block, but that's probably wrong, since we want the user's environment block. That involves also calling LogonUser.

CreateEnvironmentBlock(&lpEnvironment, GetCurrentProcessToken(), FALSE) seems to work OK without LogonUser, although the documentation of CreateEnvironmentBlock does not quite allow this use.

[TBBle]

If this one is solved by loading a new environment block on each new tab creation, then we'll still need separately to handle WM_SETTINGCHANGE to rectify the currently-closed-as-duplicate-of-this #4735.

It's not something I'm volunteering to implement myself, but if at startup the CreateEnvironmentBlock state was captured but not applied, then on WM_SETTINGCHANGE for Environment, we could CreateEnvironmentBlock again, and apply a 3-way diff (resolving conflicts in favour of the existing value: those're user-overrides) to the environment used for new tabs. That way existing user changes and overrides could be maintained, as we'd only change things that had that value in the default env block. Potentially, certain env-vars could be handled as lists, e.g., that'd make PATH do nice things if you append to the end of it as an override, and _then_ install something that adds itself to the PATH. I assume there's some way an env-var works as a list, based on the Environment settings letting you treat some env-vars as a list.

Unless of course the user has overridden a value to be the _same_ as the default env-block, and expects that override to survive if the Environment settings are changed. Then you get a less-nice outcome.

Conversely, this might have the nice effect that if the user had overridden a value, then changes the Environment to match it, and _then_ changes the Environment again, their override is gone, and the second Environment value wins. Probably what they wanted, since they changed it after the override.

I am 100% open to not being informed that I have not used the word "nice" correctly here. ^_^

[KalleOlaviNiemitalo]

Perhaps wt could "just use a fresh environment block for all connections created after startup" as previously suggested, but let the command line or settings.json list some environment variables that need to be copied from the environment of the wt process instead. Deliberately running wt with environment variables that differ from the user's defaults is a niche scenario. I don't think a three-way diff should be implemented.

[DHowett]

The environment variables bit of this was addressed in #7243; I'm leaving this issue open for the other environment settings.