Skip to content

Conversation

@lhecker
Copy link
Member

@lhecker lhecker commented Dec 16, 2025

A continuation of #19645. Maintainer modifications were disabled, unfortunately.

@lhecker
Copy link
Member Author

lhecker commented Dec 16, 2025

pr_with_some_chest_hair

@github-actions

This comment has been minimized.

til::rect _rcAltSavedClientOld{};
bool _fAltWindowChanged{ false };

SCREEN_INFORMATION* _psiAlternateBuffer = nullptr; // The VT "Alternate" screen buffer.
Copy link
Member

Choose a reason for hiding this comment

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

hey, there's nothing wrong with type name{ value } syntax :(

Copy link
Member Author

Choose a reason for hiding this comment

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

There's many arguments that can be made against {}. For instance, anything that compiles with = means that initialization is implicit. Assuming no one wrote a non-trivial non-explicit object constructor, this further implies that seeing = means that it's definitely trivial (not just implicit), and that {} is potentially non-trivial and should raise an eye-brow).

That of course excludes things such as default constructors that are non-trivial such as cppwinrt's which allocate an object to avoid "null"s (BAH! I mean it's nice, but BAH!).

@DHowett
Copy link
Member

DHowett commented Dec 16, 2025

but also, why

@lhecker
Copy link
Member Author

lhecker commented Dec 16, 2025

I mean the SCREEN_INFORMATION class is a disgrace no matter how you look at it. #19645 was also kind of meh, because it brought no real benefit. To make things right, this PR at least brings some real improvement: It groups the class methods by their use. This makes it easy in the future to find related functions at a glance (not always obvious given their name), and to clean up the class (because now all of the window methods are grouped together, for instance).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants