-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Theme-controlled color scheme switch #4066
Comments
I like the idea. It might make sense to instead of Alternatively, this feature could be implemented by making the color |
I like the object idea since it doesn't use any other new name and instead uses current one. I'll elaborate on this bit In vscode it was implemented as a 2 additional keys: Of course it doesn't concern terminal since it doesn't update its settings in runtime when Having additional keys also increases ambiguity of (what will be selected?)
Adding |
Unless someone builds a custom tuned color schema, this doesn't sound right to me. People's perception of light vs dark best colors might differ substantially (mine do), so IMHO making it a specific choice in the schema object makes sense. I'm unsure of how Windows Terminal config schema backward compatibility is treated, but my suggestions would then be: If it's OK to change the schema for an existing key: And if it's not really OK to change the key type, then I'd introduce a new keyword ( |
Any progress on this? |
Workaround: I have an AutoHotKey script that I run at sun-up and sun-down that changes the themes for programs that don't change automatically like GitKraken and KeyWeb. Windows Terminal thankfully automatically changes if you change its settings file, so this Powershell script does what I want: $from = $args[0]
$to = $args[1]
$settingsPath = $env:LocalAppData + "\Packages\Microsoft.WindowsTerminal_8wekyb3d8bbwe\LocalState\settings.json";
# https://stackoverflow.com/a/30893960
$content = [System.IO.File]::ReadAllText($settingsPath).Replace(
"`"colorScheme`": `"" + $from + "`",",
"`"colorScheme`": `"" + $to + "`",");
[System.IO.File]::WriteAllText($settingsPath, $content); It's blunt and fragile - you could probably improve it by using JQ or actually parsing the JSON. But I'm lazy and this simple string replace works. I recommend you backup your I have the following function in AHK to make running Powershell scripts easier: ; https://www.autohotkey.com/boards/viewtopic.php?p=224271#p224271
PowerShell(Script, WorkingDir := "", Options := "", Params := "-ExecutionPolicy Bypass") {
Run % "PowerShell.exe " . Params . " -Command &{" . Script . "}", % WorkingDir == "" ? A_ScriptDir : WorkingDir, % Options
} Finally, with the following, I can hit ^#!PgDn::
PowerShell("./wt-theme-switch.ps1 'One Half Light' 'Campbell'")
Return
^#!PgUp::
PowerShell("./wt-theme-switch.ps1 'Campbell' 'One Half Light'")
Return This assumes the PowerShell script is called |
I need to |
@dharmaturtle, awesome! Just created a Windows schedule based on your powershell, so the theme auto update a 7AM and 7PM https://gist.github.com/biutas/2a6132170f81319a282c7135abb3dce9 |
I had trouble getting the powershell script to work with Auto Dark Mode, so I wrote a new (slightly more powerful) script in node. For anyone interested here is the script: const { readFileSync, writeFileSync } = require("fs");
const { join } = require("path");
// color scheme configurations
// The dark scheme will be swapped with the light scheme and vice versa
const colorSchemes = [
{ dark: "Campbell", light: "Campbell Powershell" },
{ dark: "One Half Dark", light: "One Half Light" },
{ dark: "Solarized Dark", light: "Solarized Light" },
{ dark: "Tango Dark", light: "Tango Light" },
];
// parse argument to determine which mode to switch to
const modeArg = process.argv[2];
if (modeArg !== "-dark" && modeArg !== "-light") {
console.error(`Invalid Argument. Argument must be '-dark' or '-light'. Received '${modeArg}'`);
process.exit(-1);
}
const isDarkMode = modeArg === "-dark";
// read Windows Terminal config
const pathToWinTermConfig = join(
process.env.LocalAppData,
"/Packages/Microsoft.WindowsTerminal_8wekyb3d8bbwe/LocalState/settings.json"
);
const configFileContents = readFileSync(pathToWinTermConfig).toString("utf-8");
const config = JSON.parse(configFileContents);
// update Windows Terminal config
config.profiles.list.forEach((profile) => {
const color = colorSchemes.find((c) => profile.colorScheme === c[isDarkMode ? "light" : "dark"]);
if (color) profile.colorScheme = color[isDarkMode ? "dark" : "light"];
});
// write Windows Terminal config changes to file system
writeFileSync(pathToWinTermConfig, JSON.stringify(config, null, 2)); Below is the Auto Dark Mode script config. The Auto Dark Mode script file should be located in Note: I saved the node script in the root of my C drive and named it Enabled: true
Component:
TimeoutMillis: 10000
Scripts:
- Name: WindowsTerminal
Command: node
ArgsLight: [C:\toggle-win-term-color.js, -light]
ArgsDark: [C:\toggle-win-term-color.js, -dark]
AllowedSources: [Any] |
Since powershell can parse JSON I binge made a slightly more polished implementation of the ADM route. Running powershell -nologo -noprofile -executionpolicy bypass -command "(iwr `"https://gist.githubusercontent.com/Gravifer/6511126e6c174c3ab7c647be43735dcc/raw/windowsTerminal_themeToggler.ps1`").content | out-file themeToggler.tmp.ps1; .\themeToggler.tmp.ps1; remove-item themeToggler.tmp.ps1" anywhere will have the script initialized (which can be put into ADM's
Update: Now using Powershell 5.1 is OK.
|
Just to help clarify: a spec was accepted in #12613. We probably won't have time to get to this one soon, but I don't think it'd be a terribly difficult change to make. I'd be happy to give pointers if anyone's interested in contributing the code implementation roadmap:
|
@zadjii-msft I will start work on this feature (Sep 19th) for MSFT hackathon. |
Hey All, attached is a draft PR of the work done now and the code cleaned up a bit. Right now there is two big bugs that are holding me back from doing the PR. #14064 First is there is no current way to trigger a settings refresh when OS Theme switches. Any advice on how to trigger an event when the OS Theme switches would be useful. WM_THEMECHANGED does not seem to be working in the message handler. Second, the profile appearance control preview does not update to the "selected" choice for color scheme. I am still trying to debug this a bit, but any suggestions are accepted. I think this is because the preview pulls directly from the profile.defaultappearance, but we never offically save the defaultappearance "ColorSchemeName" because the user hasnt pressed save yet. So some future research will be needed to discover how to force this up. There is future work needed with the GUI, but will not be apart of this PR as @carlos-zamora mentioned this would need some design considerations from the team. For now, you can pull the branch and set the dark and light color schemes under the profile though and play around with it. example: |
Ill be traveling (Sep 23 2022) and will pick up work and considerations / conversations (Sep 24 2022)
|
Hey everybody. We've got a bit of a quandry here, that #14064 helped draw attention to. Do folks want the scheme to sync to the Terminal's theme, or the OS's theme? These each come with pro's and cons. For clarity, the Terminal theme is the window's The Scheme is the colors of the box of text itself. Importantly, this controls the main background of the window.
We're kinda at an impasse here, and want to know what folks think. I suspect most people who are looking for this feature want the scheme to match the OS theme, always, regardless of terminal theme. But I don't want to make assumptions! Maybe folks are using a tool to swap the Terminal theme based on time of day instead! There's also a mind to have the Terminal's |
This is all I care about. I'm not a friend of dark mode at day at all. And vice versa. It hurts my eyes. |
Same thing, I use a script to change the OS theme and some applications that don't have an option to follow it. Just one addition: I would also like the option to change the scheme based on the OS theme but I am not sure if that should be the default or if there's enough info to have matching dark and light schemes without user input. I think someone above mentioned VSCode having an explicit setting for picking a dark and a light scheme that should be used in this case. |
To me the most complete solution would look like this:
|
This would be my ideal behavior:
|
Wow overwhelming initial feedback seems like folks want theme AND scheme to match the OS theme. That kinda sounds like an endorsement for "the scheme matches the Terminal's theme", and anyone who wants this feature manually changes their Terminal theme to Right? |
@zadjii-msft well, yes and no. We want to be able to manually specify a scheme for each theme. That could mean a light scheme with a dark theme, or the more usual dark on dark and light on light. Being able to set a scheme per theme is the important bit IMO |
Rough proposal: two new config properties: |
Er, yes. I was too short there. More verbosely: The user can set both a If the user's only got a FWIW, this is how #14064 is currently implemented, but there was some confusion because the spec wasn't exactly clear as to which theme (terminal or OS) the scheme should be following. |
@zadjii-msft I like the interpretation you just posted. It's the most straightforward / expected default behavior. However, are you sure you are linking to the correct PR? |
SURE DIDN'T. Fatfingered that one. The correct PR is #14064. |
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request This pull request solved the problem of users not being able to set color schemes specifically for dark or light mode. Now the code has been updated to accept a dark and light color scheme in the json. The old setting is still compatible. Keep in mind if you update your color scheme through the settings UI, it will set both dark and light to the color scheme selected. This is because the settings UI update for selecting both Dark and Light color schemes is not supported yet. This also solves the problem of the UI not using the system OS theme. Now you can select system theme and your color scheme will be selected based on if the system theme is dark or light. <!-- Other than the issue solved, is this relevant to any other issues/existing PRs? --> ## References #4066 <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist * [x] Closes #4066 * [x] Closes #14050 * [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA. * [x] Tests added/passed I believe so, added one test to ColorSchemeTests.cpp and I believe it passed. Also had to modify TerminalSettingsTests.cpp to accept the new ApplyAppearanceSettings function template * [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx * [x] Schema updated. * [x] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #4066 and also teams messages with @carlos-zamora <!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments -Removed ColorSchemeName from MTSMSettings.h in order to process the setting for both string and object. -Added DarkColorSchemeName and LightColorSchemeName properties to the AppearanceConfig to replace ColorSchemeName. -Hacked a few processes to play nice with all 3 properties listed above as in some cases around the UI, we need to still use the ColorSchemeName. Once we change the UI I believe we can go back to just Dark and LightColorSchemeName -Added and Updated Test to align to the new code. Acceptable Json values, "colorScheme": { "dark": "Campbell", "light": "Campbell" } or "colorScheme": "Campbell" <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed Individual testing along with the test case added.
🎉This issue was addressed in #14064, which has now been successfully released as Handy links: |
For those who don't understand how to use new functionality:
|
This is great! But it would be better if the option was present in the UI |
Description of the new feature/enhancement
It would be nice to be able to specify two schemes per profile; one for light mode and one for dark. I imagine that would be especially useful with
"requestedTheme": "system"
.Proposed technical implementation details (optional)
Could be implemented in the form of:
or:
The text was updated successfully, but these errors were encountered: