Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Title event handler attached to control on tab discards title variable #3875

Closed
miniksa opened this issue Dec 6, 2019 · 5 comments
Closed
Assignees
Labels
Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) Product-Terminal The new Windows Terminal. Resolution-Fix-Available It's available in an Insiders build or a release
Milestone

Comments

@miniksa
Copy link
Member

miniksa commented Dec 6, 2019

control.TitleChanged([weakThis](auto newTitle) {
// Check if Tab's lifetime has expired
if (auto tab{ weakThis.lock() })
{
// The title of the control changed, but not necessarily the title of the tab.
// Set the tab's text to the active panes' text.
tab->SetTabText(tab->GetActiveTitle());
}
});

I'm trying to make the Telnet and Azure connection types set a title when they start up on the tab.

They're calling the ESC]0;<title>BEL sequence on the output handlers to make it happen.

Turns out that title makes it all the way to here and then is completely discarded in favor of whatever's already on the title.

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Dec 6, 2019
@miniksa miniksa added Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. labels Dec 6, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Dec 6, 2019
@miniksa
Copy link
Member Author

miniksa commented Dec 6, 2019

@zadjii-msft, this is the thing I was talking about.

@DHowett-MSFT
Copy link
Contributor

I looked into this. Emitting a title during Start() forces Pane to ask Control for its title before _initializedTerminal is true.

We need to rework the title eventing, and also fix starting titles to actually apply to the terminal instead of letting conhost spout it back at us.

@zadjii-msft
Copy link
Member

For the record, discarding the value of newTitle is intentional here. The Tab doesn't have a TitleChanged event, only the control does. So we hook up a handler to every control's TitleChanged event as a way of getting notified that the Tab's title might have changed.

We maybe should just set the Tab's title to the StartingTitle of the first control added to the tab, if that property is set, rather than waiting for the title to roundtrip through conpty to us.

@DHowett-MSFT DHowett-MSFT removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Dec 9, 2019
@DHowett-MSFT DHowett-MSFT added this to the Terminal v1.0 milestone Dec 9, 2019
@DHowett-MSFT DHowett-MSFT added the Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. label Dec 9, 2019
@zadjii-msft zadjii-msft added the Priority-3 A description (P3) label Jan 22, 2020
@zadjii-msft zadjii-msft self-assigned this Jul 13, 2021
@zadjii-msft
Copy link
Member

@miniksa do we think this is still important? At some point, we changed Terminal::GetConsoleTitle to return the _startingTitle if the title isn't set by VT. Looks like that was in #6433, which was roughly the 1.1 timeframe.

You can go ahead and reopen if you want to veto.

@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label Jul 13, 2021
@zadjii-msft zadjii-msft added the Resolution-Fix-Available It's available in an Insiders build or a release label Jul 13, 2021
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Jul 13, 2021
@miniksa
Copy link
Member Author

miniksa commented Jul 13, 2021

Nah its fine thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) Product-Terminal The new Windows Terminal. Resolution-Fix-Available It's available in an Insiders build or a release
Projects
None yet
Development

No branches or pull requests

4 participants