Skip to content

Commit 908fd74

Browse files
committed
Try to clone the settings directly from the control
Okay this is harder, but maybe not impossible, but probably impossible in v1. The Profile knows how to build the connection settings, and the ControlSettings, etc. However, when the settings reloads like in this bug, we'll remove all the old profiles, and build a new list. If we wanted the case 1 behavior, we could wait until the Profile is a winrt object, then have each Pane hold a strong ref to the Profile that it's hosting. Then it would be trivial to be able to duplicate the settings from it. I believe that work is tracked in #3998. How palatable is doing case 3 now (to fix the crash), then filing a follow up to do case 1 once #3998 is done?
1 parent eafe9c8 commit 908fd74

File tree

1 file changed

+18
-7
lines changed

1 file changed

+18
-7
lines changed

src/cascadia/LocalTests_TerminalApp/TabTests.cpp

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -169,10 +169,11 @@ namespace TerminalAppLocalTests
169169

170170
void TabTests::TryDuplicateBadTab()
171171
{
172-
// Create a tab with a profile with GUID 1
173-
// Reload the settings so that GUID 1 is no longer in the list of profiles
174-
// Try calling _DuplicateTabViewItem on tab 1
175-
// No new tab should be created (and more importantly, the app should not crash)
172+
// * Create a tab with a profile with GUID 1
173+
// * Reload the settings so that GUID 1 is no longer in the list of profiles
174+
// * Try calling _DuplicateTabViewItem on tab 1
175+
// * The tab that's created should use the old historySize value from the
176+
// duplicated tab. More importantly, the app should not crash.
176177
//
177178
// Created to test GH#2455
178179

@@ -249,7 +250,9 @@ namespace TerminalAppLocalTests
249250
result = RunOnUIThread([&page]() {
250251
VERIFY_IS_NOT_NULL(page);
251252
VERIFY_IS_NOT_NULL(page->_settings);
253+
252254
page->Create();
255+
Log::Comment(NoThrowString().Format(L"Create()'d"));
253256

254257
// I think in the tests, we don't always set the focused tab on
255258
// creation. Doesn't seem to be a problem in the real app, but
@@ -258,18 +261,19 @@ namespace TerminalAppLocalTests
258261
// Manually set it here, so that later, the _GetFocusedTabIndex call
259262
// in _DuplicateTabViewItem will have a sensible value.
260263
page->_SetFocusedTabIndex(0);
264+
Log::Comment(NoThrowString().Format(L"_SetFocusedTabIndex()'d"));
261265
});
262266
VERIFY_SUCCEEDED(result);
263267

264268
result = RunOnUIThread([&page]() {
265-
VERIFY_ARE_EQUAL(1u, page->_tabs.size());
269+
VERIFY_ARE_EQUAL(1u, page->_tabs.Size());
266270
});
267271
VERIFY_SUCCEEDED(result);
268272

269273
Log::Comment(NoThrowString().Format(L"Duplicate the first tab"));
270274
result = RunOnUIThread([&page]() {
271275
page->_DuplicateTabViewItem();
272-
VERIFY_ARE_EQUAL(2u, page->_tabs.size());
276+
VERIFY_ARE_EQUAL(2u, page->_tabs.Size());
273277
});
274278
VERIFY_SUCCEEDED(result);
275279

@@ -284,7 +288,14 @@ namespace TerminalAppLocalTests
284288
Log::Comment(NoThrowString().Format(L"Duplicate the tab, and don't crash"));
285289
result = RunOnUIThread([&page]() {
286290
page->_DuplicateTabViewItem();
287-
VERIFY_ARE_EQUAL(2u, page->_tabs.size(), L"We should gracefully do nothing here - the profile no longer exists.");
291+
VERIFY_ARE_EQUAL(3u,
292+
page->_tabs.Size(),
293+
L"We should duplicate the settings of the second "
294+
L"tab here, despite the profile no longer existing");
295+
296+
auto control = page->_GetStrongTabImpl(2)->GetActiveTerminalControl();
297+
VERIFY_IS_NOT_NULL(control);
298+
VERIFY_ARE_EQUAL(1, control.Settings().HistorySize());
288299
});
289300
VERIFY_SUCCEEDED(result);
290301
}

0 commit comments

Comments
 (0)