Code Health: follow ups from connection restarting #18812

Closed
opened 2026-01-31 06:25:10 +00:00 by claunia · 1 comment
Owner

Originally created by @zadjii-msft on GitHub (Nov 3, 2022).

Originally assigned to: @zadjii-msft on GitHub.

From #14060. See also https://github.com/microsoft/terminal/pull/14060#pullrequestreview-1165989506

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
Originally created by @zadjii-msft on GitHub (Nov 3, 2022). Originally assigned to: @zadjii-msft on GitHub. From #14060. See also https://github.com/microsoft/terminal/pull/14060#pullrequestreview-1165989506 The control shouldn't be responsible for restarting the connection itself. It should request a new one from the app. * [ ] Move [this code](https://github.com/sotteson1/terminal/blob/62888a5a0f2ccda00b53e52e96537fd67213e4ab/src/cascadia/TerminalControl/ControlCore.cpp#L415-L420) to the page, and bubble an event to request a new connection * [ ] Revert [this](https://github.com/sotteson1/terminal/blob/62888a5a0f2ccda00b53e52e96537fd67213e4ab/src/cascadia/TerminalConnection/ConnectionStateHolder.h#L89-L95) * [ ] Investigate only clearing the buffer to scrollback, instead of `inheritCursor`ing
Author
Owner

@zadjii-msft commented on GitHub (Dec 6, 2022):

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 commented on GitHub (Dec 6, 2022): 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
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#18812