Repository navigation
Don't clobber NewTerminalArgs settings when initializing a tab - #20810
Mike Griese (zadjii-msft) wants to merge 3 commits into
Conversation
#20328 added a call to Tab::UpdateSettings in _InitializeTab so that the tab header would pick up the window's tabWidthMode (previously done in Tab::Initialize via the global settings). However, UpdateSettings also pushes profile-only settings into every pane, overwriting settings that came from NewTerminalArgs (e.g. --suppressApplicationTitle, --colorScheme). Instead, pass the window settings into Tab::Initialize and only update the header width there, like it was before #20328. Closes #20749
| VERIFY_ARE_EQUAL(til::color{ 0xff000000 }, backgroundColor); | ||
| }); | ||
| Log::Comment(L"Sleep to let events propagate"); | ||
| Sleep(250); |
There was a problem hiding this comment.
This test is sus. Could probably be part of an existing test?
There was a problem hiding this comment.
I mean, maybe? I don't hate that it's an actually pretty specific test for the original regression.
It's more of a bigger issue that every "UI" test we write needs to add a 250ms wait at the end to make sure things worked.
we have like, insanely low test coverage for how our UI works so I'm kinda inclined to add more
| ASSERT_UI_THREAD(); | ||
|
|
||
| // GH#20749: Only apply the tab-level window settings here. Don't call | ||
| // UpdateSettings, because that would also push profile-only settings |
There was a problem hiding this comment.
Eh? Why does UpdateSettings override those launch args in the first place?
There was a problem hiding this comment.
Mostly historical reasons I guess. There's been a bug on the backlog for ages for "hot reloading the settings can clobber CLI args". I wanted to scope this fix down to just fixing this bug rather than ((however big that fix ends up being))
#20328 added a call to
Tab::UpdateSettingsin_InitializeTabso that the tab header would pick up the window'stabWidthMode(previously done inTab::Initializevia the global settings).HOWEVER,
UpdateSettingsalso pushes profile-only settings into every pane, overwriting settings that came fromNewTerminalArgs(e.g. CLI args like--suppressApplicationTitle,--colorScheme, etc).Instead, pass the window settings into
Tab::Initializeand only update the header width there, like it was before #20328.Closes #20749
Related to #20751