Selected tab title not used for Export Text file name

[elsaco]

Windows Terminal version

main 1ef4a42

Windows build number

10.0.19044.0

Other Software

No response

Steps to reproduce

• build WT with Use the tab's active title for "Export Text" patch applied, commit 2c341c8
• rename some tabs
• export text from a tab other than active one

Foo was the active tab and Bar was used for export:

Expected Behavior

• tab's title used for filename

Actual Behavior

• the active's tab title is being used instead

[DHowett]

It's much worse than that: it actually only exports the active tab. It can't export a non-active tab.

How? It takes tab in as an action argument...! Did we mess something up this egregiously?

[ianjoneill]

So this and #13942 are somewhat related.

Of the tab related context menu items "Duplicate Tab", "Split Tab", "Export Text" and "Find", the only one that does what you expect on an unfocused tab is "Split Tab".

The callbacks are all set up in TabManagement.cpp:

terminal/src/cascadia/TerminalApp/TabManagement.cpp

Lines 193 to 233 in 95a9d8c

	newTabImpl->DuplicateRequested([weakTab, weakThis{ get_weak() }]() {
	auto page{ weakThis.get() };
	auto tab{ weakTab.get() };
	if (page && tab)
	{
	page->_DuplicateTab(*tab);
	}
	});
	newTabImpl->SplitTabRequested([weakTab, weakThis{ get_weak() }]() {
	auto page{ weakThis.get() };
	auto tab{ weakTab.get() };
	if (page && tab)
	{
	page->_SplitTab(*tab);
	}
	});
	newTabImpl->ExportTabRequested([weakTab, weakThis{ get_weak() }]() {
	auto page{ weakThis.get() };
	auto tab{ weakTab.get() };
	if (page && tab)
	{
	// Passing null args to the ExportBuffer handler will default it
	// to prompting for the path
	page->_HandleExportBuffer(nullptr, nullptr);
	}
	});
	newTabImpl->FindRequested([weakTab, weakThis{ get_weak() }]() {
	auto page{ weakThis.get() };
	auto tab{ weakTab.get() };
	if (page && tab)
	{
	page->_Find();
	}
	});

They delegate to:

• TerminalPage::_DuplicateTab(): duplicates the currently focused tab
• TerminalPage::_SplitTab(): works as expected, as it focuses the selected tab and then splits it
• TerminalPage::_HandleExportBuffer(): exports the currently focused tab's content
• TerminalPage::_Find(): searches the currently focused tab's control

I guess the quick win would be to replicate what SplitTab() does - i.e. focus the selected tab and then do the duplicate/export/find. Is that reasonable?

[zadjii-msft]

That seems like a reasonable solution to me. Think that'll knock out all of this issue, #13942 and #13579 all in one go?

[ianjoneill]

Hmm doesn't look like it's that straight forward - _SetFocusedTab() is async. As @JerBast suggests - _DuplicateTab() and _SplitTab() delegate to _MakePane(), which won't necessarily get the focused tab due it's async nature.

[DHowett]

Generally speaking, I'd rather us fix the event plumbing so that things that come out of (1) the pane, (2) the tab and (3) the terminal control are tagged up appropriately so we know their origination chain. Is that something doable on a reasonable time horizon?

[serd2011]

maybe _MakePane(newTerminalArgs, bool duplicate, existingConnection) should accept something like const TerminalTab& sourceTab instead of bool duplicate?
That way _DuplicateTab will pass the tab to _MakePane and it should work.

[JerBast]

Changing _MakePane would work for #13942 for sure. For this issue, it is also possible to replace the call to pane->_HandleExportBuffer(...) with a call to pane->_ExportTab(*tab, L"") directly.

[ianjoneill]

I'd come to the same conclusions. There's no point 3 people working on it though, so I'll leave it to you 🙂

[ghost]

🎉This issue was addressed in #14673, which has now been successfully released as Windows Terminal Preview v1.17.1023.:tada:

Handy links:

• Release Notes
• Store Download