(Feature Request) Terminal Environment setting (like in VScode)

[Merith-TK]

Description of the new feature/enhancement

Arguments to the program. for example the ability to specify which mingw one would want to load, mingw64, mingw32, or msys2. like we can in VSCode

Proposed technical implementation details (optional)

This is accomplished in VSCode via this directive in settings.json

    "terminal.integrated.env.windows":
    {
        "MSYSTEM": "MINGW64",
        "CHERE_INVOKING":"1"
    },

My thougt process allowed something like

"commandlineEnv" : 
{
    "MSYSTEM" : "MINGW64"
},

In the settins.json file example. just like VSCode

Please do note, i know little to nothing about this language that this program is written in, so i don't know how difficult it would be to implement.

[zadjii-msft]

This is actually won't be terribly difficult to implement IMO. In ConhostConnection, we're already creating a EnvironmentVariableMapW with a WT_SESSION env var.

It wouldn't be impossible to also have another map of string->string key-value pairs that we also pass to the connection like this.

[Merith-TK]

Right now my current workaround is to use this .cmd file and ahve the terminal run that

set "MSYSTEM=MINGW64" 
C:/msys/usr/bin/bash.exe --login -i

[Merith-TK]

any progress? or is this put in the background with priority on bug fixes/better features? If so, i would understand.

[DHowett-MSFT]

Right now, we're focusing on bug fixes and features. You can check out our plans for October in the Terminal-1910 milestone, if you're interested. Right now, this one's sitting on the backlog ("not v1.0, but if we get some spare time we'll look at the features we've put off.")

That's not to say we don't care about this, of course. We'd be more than happy to review a pull request that adds support for this, so we've marked it Help-Wanted.

[DHowett-MSFT]

Thanks for asking!

[Merith-TK]

Ah! no worries! Sadly I know NOTHING about the lang this is being written in, so I cant be of any help here! I totally understand that! Functionality over features! Make the Program work, and THEN you add cool features (if that makes any sense)

[zadjii-msft]

Moving relevant discussion from #6604:

https://github.com/microsoft/terminal/issues/6604

Following DHowett suggestion in #6585.

Description of the new feature/enhancement

For tools like MinGW you need certain environment variables to be set that define their behavior. It would be convenient to have these settings in the profile.

I suggest to add a property to the profile description in settings.json to set environment variables. For example

{
    "environment" : {
        "PATH": "c:/doomsday/emergency_commands/bin;${env:PATH}",
        "EMRGENCY_LEVEL_HIGH": "1"
    }
}

https://github.com/microsoft/terminal/issues/6604#issuecomment-728808569

If you have

{
    "environment" : {
        "HOMEDRIVE": "C:",
        "HOME": "${env:HOMEDRIVE}${env:HOMEPATH}",
        "HOMEPATH": "\\Users\\example"
    }
}

then should HOME use the original or new value of HOMEPATH? Should that depend on the lexical order of properties in the JSON object?

It might be easiest to implement this so that ${env:VAR} always means the original value, and the lexical order does not matter. If it becomes possible to define environment variables both globally and in each profile, then ${env:VAR} in a profile could refer to the value defined in the global environment object, and ${env:VAR} in the global environment could refer to the value received from Windows.

The syntax leaves room for adding ${new-env:VAR} with topological sorting later if needed.

[UndarkAido]

Is there any workaround for this? I'm trying to set up an SSH session with X forwarding. The environment variables get set weirdly when I do "commandline": "cmd /c \"set TMPDIR=C:\\Temp && set XAuthLocation=C:\\Progra~1\\VcXsrv\\xauth.exe && set DISPLAY=localhost:0 && ssh -X [email protected]\""

Edit: Remove the spaces before the &&s: "commandline": "cmd /c \"set TMPDIR=C:\\Temp&& set XAuthLocation=C:\\Progra~1\\VcXsrv\\xauth.exe&& set DISPLAY=localhost:0&& ssh -X [email protected]\""

[christapley]

Hello @zadjii-msft, I would like to implement this feature. I'm still getting up to speed with your processes. Do you have any documentation on how you run your tests (specifically LocalTests_SettingsModel)?

[zadjii-msft]

That is a good question! I'm fairly certain that everyone on the team has a slightly different process but here's how I do it:

1. Open a new cmd.exe tab/pane
2. navigate to the directory the repo is in
3. run tools\razzle.cmd
4. then navigate to wherever the project you want to build is (in this case, cd src\cascadia\LocalTests_SettingsModel)
5. bx to build the project in this dicrectory
6. Then I use runut /name:*TestNameHere* to run the tests. The /name: arg lets you select tests to run based off the fully-qualified name. So if you wanted to run all the local tests for TSM, you could run runut /name:*SettingsModelLocalTests*. Or you could run a specific test with something like runut /name:*CanLayerColorScheme*. It's fairly flexible.

You only need to do steps 1-3 once. Once you have the "razzle" window set up, you can just keep switching to new directories and building them, you don't need to start a new window each time.

Lemme know if you need any other help. All the "Local" tests are a little weird, so I wouldn't be surprised if you ran into other issues ☺️

[christapley]

Thank you @zadjii-msft, I got the tests running.

[christapley]

I have a couple of questions.

1. Since we can set environment variables in a parent Profile, should it be the responsibility of a Profile to merge all the environment variables set in its parent? I believe we would need an API for this separate to the EnvironmentVariables getter and setter as those will need to be used for the settings editor to set the environment variables for a specific profile.

namespace Microsoft.Terminal.Settings.Model
{
    [default_interface] runtimeclass Profile {
        ...

        Boolean HasEnvironmentVariables();
        void ClearEnvironmentVariables();
        Windows.Foundation.Collections.StringMap EnvironmentVariables;
        Windows.Foundation.Collections.StringMap GetMergedAndResolvedEnvironmentVariables();
    }
}

Profile::GetMergedAndResolvedEnvironmentVariables will iterate through its parents building a StringMap of environment variables (duplicates are overwritten by the child nodes) and then resolve all variables (i.e. ${env:KEY}) before returning the resolved environment variables StringMap.

TerminalSettings would use this API to populate its EnvironmentVariables property as apposed to Profile::EnvironmentVariables

2. For this ticket do you want the environment variables editable in the Microsoft.Terminal.Settings.Editor?

[zadjii-msft]

Okay I think I'm gonna need a bit more coffee to figure out the first half of your question 😆 I'll tag in @carlos-zamora since he's been working in that area so heavily. As far as the settings UI is concerned, you can probably just forego that for now. We can come back to that in the future. Especially since this might be a bit trickier of a UI element. I don't think we need to have every setting in the UI quite yet. It's still a work in progress.

[carlos-zamora]

1. Should it be the responsibility of a Profile to merge all the environment variables set in its parent?

100% yes! I think the design may have to change a bit though? What about something like this:

		// these work like a normal setting tbh
        Boolean HasEnvironmentVariables();
        void ClearEnvironmentVariables();
		String EnvironmentVariables { get; set; };

		// this is a supplemental/meta setting that is exposed to the JSON and Settings UI
        Boolean HasShouldMergeEnvironmentVariables();
        void ClearShouldMergeEnvironmentVariables();
		Boolean ShouldMergeEnvironmentVariables { get; set; };

		// this works like startingDirectory and backgroundImage. We do a bit of processing before handing it to the caller.
		// if `ShouldMergeEnvironmentVariables` is false, we should only return a conversion of the local `EnvironmentVariables` into the string map. Otherwise, we take our parent's, merge it with ours, then convert that into a string map that we return.
        Windows.Foundation.Collections.StringMap EvaluatedEnvironmentVariables { get; };

NOTE: my reasoning for introducing another setting that handles merging the EVs is that it's more explicit to the user. I played around with the idea of EvaluatedEnvironmentVariables(bool shouldMergeWithParent), but then realized that it doesn't make sense to expose that option to the caller; the settings model should just do that work for you and know what you want.

2. For this ticket do you want the environment variables editable in the Microsoft.Terminal.Settings.Editor?

Don't worry about this yet. Let's get a design we're happy with first. This is definitely going to be a case where we'll have to do a separate PR for the Settings UI than the actual feature (similar to startup actions). That way, we get a really polished cool UI 😊

3. (you didn't ask this, but it is relevant) How should we expose this in the JSON?

I think we should go with the design I mentioned above. Maybe change the name of the setting but that's not really what I want to focus in this proposal. Focus on the design. @cinnamon-msft @DHowett @zadjii-msft thoughts?

[DHowett]

I'm staunchly against a new setting for "merging environment variables", because it opens us up immediately to questions about how system environment variables are treated. We always merge those in, after all, on every launch. We're never going to stop (because processes cannot handle empty environment blocks.)

Additionally, I don't expect users to ever have to use one setting to tell us to inherit another.

[zadjii-msft]

I'm throwing @DHowett on the "assigned-to" line, but he's not working on this, he's just got a plan for how to deal with this.

[christapley]

Next question @zadjii-msft and @DHowett. If there are circular or self references in the environment variables it is currently throwing during evaluation. This just crashes the process when evaluating the environment variables in TerminalSettings. However, I can't do it on LayerJson because the parents won't have been inserted yet.

Should there be a profile validation step after json has been loaded and parents set? Or should I handle the exception in TerminalSettings and show a dialog from there. I think the latter will be more difficult.

[zadjii-msft]

@christapley So there's already CascadiaSettings::_ValidateSettings that might be the perfect time to do this validation. That's called once all the settings are loaded and layered, to ensure that there are sensible values. We already do a bunch of other checks there, after loading all the settings. In each of the different parts of that validation, if there's something wrong, then you can append SettingsLoadWarnings to the list of _warnings, and we'll automatically display a little message when the settings are loaded. That's where I'd do the evaluation of the env vars.

[christapley]

I could just throw from CascadiaSettings::_ValidateSettings that way a more fine grained errror text is picked up However, it won't be internationalised. I presume internationalisation is what you want so I will add another warning to the enum with an accompanying message.

[jacob-pro]

@Merith-TK A simple work around I am using for MSYS is to use env.exe to set environment variables like

C:\msys64\usr\bin\env.exe MSYSTEM=MINGW64 CHERE_INVOKING=1 /usr/bin/bash.exe --login

(https://github.com/msys2/msys2.github.io/issues/215)

[volviq]

Is this features going to be implemented anytime soon?