Can we get rid of the AdaptDefaults interface? #16582

Closed
opened 2026-01-31 05:16:23 +00:00 by claunia · 4 comments
Owner

Originally created by @j4james on GitHub (Feb 1, 2022).

Originally assigned to: @j4james on GitHub.

Description of the new feature/enhancement

There are three methods in the AdaptDefaults interface:

  • The Execute method is never used at all. This is equivalent to the ExecuteChar method in ITerminalApi, which is also not used.
  • The Print method, which takes a char, is ultimately implemented as a call to PrintString, so this could easily be replaced with PrintString in the calling code, as is done in the TerminalDispatch implementation.
  • The PrintString method is all that's left, and that could be moved into the ConGetSet interface, the same way we have a PrintString method in the ITerminalApi interface.

This means the AdaptDispatch class only needs to be constructed with a ConGetSet parameter, which would more closely match the TerminalDispatch class.

Proposed technical implementation details (optional)

Most of the details are covered above. Essentially we'd move the PrintString method into ConGetSet interface and nuke everything else.

Originally created by @j4james on GitHub (Feb 1, 2022). Originally assigned to: @j4james on GitHub. # Description of the new feature/enhancement There are three methods in the [`AdaptDefaults`](https://github.com/microsoft/terminal/blob/fb597ed304ec6eef245405c9652e9b8a029b821f/src/terminal/adapter/adaptDefaults.hpp#L21) interface: * The `Execute` method is never used at all. This is equivalent to the [`ExecuteChar`](https://github.com/microsoft/terminal/blob/62c95b5017e92a780cdc43008e30b4e43d2edc9b/src/cascadia/TerminalCore/ITerminalApi.hpp#L23) method in `ITerminalApi`, which is also not used. * The `Print` method, which takes a char, is ultimately implemented as a call to `PrintString`, so this could easily be replaced with `PrintString` in the calling code, as is done in the [`TerminalDispatch`](https://github.com/microsoft/terminal/blob/main/src/cascadia/TerminalCore/TerminalDispatch.cpp#L29) implementation. * The `PrintString` method is all that's left, and that could be moved into the `ConGetSet` interface, the same way we have a `PrintString` method in the `ITerminalApi` interface. This means the `AdaptDispatch` class only needs to be constructed with a `ConGetSet` parameter, which would more closely match the `TerminalDispatch` class. # Proposed technical implementation details (optional) Most of the details are covered above. Essentially we'd move the `PrintString` method into `ConGetSet` interface and nuke everything else.
Author
Owner

@DHowett commented on GitHub (Feb 1, 2022):

We absolutely can!

@DHowett commented on GitHub (Feb 1, 2022): We absolutely can!
Author
Owner

@DHowett commented on GitHub (Feb 1, 2022):

James -- do you want me to formally assign you on these "can we do a cleanup" bugs? You're knocking it out of the park here, as usual. :)

@DHowett commented on GitHub (Feb 1, 2022): James -- do you want me to formally assign you on these "can we do a cleanup" bugs? You're knocking it out of the park here, as usual. :)
Author
Owner

@j4james commented on GitHub (Feb 1, 2022):

You can if that helps. I'm definitely working on this one and #12253, but ideally they'd both need PR #12247 merged first.

@j4james commented on GitHub (Feb 1, 2022): You can if that helps. I'm definitely working on this one and #12253, but ideally they'd both need PR #12247 merged first.
Author
Owner

@ghost commented on GitHub (May 24, 2022):

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

Handy links:

@ghost commented on GitHub (May 24, 2022): :tada:This issue was addressed in #12390, which has now been successfully released as `Windows Terminal Preview v1.14.143`.:tada: Handy links: * [Release Notes](https://github.com/microsoft/terminal/releases/tag/v1.14.143) * [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#16582