Refactor the TerminalDispatch graphics rendition to match AdaptDispatch #9337

Open
opened 2026-01-31 01:52:02 +00:00 by claunia · 0 comments
Owner

Originally created by @j4james on GitHub (Jun 30, 2020).

Description of the new feature/enhancement

I'd like to propose we refactor the SetGraphicsRendition implementation in the TerminalDispatch class to match the refactoring done for AdaptDispatch in PR #5758.

I've been looking at adding a few more of the SGR attributes, but I found that adding them to the TerminalDispatch was a bit of a pain, because every new attribute required a new method in the ITerminalApi interface. If we matched the AdaptDispatch implementation, it would just be one more entry in the switch statement.

Other benefits are that the AdaptDispatch implementation is a bit simpler, and easier to follow. It should be a little more efficient when dealing with multiple SGR options. And ultimately we're going to need these implementations to be identical anyway, so we can deduplicate them for issue #3849.

Proposed technical implementation details (optional)

  1. Add GetTextAttributes and SetTextAttributes methods to the ITerminalApi interface to match the PrivateGetTextAttributes and PrivateSetTextAttributes methods in the ConGetSet interface.
  2. Essentially just copy and paste the AdaptDispatch::SetGraphicsRendition implementation into TerminalDispatch. I think the only difference is we don't need the color index remapping, and the color index constants are different for the same reason (resolving that discrepancy is still on my TODO list).
  3. Delete all the methods in the ITerminalApi interface that are no longer required.
Originally created by @j4james on GitHub (Jun 30, 2020). # Description of the new feature/enhancement I'd like to propose we refactor the `SetGraphicsRendition` implementation in the `TerminalDispatch` class to match the refactoring done for `AdaptDispatch` in PR #5758. I've been looking at adding a few more of the SGR attributes, but I found that adding them to the `TerminalDispatch` was a bit of a pain, because every new attribute required a new method in the `ITerminalApi` interface. If we matched the `AdaptDispatch` implementation, it would just be one more entry in the switch statement. Other benefits are that the `AdaptDispatch` implementation is a bit simpler, and easier to follow. It should be a little more efficient when dealing with multiple SGR options. And ultimately we're going to need these implementations to be identical anyway, so we can deduplicate them for issue #3849. # Proposed technical implementation details (optional) 1. Add `GetTextAttributes` and `SetTextAttributes` methods to the `ITerminalApi` interface to match the `PrivateGetTextAttributes` and `PrivateSetTextAttributes` methods in the `ConGetSet` interface. 2. Essentially just copy and paste the `AdaptDispatch::SetGraphicsRendition` implementation into `TerminalDispatch`. I think the only difference is we don't need the color index remapping, and the color index constants are different for the same reason (resolving that discrepancy is still on my TODO list). 3. Delete all the methods in the `ITerminalApi` interface that are no longer required.
claunia added the Issue-FeatureNeeds-TriageResolution-Fix-CommittedNeeds-Tag-Fix labels 2026-01-31 01:52:02 +00:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#9337