Prepare for per-window settings by zadjii-msft · Pull Request #20328 · microsoft/terminal

Mike Griese (zadjii-msft) · 2026-06-16T21:47:17Z

This is part one of a two-part pair of PRs to add support for per window name settings to the Windows terminal.

This first PR functionally does very little. It introduces the concept of a WindowSettings object, where each WindowSettings object may be different based off of the name of the window. However, the settings model doesn't actually expose different instances of WindowSettings currently. It just introduces one fake instance that actually just wraps the global settings object to store all of them. This means that functionally the code hasn't actually changed in this PR. We are just accessing these properties through an intermediate type.

Largely this PR is mostly mechanical. where we previously would have been accessing properties on the GlobalAppSettings, now we are just accessing the _currentWindowSettings() (via a helper, or passing as a parameter).

All of this refactoring can be done now to always access the correct window settings object so that the next PR is much easier to read. That is where it will get more interesting and we will have more instances of window settings.

re: #9992

…w-prequel

…n AppLogic on startup

…w-prequel

…icrosoft/terminal into user/migrie/per-window-prequel

Carlos Zamora (carlos-zamora)

Ok. I reviewed the PR and took a look at how it's used in part 2 (PR #20329) to get an idea of the feature being implemented.

Off the bat, the main design/architecture issues I have are...

I don't like that WindowSettings is now everywhere. Though the changes are mechanical, it feels weird that everywhere in the code now has a responsibility of knowing what window we're in so that we can retrieve the right set of settings. I'd rather that that's only/mainly the responsibility of the settings model layer, so that each consumer just has to say "give me the value of this setting" rather than "give me the value in this window". I get why that's not the case, but I have an idea on how to fix it (more below).
I don't like that the settings are using themes to apply them. Themes sound like they are intended to be purely visual. In MTSMSettings.h, I see that MTSM_GLOBAL_ONLY_SETTINGS are settings that cannot differ across windows (good), but there's a good amount of settings in MTSM_WINDOW_SETTINGS that don't make sense to me. For example, copyOnSelect, scrollToX are more interactivity-focused, so it would be weird to have two different windows have two different interaction behaviors. I also personally don't like that themes isn't in the settings UI yet, but that's a broader issue, so I feel weird about adding more functionality to an area that isn't exposed there yet.
(maybe more of a question for part 2) Should settings changed at runtime (i.e. "toggle always on top") be preserved? I think that would make sense.

I've been thinking about how these issues can be addressed. Some ideas:

Replace WindowSettings + GlobalAppSettings coupling with GlobalAppSettings inheritance
- IInheritable already does the kind of behavior I want: consumer calls a getter for a setting (i.e. AlwaysShowTabs()) and they just get the value directly without needing to think about the context. If we can use this, we could remove WindowSettingsDefaults() entirely because the value would just be resolved by the getter nice and clean.
- Profile already has a similar system with profile defaults, so doing something like this would be more consistent.
- One of the main issues right now is that GlobalAppSettings carries around _actionMap, _colorSchemes, and _themes. That said, I think we could just move them up to CascadiaSettings and it shouldn't be too much trouble. Honestly, it would make more sense like that anyways.
Directly allow global settings to be set in each "windows" entry
- I think this is more intuitive. Again, it follows the design of profiles and profile defaults, so it should be much easier to understand. Means that you can have dockWindow be a global setting too.

Hope this helps!

Carlos Zamora (carlos-zamora) · 2026-06-24T23:01:26Z

        {
            // Create this only on the first time we load the settings.
-            _terminalSettingsCache = std::make_shared<TerminalSettingsCache>(settings);
+            _terminalSettingsCache = std::make_shared<TerminalSettingsCache>(settings, settings.WindowSettings(_WindowProperties.WindowName()));


Suggested change

_terminalSettingsCache = std::make_shared<TerminalSettingsCache>(settings, settings.WindowSettings(_WindowProperties.WindowName()));

_terminalSettingsCache = std::make_shared<TerminalSettingsCache>(settings, _currentWindowSettings());

nit: easier to read

Mike Griese (zadjii-msft) · 2026-07-10T10:44:53Z

RE: flipping things around, so that the settings model just gives you the right setting for the current window:

Broadly, I don't think that's possible with our current settings model. Like the settings model is designed to be agnostic of the actual window instance that's hosting it. Like theoretically, we could make a separate settings window that's not attached to any terminal window.

But most importantly, we only ever have one instance of the settings model. That one instance is shared across all terminal windows. How could we ever access this window's specific settings, without somehow passing in something to identify the window?

re:

Replace WindowSettings + GlobalAppSettings coupling with GlobalAppSettings inheritance

I didn't want to do that approach, because I felt like there were definitely settings that didn't make sense to be per-window. windowingBehavior is a great example - that cannot make sense as a per-window setting. If we can inherit the whole blob of GlobalAppSettings... then like, all these settings would have to move to CascadiaSettings directly, which still just feels weird.

Carlos Zamora (carlos-zamora)

Discussed the IInheritable idea offline. We decided that the current approach is better.

Gave it all another full review. Mainly blocking over the one labelled 🛑.

Carlos Zamora (carlos-zamora) · 2026-07-15T20:49:46Z

    {
        MainPage() = delete;
-        MainPage(const Model::CascadiaSettings& settings);
+        MainPage(const Model::CascadiaSettings& settings, const Model::WindowSettings& windowSettings);


🛑 Uh oh. What if you have two windows (light theme and dark theme), then you drag the settings tab from one window to the other? The theme definitely won't match up, and it looks like we'll be modifying the wrong WindowSettings.

Best fix might be to call UpdateSettings() when the tab/pane attaches to the new window. The only other option I can think of is passing around the window name instead, but I think we've mentioned that that's a bit fragile to renames.

So, here's the sitch.

I very explicitly didn't try to handle the settings tab for multiple different window settings. The settings tab will always only let you edit the default window settings. There's not really any affordance within the settings tab itself to try to communicate which window you are editing the settings for.

I also felt like we could probably just p punt that decision for now, considering we're also actively working on redesigning huge parts of the settings.

(Reviewing out loud, so feel free to skip to the end of this comment)

https://github.com/microsoft/terminal/pull/20328/changes#diff-eedad99a72f169d53e254be79a87a7baef15686c06b920bb8e94167b580654c0L4590

This is the part I'm still concerned about though. We're initializing the settings UI with _currentWindowSettings().

Then when we look at the MainPage ctor we have...

_windowSettingsSource{ windowSettings }, _windowSettingsClone{ _settingsClone.WindowSettingsDefaults() },

Looking through the TerminalSettingsEditor changes:

MainPage

MainPage::SettingsNav_Loaded: uses _windowSettingsSource (aka current window settings) ✅ (means we use that for theming)

MainPage::ResetButton_Click(): uses _windowSettingsSource ✅

profile view models: _windowSettingsClone ✅

_UpdateBackgroundForMica(): uses _windowSettingsSource for current theme ✅

Compatibility.h: uses WindowSettingsDefaults() ✅

GlobalAppearanceViewModel.cpp: uses _WindowSettings (aka _windowSettingsClone --> WindowSettingsDefaults() passed in above) ✅

InteractionViewModel: uses _WindowSettings (aka _windowSettingsClone --> WindowSettingsDefaults() passed in above) ✅

LaunchViewModel: uses WindowSettingsDefaults() ✅

NewTabMenuViewModel.cpp: uses WindowSettingsDefaults() ✅

ProfileViewModel: uses _windowSettings (aka _windowSettingsClone --> WindowSettingsDefaults() passed in above) ✅

RenderingViewModel: WindowSettingsDefaults ✅

📝 I noticed that this one doesn't modify the ctor. We just grab the WindowSettingsDefaults off of _settings. Personally, I like this pattern more. The author doesn't even get the chance to use the wrong one.

So yeah, looks like everything is good here. Long-term, I think matching the RenderingViewModel pattern would be best because it prevents people from using the wrong one and it makes it extra clear that we're accessing WindowSettingsDefaults. Especially since we're punting exposing different windows' settings. That's just my 2 cents tho.

…w-prequel

Carlos Zamora (carlos-zamora)

Cool. Just blocking over the LogSettingChanges comment.

Re: #20328 (comment)
No longer blocking, though it would be nice to follow the RenderingViewModel pattern.

Carlos Zamora (carlos-zamora) · 2026-08-10T19:50:35Z

+        _windowSettingsSource = windowSettings;
+        _windowSettingsClone = _settingsClone.WindowSettingsDefaults();


nit: I recommend adding a brief comment here including what we talked about in #20328 (comment)

Reason is, I'm concerned somebody will come by later and see that this should be:

_windowSettingsSource = windowSettings; _windowSettingsClone = windowSettings.Copy();

to match the _settingsSource/_settingsClone assignments above.

Based on the linked thread, we don't want anybody coming by and changing it like that. A comment would help prevent that from happening imo.

Carlos Zamora (carlos-zamora) · 2026-08-10T19:54:44Z

            else if (*clickedItemTag == interactionTag)
            {
-                contentFrame().Navigate(xaml_typename<Editor::Interaction>(), winrt::make<NavigateToPageArgs>(winrt::make<InteractionViewModel>(_settingsClone.GlobalSettings()), *this, elementToFocus));
+                contentFrame().Navigate(xaml_typename<Editor::Interaction>(), winrt::make<NavigateToPageArgs>(winrt::make<InteractionViewModel>(_settingsClone.GlobalSettings(), _settingsClone.WindowSettingsDefaults()), *this, elementToFocus));


Suggested change

contentFrame().Navigate(xaml_typename<Editor::Interaction>(), winrt::make<NavigateToPageArgs>(winrt::make<InteractionViewModel>(_settingsClone.GlobalSettings(), _settingsClone.WindowSettingsDefaults()), *this, elementToFocus));

contentFrame().Navigate(xaml_typename<Editor::Interaction>(), winrt::make<NavigateToPageArgs>(winrt::make<InteractionViewModel>(_settingsClone.GlobalSettings(), _windowSettingsClone), *this, elementToFocus));

nit: for consistency with the other branches in this function

Carlos Zamora (carlos-zamora) · 2026-08-10T20:23:15Z

    {
        MainPage() = delete;
-        MainPage(const Model::CascadiaSettings& settings);
+        MainPage(const Model::CascadiaSettings& settings, const Model::WindowSettings& windowSettings);


(Reviewing out loud, so feel free to skip to the end of this comment)

https://github.com/microsoft/terminal/pull/20328/changes#diff-eedad99a72f169d53e254be79a87a7baef15686c06b920bb8e94167b580654c0L4590

This is the part I'm still concerned about though. We're initializing the settings UI with _currentWindowSettings().

Then when we look at the MainPage ctor we have...

_windowSettingsSource{ windowSettings }, _windowSettingsClone{ _settingsClone.WindowSettingsDefaults() },

Looking through the TerminalSettingsEditor changes:

MainPage

MainPage::SettingsNav_Loaded: uses _windowSettingsSource (aka current window settings) ✅ (means we use that for theming)

MainPage::ResetButton_Click(): uses _windowSettingsSource ✅

profile view models: _windowSettingsClone ✅

_UpdateBackgroundForMica(): uses _windowSettingsSource for current theme ✅

Compatibility.h: uses WindowSettingsDefaults() ✅

GlobalAppearanceViewModel.cpp: uses _WindowSettings (aka _windowSettingsClone --> WindowSettingsDefaults() passed in above) ✅

InteractionViewModel: uses _WindowSettings (aka _windowSettingsClone --> WindowSettingsDefaults() passed in above) ✅

LaunchViewModel: uses WindowSettingsDefaults() ✅

NewTabMenuViewModel.cpp: uses WindowSettingsDefaults() ✅

ProfileViewModel: uses _windowSettings (aka _windowSettingsClone --> WindowSettingsDefaults() passed in above) ✅

RenderingViewModel: WindowSettingsDefaults ✅

📝 I noticed that this one doesn't modify the ctor. We just grab the WindowSettingsDefaults off of _settings. Personally, I like this pattern more. The author doesn't even get the chance to use the wrong one.

So yeah, looks like everything is good here. Long-term, I think matching the RenderingViewModel pattern would be best because it prevents people from using the wrong one and it makes it extra clear that we're accessing WindowSettingsDefaults. Especially since we're punting exposing different windows' settings. That's just my 2 cents tho.

Carlos Zamora (carlos-zamora) · 2026-08-10T20:24:30Z

🛑 Noticed one thing worth flagging. Looks like window settings aren't hooked up to LogSettingChanges(), so we won't be tracking usage for those global settings anymore. That should probably be fixed.

Carlos Zamora (carlos-zamora)

Thanks for bearing with me. Nice work :)

Dustin L. Howett (DHowett)

Looks good to me. Open questions which I shall hope for you to answer before merging:

did we miss updating the window settings object when we rename a window? what happens if you rename a window to a different window settings group anyway...
that's all

good work. i love how you've staged this

Dustin L. Howett (DHowett) · 2026-08-13T22:43:56Z

        // Stash away the current requested theme of the app. We'll need that in
        // _BackgroundBrush() to do a theme-aware resource lookup
-        _requestedTheme = settings.GlobalSettings().CurrentTheme().RequestedTheme();
+        _requestedTheme = settings.GlobalSettings().CurrentTheme(windowSettings).RequestedTheme();


curious why this one is not on WindowSettings

i talked myself into a (rather unsatisfying but acceptable) answer

Dustin L. Howett (DHowett) · 2026-08-13T22:48:29Z

+        _TextMeasurement = windowSettings.TextMeasurement();
+        _AmbiguousWidth = windowSettings.AmbiguousWidth();


these two are actually application-level toggles that we can't make per-window; what should we do with them?

they impact the CodepointWidthDetector singleton

Dustin L. Howett (DHowett) · 2026-08-13T22:53:50Z

 }

-winrt::Microsoft::Terminal::Settings::Model::Theme GlobalAppSettings::CurrentTheme() noexcept
+winrt::Microsoft::Terminal::Settings::Model::Theme GlobalAppSettings::CurrentTheme(const Model::WindowSettings& window) noexcept


I read all the way here and I still don't understand why this is here. Is it because _themes is a member of GlobalAppSettings and it's the only way to access it?

- Merge upstream main (per-window settings refactor microsoft#20328 and 24 others) - Resolve TabPosition migration: global -> WindowSettings (_currentWindowSettings()) - Fix CurrentTheme() calls to new signature CurrentTheme(windowSettings)

Leonard Hecker (lhecker) · 2026-08-18T14:08:09Z

Sad news: Test harness broke.

Mike Griese (zadjii-msft) · 2026-08-18T14:23:12Z

asldkfjalskdfjasldkf

Okay I fixed the local tests but I'll take a look at the PGO ones. Sorry :/

Regressed in #20328.

Mike Griese (zadjii-msft) added 12 commits April 24, 2026 06:14

Start by standing up a dummy type

d73b4e5

just as I thought - we didn't actually need this

5b0e4b8

port the changes over to main ; just the refactoring, none of the impl

d2edb0a

Merge branch 'main' into user/migrie/per-window-prequel

6847c27

Merge remote-tracking branch 'origin/main' into user/migrie/per-windo…

12f3df2

…w-prequel

i think this makes the diff smaller

75d3250

move WindowSettings into GAS because cppwinrt includes suck

8d74e96

move startup actions to the window itself, rather than being parsed i…

611ee57

…n AppLogic on startup

tab settings shouldn't be the defaults

486d251

a lot of the theme moving to be per-window-name

448487b

theming needs to be windowsettings aware

bb8b7ab

you can use a namespace for that

2b7838d

Mike Griese (zadjii-msft) mentioned this pull request Jun 16, 2026

Add support for per-window-name settings #20329

Open

Mike Griese (zadjii-msft) added 6 commits June 22, 2026 14:10

Merge remote-tracking branch 'origin/main' into user/migrie/per-windo…

c71f8fc

…w-prequel

tests and main merge fixes

1d2df3b

Merge remote-tracking branch 'origin/main' into user/migrie/per-windo…

6cb1e06

…w-prequel

Merge branch 'user/migrie/per-window-prequel' of https://github.com/m…

6ff5e11

…icrosoft/terminal into user/migrie/per-window-prequel

fix tests

f0cc8b4

Merge branch 'user/migrie/per-window-prequel' of https://github.com/m…

f6bf64a

…icrosoft/terminal into user/migrie/per-window-prequel

Carlos Zamora (carlos-zamora) suggested changes Jun 25, 2026

View reviewed changes

microsoft-github-policy-service Bot added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jun 25, 2026

microsoft-github-policy-service Bot added the No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. label Jul 2, 2026

Carlos Zamora (carlos-zamora) removed the No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. label Jul 2, 2026

microsoft-github-policy-service Bot added the No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. label Jul 9, 2026

This comment was marked as off-topic.

Sign in to view

Mike Griese (zadjii-msft) removed the No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. label Jul 10, 2026

microsoft-github-policy-service Bot added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Jul 10, 2026

Mike Griese (zadjii-msft) removed the Needs-Attention The core contributors need to come back around and look at this ASAP. label Jul 10, 2026

Carlos Zamora (carlos-zamora) suggested changes Jul 16, 2026

View reviewed changes

microsoft-github-policy-service Bot added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jul 16, 2026

Dustin L. Howett (DHowett) mentioned this pull request Jul 21, 2026

Allow choosing separate light & dark color schemes in the Settings UI #20390

Closed

2 of 4 tasks

microsoft-github-policy-service Bot added the No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. label Jul 23, 2026

Mike Griese (zadjii-msft) removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. labels Jul 23, 2026

Mike Griese (zadjii-msft) self-assigned this Jul 23, 2026

Mike Griese (zadjii-msft) added 3 commits August 7, 2026 06:29

Merge remote-tracking branch 'origin/main' into user/migrie/per-windo…

8f9297a

…w-prequel

nits

b817c64

format

7f1e73c

Mike Griese (zadjii-msft) assigned Carlos Zamora (carlos-zamora) and unassigned Mike Griese (zadjii-msft) Aug 7, 2026

Mike Griese (zadjii-msft) removed the Needs-Discussion Something that requires a team discussion before we can proceed label Aug 7, 2026

fix localtests

e27b167

Carlos Zamora (carlos-zamora) suggested changes Aug 10, 2026

View reviewed changes

microsoft-github-policy-service Bot added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Aug 10, 2026

pr nits

fc5810c

microsoft-github-policy-service Bot removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Aug 13, 2026

Carlos Zamora (carlos-zamora) approved these changes Aug 13, 2026

View reviewed changes

Dustin L. Howett (DHowett) approved these changes Aug 13, 2026

View reviewed changes

Dustin L. Howett (DHowett) merged commit b4229d0 into main Aug 13, 2026
20 checks passed

Dustin L. Howett (DHowett) deleted the user/migrie/per-window-prequel branch August 13, 2026 22:57

Leonard Hecker (lhecker) mentioned this pull request Aug 18, 2026

Sixel image output crashes the terminal #20548

Closed

Mike Griese (zadjii-msft) mentioned this pull request Aug 18, 2026

Fix PGO tests #20560

Merged

Leonard Hecker (lhecker) pushed a commit that referenced this pull request Aug 18, 2026

Fix PGO tests (#20560)

1507c46

Regressed in #20328.

Dustin L. Howett (DHowett) mentioned this pull request Sep 30, 2026

per-window-settings breaks --suppressApplicationTitle #20749

Open

Dustin L. Howett (DHowett) mentioned this pull request Oct 8, 2026

add setting to disable drag/drop #20394

Open

2 of 4 tasks

	_terminalSettingsCache = std::make_shared<TerminalSettingsCache>(settings, settings.WindowSettings(_WindowProperties.WindowName()));
	_terminalSettingsCache = std::make_shared<TerminalSettingsCache>(settings, _currentWindowSettings());

		_windowSettingsSource = windowSettings;
		_windowSettingsClone = _settingsClone.WindowSettingsDefaults();

	contentFrame().Navigate(xaml_typename<Editor::Interaction>(), winrt::make<NavigateToPageArgs>(winrt::make<InteractionViewModel>(_settingsClone.GlobalSettings(), _settingsClone.WindowSettingsDefaults()), *this, elementToFocus));
	contentFrame().Navigate(xaml_typename<Editor::Interaction>(), winrt::make<NavigateToPageArgs>(winrt::make<InteractionViewModel>(_settingsClone.GlobalSettings(), _windowSettingsClone), *this, elementToFocus));

		_TextMeasurement = windowSettings.TextMeasurement();
		_AmbiguousWidth = windowSettings.AmbiguousWidth();

Conversation

Mike Griese (zadjii-msft) commented Jun 16, 2026

Uh oh!

Carlos Zamora (carlos-zamora) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as off-topic.

Mike Griese (zadjii-msft) commented Jul 10, 2026

Uh oh!

Carlos Zamora (carlos-zamora) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Carlos Zamora (carlos-zamora) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Carlos Zamora (carlos-zamora) left a comment

Choose a reason for hiding this comment

Uh oh!

Dustin L. Howett (DHowett) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Leonard Hecker (lhecker) commented Aug 18, 2026

Uh oh!

Mike Griese (zadjii-msft) commented Aug 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants