Convert PointerEvent's toString to Diagnosticable #27904
Conversation
Rather than building a description string by hand, use Diagnosticable properties. Simplifies output, since many fields have uninteresting default values in many cases, and makes it easier to add more loggable properties in subclasses.
|
I used my best judgment on when to use default values, and on some formatting specifics (e.g., DiagnosticsProperty for |
|
/cc @jacob314 |
|
@goderbauer Hixie suggested you for review, but feel free to redirect if there's a better candidate. |
|
I'm on vacation this week so I will be a bit slow to respond. Great to see
more classes use diagnosticable. It helps a lot for our GUI debugging tools.
…On Wed, Feb 13, 2019, 1:35 PM stuartmorgan ***@***.*** wrote:
@goderbauer <https://github.com/goderbauer> Hixie suggested you for
review, but feel free to redirect if there's a better candidate.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#27904 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABK4PNbGqvjBAPRxOyMUmVxz1apsHPm8ks5vNKEwgaJpZM4a6eem>
.
|
zoechi
added
framework
Ah sorry, when you said not to worry about the breaking change because nobody would have done it I thought that meant it didn't need the full process. Will do when I have time. |
|
I would be really surprised if any users are broken by refining toStringFull to not include irrelevant information. My guess would be that irrelevant information was previously included in it just because it is too inconvenient to filter out properties matching the default value when using a single large string interpolation template but someone more familiar with the API would have a better idea. |
|
Updated; this should now cause only minor tweaks in terms of output behavior from calling |
jacob314
left a comment
jacob314
left a comment
There was a problem hiding this comment.
The use of DiagnosticLevel.debug for all the unimportant properties looks good to me.
|
The breaking change process is underway. Meanwhile, @goderbauer @Hixie any other comments/concerns with the current version of the patch? |
6 participants
Rather than building a description string by hand, use Diagnosticable
properties. Simplifies output, since many fields have uninteresting
default values in many cases, and makes it easier to add more loggable
properties in subclasses.
Issue #28014