Duplicate code in TermControl.cpp #15657

Closed
opened 2026-01-31 04:44:46 +00:00 by claunia · 3 comments
Owner

Originally created by @serd2011 on GitHub (Oct 22, 2021).

Originally assigned to: @zadjii-msft on GitHub.

While reading code in TermControl.cpp i stumble upon two lines of code that are absolutely the same and are called one after another.
670ae2bd1c/src/cascadia/TerminalControl/TermControl.cpp (L333-L334)
670ae2bd1c/src/cascadia/TerminalControl/TermControl.cpp (L380-L381)

They are both called from TermControl::UpdateSettings()
_UpdateSettingsFromUIThread -> _ApplyUISettings and _UpdateAppearanceFromUIThread

Not sure if this is because of the situations in which this functions might be called separately but this just looks weird

Originally created by @serd2011 on GitHub (Oct 22, 2021). Originally assigned to: @zadjii-msft on GitHub. While reading code in [TermControl.cpp](https://github.com/microsoft/terminal/blob/main/src/cascadia/TerminalControl/TermControl.cpp) i stumble upon two lines of code that are absolutely the same and are called one after another. https://github.com/microsoft/terminal/blob/670ae2bd1c78fd2cc5b8e88f426eacab71fe1bfb/src/cascadia/TerminalControl/TermControl.cpp#L333-L334 https://github.com/microsoft/terminal/blob/670ae2bd1c78fd2cc5b8e88f426eacab71fe1bfb/src/cascadia/TerminalControl/TermControl.cpp#L380-L381 They are both called from ```TermControl::UpdateSettings()``` ```_UpdateSettingsFromUIThread``` -> ```_ApplyUISettings``` and ```_UpdateAppearanceFromUIThread``` Not sure if this is because of the situations in which this functions might be called separately but this just looks weird
Author
Owner

@zadjii-msft commented on GitHub (Oct 23, 2021):

I think I already got rid of this code in the branch I'm working on this 😋 But I'll leave it open to make sure.

@zadjii-msft commented on GitHub (Oct 23, 2021): I think I already got rid of this code in the branch I'm working on this 😋 But I'll leave it open to make sure.
Author
Owner

@zadjii-msft commented on GitHub (Oct 26, 2021):

Just double checked this, yep, I've got this fixed. I'll include this one in the PR.

@zadjii-msft commented on GitHub (Oct 26, 2021): Just double checked this, yep, I've got this fixed. I'll include this one in the PR.
Author
Owner

@ghost commented on GitHub (Feb 3, 2022):

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

Handy links:

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