Add toString methods for window positioner classes#182857
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
There was a problem hiding this comment.
Code Review
This pull request adds toString() methods to WindowPositionerConstraintAdjustment and WindowPositioner to aid in debugging. My review focuses on improving the code style and formatting of these new methods to align with the repository's guidelines. I've suggested using an expression body for conciseness and formatting long strings to improve readability.
| @override | ||
| String toString() => | ||
| 'WindowPositionerConstraintAdjustment(flipX: $flipX, flipY: $flipY, slideX: $slideX, slideY: $slideY, resizeX: $resizeX, resizeY: $resizeY)'; |
There was a problem hiding this comment.
This toString implementation results in a very long line of code, which goes against the repository's style guide that enforces dart format and its line length limits. For better readability and to adhere to the style guide, please format this long string to span multiple lines.
@override
String toString() => 'WindowPositionerConstraintAdjustment('
'flipX: $flipX, '
'flipY: $flipY, '
'slideX: $slideX, '
'slideY: $slideY, '
'resizeX: $resizeX, '
'resizeY: $resizeY)';References
- All Dart code should be formatted using
dart format, which typically enforces an 80-character line limit to maintain readability. This is specified on line 36 of the repository style guide. (link)
| @override | ||
| String toString() { | ||
| return 'WindowPositioner(parentAnchor: $parentAnchor, childAnchor: $childAnchor, offset: $offset, constraintAdjustment: $constraintAdjustment)'; | ||
| } |
There was a problem hiding this comment.
For consistency and to adhere to Effective Dart style guidelines (referenced in the repo style guide), it's better to use an expression body (=>) for this simple one-line method. Additionally, the resulting string is very long and should be formatted across multiple lines to respect the line length limit.
@override
String toString() => 'WindowPositioner('
'parentAnchor: $parentAnchor, '
'childAnchor: $childAnchor, '
'offset: $offset, '
'constraintAdjustment: $constraintAdjustment)';References
- The repository style guide (line 20) defers to 'Effective Dart: Style', which recommends using
=>for simple one-line members for conciseness. (link) - All Dart code should be formatted using
dart format, which typically enforces an 80-character line limit to maintain readability. This is specified on line 36 of the repository style guide. (link)
Function was added in #182857
Useful for debugging.
Function was added in flutter#182857
Useful for debugging.
Function was added in flutter#182857
Useful for debugging.
Function was added in flutter#182857
Useful for debugging.