Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider adding a WT_PROFILE env var pointing to settings file #4566

Open
oising opened this issue Feb 13, 2020 · 21 comments
Open

Consider adding a WT_PROFILE env var pointing to settings file #4566

oising opened this issue Feb 13, 2020 · 21 comments
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Help Wanted We encourage anyone to jump in on these. Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Terminal The new Windows Terminal.
Milestone

Comments

@oising
Copy link
Collaborator

oising commented Feb 13, 2020

If the settings UI is not planned until after 1.0, then I strongly recommend adding an environment variable with the path to the json settings file, so it can be edited with the editor of choice (vi, notepad, nano, emacs, whatever) from within the shell of choice without burning brain calories trying to find it buried in AppData. Also: Relying on the GUI menu is, well, unreliable, due to there being no guarantee of a valid file association for json files.

We've already got WT_SESSION env var -- something like WT_PROFILE would be useful.

Then from within cmd or powershell, I can easily edit settings with my preferred editor, or interactively using $settings = gc $env:WT_PROFILE | convertto-json and flipping settings on the object model directly before saving it back out.

@oising oising added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Feb 13, 2020
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Feb 13, 2020
@oising oising added Area-Settings Issues related to settings and customizability, for console or terminal Help Wanted We encourage anyone to jump in on these. Product-Terminal The new Windows Terminal. labels Feb 13, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Feb 13, 2020
@akulbe
Copy link

akulbe commented Feb 13, 2020

@oising Are you able to access settings at all? I no longer find a profiles.json file, and Ctrl-, no longer even pulls one up.

@oising
Copy link
Collaborator Author

oising commented Feb 13, 2020

@akulbe I think now there is only a defaults.json and you may create your own profiles.json file as a differential to this (things in your file will override the defaults).

There's a readonly defaults.json alongside the executable, and a user-editable one for overrides in your local app data folder.

Run the following one-liner in powershell to open the defaults.json file:

notepad (join-path ((dir (gcm wt.exe).source).target | split-path) defaults.json)

The profiles.json file is in %LOCALAPPDATA%\packages\Microsoft.WindowsTerminal_8wekyb3d8bbwe\LocalState\

@JustinGrote
Copy link

Conflicts with #3589 request, maybe have the variable be WT_CONFIGFILE?

Currently the location is pretty stable and discoverable, this change would only be relevant if you could also customize the path.

@Jaykul
Copy link
Contributor

Jaykul commented Feb 20, 2020

Does anyone actually have a use for WT_Session? Is it just so you can distinguish different terminal connections to the same (e.g. ssh) session? To me, the WT_Profile in #3589 would be really useful. This one, pointing to the config file (confusingly named profile.json), would also be useful but it feels less useful -- isn't there only one location (plus one for dev builds)?

@oising
Copy link
Collaborator Author

oising commented Feb 20, 2020

Conflicts with #3589 request, maybe have the variable be WT_CONFIGFILE?

Currently the location is pretty stable and discoverable, this change would only be relevant if you could also customize the path.

Well there are actually two config files, in different locations - I propose:

env var WT_PROFILES (user editable) for:

%LOCALAPPDATA%\packages\Microsoft.WindowsTerminal_8wekyb3d8bbwe\LocalState\profiles.json

env var WT_DEFAULTS (read only, for reference) for:

%PROGRAMFILES%\WindowsApps\Microsoft.WindowsTerminal_0.9.433.0_x64__8wekyb3d8bbwe\defaults.json

While they are "discoverable" - for certain definitions of discoverability - I think they are unwieldy enough to benefit from an environment variable.

@Jaykul I really like your idea for the active profile ID. What do you suggest for a name? I think WT_PROFILE is too close to WT_PROFILES (which matches the name of the json file, likewise for WT_DEFAULTS)

@DHowett-MSFT DHowett-MSFT removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Feb 22, 2020
@DHowett-MSFT DHowett-MSFT added this to the Terminal v1.x milestone Feb 22, 2020
@chwarr
Copy link
Member

chwarr commented Mar 8, 2020

On February 20, 2020, @oising said:

env var WT_DEFAULTS (read only, for reference)

Can you expand on what you mean by a read-only environment variable? Windows, doesn't have anything like read-only environment variables AFAIK. A process is free to modify its own environment or to start children with custom environments.

My expectation of a variable like WT_PROFILE would be that I could use it to change where Windows Terminal reads user settings from. I would use it to move this to my "dotfiles" repository where I keep various application settings so I can version and sync them across machines.

Based on how other, similar software works, I think it would make sense if Windows Terminal only read this environment variable at process start up and then monitored the path for changes. It would also be reasonable for Windows Terminal to detect changes to the environment "template" by handling WM_SETTINGCHANGE notifications. (Though, there are some subtle details to consider about when changes are applied. Lots of thoughts in #1125, "Feature Request: Terminal should listen for the WM_SETTINGCHANGE for environment variable updates".)

@oising
Copy link
Collaborator Author

oising commented Mar 9, 2020

@chwarr The WT_DEFAULTS environment variable will point to the readonly file defaults.json. It's not a readonly env var.

As for your thoughts on WT_PROFILE, well, sure, that makes sense but I'm not sure that would make it into my PR. I'd imagine that may be something that gets haggled about for post v1.0.

@chwarr
Copy link
Member

chwarr commented Mar 9, 2020

Ah, @oising was talking about the mutability of the file, not the variable.

There's a related issue in this space, "Add support for roaming settings.json or storing it elsewhere".

@oising
Copy link
Collaborator Author

oising commented Mar 9, 2020

I've added WT_DEFAULTS and WT_PROFILES which reflect defaults.json and profiles.json respectively. They also will translate correctly under WSL.

@DHowett-MSFT
Copy link
Contributor

So, here's the thing. I adore the work you did in #4852, and I'd like to land some of it as it improves environment handling in Terminal across the board.

What I'm most worried about here is introducing what amounts to a public API this close to v1.

As I reviewed it, I found myself asking some questions:

  1. Does this expose anything that can't otherwise be determined?
    • sorta, it's annoying to detect it but the location is broadly stable. if we ever solve profile includes, the location gets more complex.
  2. Are we agreeing to ship this forever, or somehow communicate that we're making a breaking change later?
  3. If we produce a WT_SETTINGS environment variable today, why don't we consume one?
    • If we plan to consume one, we need to matrix-in to our support flow that people might have globally overridden their settings locations
    • If we plan to consume it, we're somewhat unique among applications of this type
    • Spawning WT from WT logically "rolls-forward" the settings used by the parent. Cool! But...
    • Which config file wins when you run wt split-pane and we finally land the ability to split a pane in the existing terminal? The one in WT_SETTINGS or the one in the destination terminal instance? Both? Neither? Do we get an error?

I'm wary of doing this for 1.0 because we don't have time to answer those questions. That doesn't mean we're not interested in the change overall. 😄

To turn the discussion around a bit, though... copying a comment from @Jaykul on the pull request:

And since the VT color queries were also never finished, there's no way to discover what crazy colors someone's using in their terminal

Why would someone want this? Honest question. I realize it's the conjunction of PROFILE_ID and SETTINGS, so I don't have a good place to discuss it, but:

If an application has specific color requirements, it shouldn't need to parse our settings files--both of them--then layer them with the exact same logic we do to figure out what colors the user's going to see. It should just set the palette it wants and be done with it, or use the 24-bit color escapes.

We've been burned so many times, in so many ways, when we've offered public APIs or "detectables" to determine console/terminal state. People take dependencies on reserved flag values not being used (oops), and that consoles have an HWND (thanks SSH), and that the high 8 bits in a cell are unused when you're using the ANSI versions of the APIs (thanks Pascal).


I think it's worth getting in #4852 without the path versions and only including the WT_PROFILE_ID var (closing #3589 in the process.) What do you think?

@DHowett-MSFT
Copy link
Contributor

(Also, thank you for bearing with us as we release-engineer our first v1 of something: this is painful in part because I'm not communicating very well about it. I'm sorry.)

@oising
Copy link
Collaborator Author

oising commented Apr 14, 2020

I think it's worth getting in #4852 without the path versions and only including the WT_PROFILE_ID var (closing #3589 in the process.) What do you think?

@DHowett-MSFT If I'm reading this correctly, then you may ship this in v1.0 if I can drop the WT_DEFAULTS and WT_SETTINGS env vars? I can do that, of course.

Update: It's done. Just remove two lines, and edited one.

@oising
Copy link
Collaborator Author

oising commented Apr 14, 2020

(Also, thank you for bearing with us as we release-engineer our first v1 of something: this is painful in part because I'm not communicating very well about it. I'm sorry.)

It's quite alright, Dustin. Thank you for your very thoughtful commentary.

@ghost ghost removed the In-PR This issue has a related PR label Apr 17, 2020
ghost pushed a commit that referenced this issue Apr 17, 2020
This commit adds a `WT_PROFILE_ID` environment variable, which contains
the guid of the active profile.

It also teaches ConptyConnection to take an environment map on creation.

We had to do a little manual jiggery with the WSLENV environment
variable as passed by the creator.

* [x] CLA signed
* [ ] Tests added/passed
* [ ] Requires documentation to be updated
* [x] I've discussed this with core contributors already.

Ran terminal, validated vars and translated paths under windows and WSL. 

References #4566 (this PR originally introduced WT_SETTINGS/DEFAULTS)
Closes #3589
@DHowett DHowett changed the title Consider adding a WT_PROFILE env var pointing to settings file for 1.0 release Consider adding a WT_PROFILE env var pointing to settings file Jun 26, 2020
@sassdawe
Copy link

Just to complicate things a little bit.

What about WT_SettingsPath which could give back the path of the same file which gets opened by hitting ctrl+, ?

The path in the name would really limit the scope, and make most of the questions irrelevant. I realize that this is not the end solution but sounds like a great MVP.

If you only give back the currently used, and editable path, it will be up to the users to decide what do they want to do with it. This approach sounds supportable in the future, because if you change the path - to support roaming or whatever - or the file name, etc., WT must always know where the active settings are, because it had to read the content.


I'm editing the content of the settings.json in PSWinTerminal but figuring out if the running terminal is the GA version or it Preview isn't pretty. I'd to look for the parent process' path and look for the word preview in the folder name. With PowerShell this is easy, but with Windows PowerShell I had to use Get-CimInstance.

It would be really nice if I can simply just use WT_SettingsPath to get the path of the settings file.


What do you think?

@zadjii-msft
Copy link
Member

From @arqtiq in #7773

Numerous times I tried to automate some settings / profiles edits on the Terminal settings themselves, and I usually lack informations about the current tab environment :

  • What is the current loaded profile / scheme ?

  • Is the host the preview package ?

I'm sure there are also more info we could export to env vars based on people needs.
This would be veery useful to manage the terminal settings / themes / profiles directly from the command line !

When opening a new tab, set:

  • $WT_CURRENT_PROFILE (Microsoft PowerShell, CMD, ...)

  • $WT_CURRENT_SCHEME

  • $WT_IS_PREVIEW (1 / 0, if the current terminal running package is the preview one)

@bradrice
Copy link

How do you find the editable defaults.json file if I run notepad (join-path ((dir (gcm wt.exe).source).target | split-path) defaults.json)
it pulls up the one that I can't edit. If I hit Alt-Settings, it opens a defaults.json file, but I can't save it.

@zadjii-msft
Copy link
Member

@bradrice Yep, that's correct. You shouldn't be editing defaults.json. That file technically isn't even read by the Terminal - it's more of a reference of what the Terminal's hard-coded defaults are.

@bradrice
Copy link

The problem I have with Powershell and Windows Terminal is using a theme for vim. I would love to ise gruvbox. For some reason my vim has these red backgrounds with red type and I can't read it, when I'm using code completion that is flagging me of an error in my typescript. Is there a way to make a powershell terminal behave more like a linix or mac terminal with regard to respecting a vim colorscheme?

@zadjii-msft
Copy link
Member

You might be better off filing a new issue rather than bumping this old one. Probably best to include if you're using vim.exe or WSL vim, maybe a link to the vim scheme you're trying to use, and include your settings.json

@sassdawe
Copy link

it pulls up the one that I can't edit. If I hit Alt-Settings, it opens a defaults.json file, but I can't save it.

@bradrice you don't need the ALT button, a pure click on the Settings will open the setting.json, which is editable.

@bradrice
Copy link

You're right @zadjii-msft , I should keep issues separate. Thanks @sassdawe that helped. I didn't see the link to the json file when I hit settings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Help Wanted We encourage anyone to jump in on these. Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Terminal The new Windows Terminal.
Projects
None yet
Development

No branches or pull requests

10 participants