Selected tab title not used for Export Text file name #18408

Closed
opened 2026-01-31 06:13:05 +00:00 by claunia · 9 comments
Owner

Originally created by @elsaco on GitHub (Sep 8, 2022).

Windows Terminal version

main 1ef4a4276

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 2c341c8de
  • rename some tabs
  • export text from a tab other than active one

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

Expected Behavior

  • tab's title used for filename

Actual Behavior

  • the active's tab title is being used instead
Originally created by @elsaco on GitHub (Sep 8, 2022). ### Windows Terminal version main 1ef4a4276 ### 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 2c341c8de - rename some tabs - export text from a tab other than active one `Foo` was the active tab and `Bar` was used for export: ![wt_export_text](https://user-images.githubusercontent.com/3933920/189212775-956220d7-7607-43b3-bd62-a78d51fae0ea.PNG) ### Expected Behavior - tab's title used for filename ### Actual Behavior - the active's tab title is being used instead
Author
Owner

@DHowett commented on GitHub (Sep 8, 2022):

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?

@DHowett commented on GitHub (Sep 8, 2022): 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?
Author
Owner

@ianjoneill commented on GitHub (Sep 9, 2022):

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:

95a9d8c31b/src/cascadia/TerminalApp/TabManagement.cpp (L193-L233)

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?

@ianjoneill commented on GitHub (Sep 9, 2022): 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`: https://github.com/microsoft/terminal/blob/95a9d8c31b3f4e27fb10ccefa525d66eacf26f96/src/cascadia/TerminalApp/TabManagement.cpp#L193-L233 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?
Author
Owner

@zadjii-msft commented on GitHub (Sep 9, 2022):

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

@zadjii-msft commented on GitHub (Sep 9, 2022): That seems like a reasonable solution to me. Think that'll knock out all of this issue, #13942 and #13579 all in one go?
Author
Owner

@ianjoneill commented on GitHub (Sep 9, 2022):

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.

@ianjoneill commented on GitHub (Sep 9, 2022): 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.
Author
Owner

@DHowett commented on GitHub (Sep 9, 2022):

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?

@DHowett commented on GitHub (Sep 9, 2022): 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?
Author
Owner

@serd2011 commented on GitHub (Sep 9, 2022):

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.

@serd2011 commented on GitHub (Sep 9, 2022): 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.
Author
Owner

@JerBast commented on GitHub (Sep 9, 2022):

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.

@JerBast commented on GitHub (Sep 9, 2022): 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.
Author
Owner

@ianjoneill commented on GitHub (Sep 9, 2022):

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

@ianjoneill commented on GitHub (Sep 9, 2022): I'd come to the same conclusions. There's no point 3 people working on it though, so I'll leave it to you 🙂
Author
Owner

@ghost commented on GitHub (Jan 24, 2023):

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

Handy links:

@ghost commented on GitHub (Jan 24, 2023): :tada:This issue was addressed in #14673, which has now been successfully released as `Windows Terminal Preview v1.17.1023`.:tada: Handy links: * [Release Notes](https://github.com/microsoft/terminal/releases/tag/v1.17.1023) * [Store Download](https://www.microsoft.com/store/apps/9n8g5rfz9xk3?cid=storebadge&ocid=badge)
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#18408