Skip to content

Remove the FontSizeChanged event from TermControl#15867

Merged
zadjii-msft merged 4 commits intomainfrom
dev/migrie/b/1104-its-high-time-to-remove-this
Aug 24, 2023
Merged

Remove the FontSizeChanged event from TermControl#15867
zadjii-msft merged 4 commits intomainfrom
dev/migrie/b/1104-its-high-time-to-remove-this

Conversation

@zadjii-msft
Copy link
Member

I originally just wanted to close #1104, but then discovered that hey, this event wasn't even used anymore. Excerpts of Teams convo:

Pane::Relayout functionally did nothing because sizing was switched
to star sizing at some point in the past, so it was just deleted.

From Misc pane refactoring by Rosefield · Pull Request #11373 · microsoft/terminal

So, great. We can kill part of it, and convert the rest to a TypedEvent, and get rid of DECLARE_ / DEFINE_.

ScrollPositionChangedEventArgs was ALSO apparently already promoted to a typed event, so kill that too.

@microsoft-github-policy-service microsoft-github-policy-service bot added Issue-Task It's a feature request, but it doesn't really need a major design. Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Product-Terminal The new Windows Terminal. labels Aug 22, 2023

const auto actualNewSize = _actualFont.GetSize();
_FontSizeChangedHandlers(actualNewSize.width, actualNewSize.height, initialUpdate);
_FontSizeChangedHandlers(*this, *winrt::make_self<FontSizeChangedArgs>(actualNewSize.width, actualNewSize.height));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*make_self<T>() is equivalent to make<T> when T's default interface is the same as the one you're converting to.

For new code, we should prefer the clearer one :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot that make works even on a non projected ctor

@zadjii-msft zadjii-msft enabled auto-merge (squash) August 23, 2023 18:36
Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot that make works even on a non projected ctor

@DHowett
Copy link
Member

DHowett commented Aug 24, 2023

I pushed a merge so that it would kick the CI

@zadjii-msft zadjii-msft merged commit 0f61b5f into main Aug 24, 2023
@zadjii-msft zadjii-msft deleted the dev/migrie/b/1104-its-high-time-to-remove-this branch August 24, 2023 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

All TermControl/TermApp/TermConnection event handlers should be TypedEventHandlers

3 participants