microsoft / terminal

The new Windows Terminal and the original Windows console host, all in the same place!
MIT License
94.85k stars 8.21k forks source link

Switching OS input method resets Terminal colors and zoom #11522

Open antoineco opened 2 years ago

antoineco commented 2 years ago

Windows Terminal version (or Windows build number)

Windows 11 Version: 10.0.22000.258

Windows Terminal Preview Version: 1.11.2731.0

Other Software

Bash (in WSL)

Steps to reproduce

  1. Inside a xterm terminal, override the terminal's colors using escape sequences such as:

    # Set "red" color to hex(ab4642)
    printf '\033]4;%d;rgb:%s\033\\' 1 'ab/46/42';
    # Set "background" color to hex(181818)
    printf '\033]%d;rgb:%s\033\\' 11 '18/18/18';

    The full theme I use can be applied using the following script https://github.com/fnune/base16-shell/blob/master/scripts/base16-default-dark.sh. This method allows me to conveniently switch between dark and light themes, regardless of the OS I'm currently using.

  2. Assuming you have multiple input methods configured in Windows (e.g. I switch between "US" and "US-International" regularly), press either Ctrl+Shift or Win+Space to switch between input methods.

    Switching input from the task bar using the mouse also triggers this bug.

Expected Behavior

Switching input methods in Windows has no impact on the configuration of my open Terminal sessions.

Actual Behavior

The terminal's colors get reset to the color scheme configured in Terminal for the current session.

Before switching input methods: image

After switching input methods: image

zadjii-msft commented 2 years ago

Hmm. This is a decent point. We reload the settings when we get a keyboard layout change. This is important for things like the actions, who might need to be re-parsed for different keys in the new layout. But this is something that's totally unrelated that's getting blown away.

I'm running into a similar issue with the work in ragnarok for #5000. If you set the colors with VT, then hot-reload the settings, we'll remove the changes to those colors. (same with font size, opacity, anything that can get changed at runtime really). The question is - does a change to the settings override a runtime change by the user? And I think I'm leaning "it probably shouldn't". But then you get into weird things like "should 'preview color scheme' still work on top of the user setting the colors with VT" and then I lean "yes it should"

antoineco commented 2 years ago

I think it is acceptable to expect the settings to be reloaded when certain things change.

I was just surprised to see something, which seemed unrelated to me, cause a reload in Terminal.

If this is not something that can (or should) be avoided, I'm also fine closing as "works as intended". At least the issue should contain enough keywords for someone to find it if they run into a similar issue.

zadjii-msft commented 2 years ago

I think I'm actually gonna leave this open. I think I've got an idea in my notes for now to get this to work. Setting the colors with VT will set a runtime "colors have been overridden" flag, and previewing a scheme will clear that flag out. But a plain-old settings reload, that won't touch that flag, so that'll allow the vt-set colors to persist across hot-reloads of the settings. We might be able to get even more precise and determine that the color scheme for a profile changed, and in that scenario, also clear out the flag.

A bunch of work really, but definitely possible.

j4james commented 2 years ago

Setting the colors with VT will set a runtime "colors have been overridden" flag, and previewing a scheme will clear that flag out.

Can't we just make it so that the settings dialog only updates properties that have actually been edited in some way? So if you change the color scheme for a profile, or edit something on the Color Schemes page, that'll override any colors that have been set programmatically, but if you edit something unrelated (like say the Actions page), then we leave the colors as they are.

This applies to other settings as well. If some application has left the cursor switched to an underscore when your preference is a block, you should be able to go to the Appearance page and reapply your preference by toggling the value or perhaps just clicking Save. It doesn't seem reasonable to me for an application to lock in changes to things like colors and the cursor so your only recourse is to close the tab and start again.

Following the principle of least astonishment:

  1. If you edit something on the Actions page in the settings, would you be surprised if that also caused the colors in the terminal to be reset? I think the answer is probably "yes" (as seen from this issue), but perhaps not hugely surprising when you think about what might have triggered that change.
  2. If you try and change the color scheme for your profile, would you be surprised if that didn't have any effect, just because some app sent a palette-altering VT sequence at some point in the past? In this case I think the answer is definitely "yes", and it would almost certainly be seen as a bug, because it's not at all obvious why it would have failed.
zadjii-msft commented 2 years ago

Can't we just make it so that the settings dialog only updates properties that have actually been edited in some way? So if you change the color scheme for a profile, or edit something on the Color Schemes page, that'll override any colors that have been set programmatically, but if you edit something unrelated (like say the Actions page), then we leave the colors as they are.

I would love that. Unfortunately we don't really have the architecture in place at the moment to say "this part of the settings changed" 😞

j4james commented 2 years ago

Unfortunately we don't really have the architecture in place at the moment to say "this part of the settings changed"

I get that, but I thought what you were proposing was going to take a bunch of work anyway, and would be a step backwards for the second scenario I described (assuming I've understood you correctly). So if anyone is going to be spending significant time on trying to fix this, it would be nice to try and get it right (assuming we're in agreement on what "right" is).

It could be something simple like saving the settings we care about (e.g. colors and cursor shapes) when the dialog is opened, and then when the user saves, you only apply those settings to any open tabs if the values have changed from what they were at the start.

zadjii-msft commented 2 years ago

future self: 1aa2849d9 has the commit for the incredibly silly RUNTIME_PROPERTY

#define RUNTIME_PROPERTY(type, name, setting, ...)                \
private:                                                          \
    type _##name{ __VA_ARGS__ };                                  \
    std::optional<type> _runtime##name{ std::nullopt };           \
                                                                  \
public:                                                           \
    void name(const type newValue) { _runtime##name = newValue; } \
                                                                  \
    type name() const { return _runtime##name ? *_runtime##name : _##name; }

and then we'd stealth slide the updated settings into the _##name values when the settings are updated instead of just discarding the entire ControlSettings/ControlAppearance objects. That however is mildly breaking and probably out of scope for the refactor. Hence, leaving it here.

Kinda like #11067 also.

I'm trusting that dustin did the diligence searching.

zadjii-msft commented 2 years ago

falling-asleep-thought: we could probably hook the language change notifier up to a method that only reloads the keybindings, since those are the only thing that would have changed. Seems like an easy solution....

pbl987 commented 2 years ago

I think i have a similiar problem. When i open an "admin" console, it gets a red admin color scheme assigned. (Picture 1). When i then press Win + Space, it gets reset to standard. It also happens on other occasions, but i can't say why. (Picture 2). Maybe it's like in the title.

Screenshot 2022-05-26 003221

Screenshot 2022-05-26 003304

Screenshot 2022-05-26 003454

zadjii-msft commented 2 years ago

@pbl987 That's definitely this same bug. Might I interest you in the elevate setting, new in 1.13? image

That'll let you mark a profile as requiring an admin window, and will automatically create an admin one if you're running unelevated. Then, you could just set the tab color and colorscheme for that elevate:true profile normally, and a language change won't blow them away πŸ˜‰

pbl987 commented 2 years ago

@zadjii-msft I would love that. Thank you for working on this!

Reading the first posts, i got the impression that it's not a priority issue. But (minor) stuff like that is really annoying, so i'm glad you guys are on it! :-)

By the way, the runas feature is also much-needed, as a non-professional it took me some time to figure out the parameters for the commandline and the escape sequences. (original version had been this beauteousness :D : "C:\Users\patrick\AppData\Local\Microsoft\WindowsApps\Microsoft.PowerShell_8wekyb3d8bbwe\pwsh.exe" -Command Start-Process -verb runas -Filepath "wt" -ArgumentList """-p """"PowerShell ```(Admin```)"""" --tabColor #E74856 --colorScheme Adminfarben""" )

zadjii-msft commented 2 years ago

By the way, the runas feature is also much-needed, as a non-professional it took me some time to figure out the parameters for the commandline and the escape sequences

Yea, that's why the elevate setting is so great, now you can replace that whole commandline with just "C:\Users\patrick\AppData\Local\Microsoft\WindowsApps\Microsoft.PowerShell_8wekyb3d8bbwe\pwsh.exe" ☺️

vient commented 1 year ago

Please take a look at this πŸ™πŸΌ It is uncomfortable to constantly adjust terminal zoom back when you forgot to change layout back to EN before switching to terminal window.

vient commented 1 year ago

So, my comment was marked as spam: is there any other mechanism to vote for issues? My point is that this issue is causing problems for anybody using more than one keyboard layout, yet it is not fixed more than year later.

I myself reported it three months ago, it was just marked as duplicate.

zadjii-msft commented 1 year ago

If you'd like to "+1" this feature, the best way to do that is by hitting the πŸ‘ button on this issue

image

That way, you avoid unnecessarily pinging everyone following this thread. Thanks!

vient commented 1 year ago

FYI: this behavior can be bypassed, in a way, by enabling Per-Window Keyboard Layout in Windows settings (I just thought "what if Windows could have per-window keyboard layouts" and well, it already has this functionality).

akaltar commented 1 year ago

Would like to add that this also resets the tab titles: before after

zadjii-msft commented 1 year ago

@jsidewhite just found a newer, wackier version of this repro:

now why does that reload the settings?