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.

Sign up for GitHub

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

Jump to bottom

Code Health: follow ups from connection restarting #14327

Closed
3 tasks
zadjii-msft opened this issue Nov 3, 2022 · 1 comment · Fixed by #15240
Closed
3 tasks

Code Health: follow ups from connection restarting #14327

zadjii-msft opened this issue Nov 3, 2022 · 1 comment · Fixed by #15240
Assignees
@zadjii-msft
Labels
Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Area-TerminalConnection Issues pertaining to the terminal<->backend connection interface In-PR This issue has a related PR Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal. Severity-Blocking We won't ship a release like this! No-siree.
Milestone
Terminal v1.18

Comments

@zadjii-msft
Copy link
Member

From #14060. See also #14060 (review)

The control shouldn't be responsible for restarting the connection itself. It should request a new one from the app.

  • Move this code to the page, and bubble an event to request a new connection
  • Revert this
  • Investigate only clearing the buffer to scrollback, instead of inheritCursoring
@zadjii-msft zadjii-msft added Area-TerminalConnection Issues pertaining to the terminal<->backend connection interface Product-Terminal The new Windows Terminal. Issue-Task It's a feature request, but it doesn't really need a major design. Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Severity-Blocking We won't ship a release like this! No-siree. labels Nov 3, 2022
@zadjii-msft zadjii-msft added this to the Terminal v1.17 milestone Nov 3, 2022
@ghost ghost added the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Nov 3, 2022
@carlos-zamora carlos-zamora removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Nov 3, 2022
@zadjii-msft
Copy link
Member Author

zadjii-msft commented Dec 6, 2022 •
edited
Loading

I very much want to punt this to 1.18. There's a lot of stuff that I built for tear-out which will make this a lot easier to manage.

Now, we have to like, raise an event up to the Pane (who is hanging on to the Profile for this control), to ask the TerminalPage to build a new connection (because TPage alone knows how), then wire that back into the control

But tearout branches have more elaborate ConnectionInformation that we could just stash in the ControlCore and reuse immediately to build a new connection.

I wonder how painful that "backport" would be...

dev/migrie/b/cxn-restarting-attempt-1-backport has the start of this

@zadjii-msft zadjii-msft mentioned this issue Dec 13, 2022
Closed
@ghost ghost added the In-PR This issue has a related PR label Dec 13, 2022
@zadjii-msft zadjii-msft mentioned this issue Apr 26, 2023
Merged
DHowett pushed a commit that referenced this issue Apr 28, 2023
@zadjii-msft
(A better) Refactoring of how connection restarting is handled (#15240)
0d6642a 
A different take on #14548.

> We didn't love that a connection could transition back in the state
diagram, from Closed -> Start. That felt wrong. To remedy this, we're
going to allow the ControlCore to...

ASK the app to restart its connection. This is a much more sensible
approach, than leaving the ConnectionInfo in the core and having the
core do the restart itself. That's mental.

Cleanup from #14060

Closes #14327

Obsoletes #14548
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Apr 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zadjii-msft zadjii-msft

Labels
Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Area-TerminalConnection Issues pertaining to the terminal<->backend connection interface In-PR This issue has a related PR Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal. Severity-Blocking We won't ship a release like this! No-siree.
Projects
None yet
Milestone
Terminal v1.18
2 participants
@carlos-zamora @zadjii-msft