Implementing `switch` expressions in `rendering/` #143812

nate-thegrate · 2024-02-21T02:49:05Z

This pull request is part of the effort to solve issue #136139.

The previous switch expressions PR was comprised of many simple changes throughout flutter/lib/src/, but due to some more in-depth refactoring in flutter/lib/src/rendering/, I decided to submit the changes to this directory as a separate pull request.

There was really just one function that I changed significantly; I'll add a comment for explanation.

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
All existing and new tests are passing.

packages/flutter/lib/src/rendering/sliver_padding.dart

goderbauer · 2024-02-21T18:09:28Z

@Piinks can you take a look at this one since it is mostly slivers and scrolling?

Piinks · 2024-02-21T21:23:00Z

@Piinks can you take a look at this one since it is mostly slivers and scrolling?

For sure!

Piinks · 2024-02-21T21:51:47Z

packages/flutter/lib/src/rendering/flex.dart

        lastFlexChild = child;
      } else {
        final BoxConstraints innerConstraints;
        if (crossAxisAlignment == CrossAxisAlignment.stretch) {


Hmm, this caught my eye. It is an enum without an exhaustive switch statement!
Do you think crossAxisAlignment and _direction can be used together?

I think there is one other if (crossAxisAlignment == CrossAxisAlignment.stretch) { in this file too if it seems feasible to alter the loci to be exhaustive.

What do you think?

I think that sounds great 🙂👍

I updated _computeSizes() to get rid of every == enum comparison. Really happy with how it looks now!

Piinks · 2024-02-21T21:54:04Z

packages/flutter/lib/src/rendering/decorated_sliver.dart

-      final Size childSize;
-      final Offset scrollOffset;


Could these be combined into a record for one switch statement? I noticed that in packages/flutter/lib/src/rendering/paragraph.dart in this PR. :)

Yes they could! I had them combined originally but the line length was a bit much.

But after looking over it again, I think we could make it work by inverting the if-statement, so it returns early instead of indenting the whole function.

While I'm at it, I'll refactor position into an exhaustive switch statement too :)

packages/flutter/lib/src/rendering/sliver_padding.dart

Piinks

LGTM!

goderbauer

LGTM

…utter#143812)

flutter/flutter@b77560e...c30f998 2024-02-27 [email protected] [flutter_tools] Fix missing stack trace from daemon (flutter/flutter#144113) 2024-02-27 [email protected] Roll Flutter Engine from 04ff2868ce80 to 0bc21ea7bc92 (21 revisions) (flutter/flutter#144208) 2024-02-26 [email protected] Move debugShowWidgetInspectorOverride (flutter/flutter#144029) 2024-02-26 [email protected] Allow `Listenable.merge()` to use any iterable (flutter/flutter#143675) 2024-02-26 [email protected] Mark firebase_release_smoke_test as `bringup: true` (flutter/flutter#144186) 2024-02-26 [email protected] refactor: Differentiate pubspec and resolution errors for plugins (flutter/flutter#142035) 2024-02-26 [email protected] Roll Flutter Engine from 34a8b9bdaaac to 04ff2868ce80 (1 revision) (flutter/flutter#144159) 2024-02-26 [email protected] Implementing `switch` expressions in `rendering/` (flutter/flutter#143812) 2024-02-26 [email protected] Roll Flutter Engine from 7a573c21a4ad to 34a8b9bdaaac (1 revision) (flutter/flutter#144147) 2024-02-26 [email protected] Roll Flutter Engine from a15326b2c439 to 7a573c21a4ad (1 revision) (flutter/flutter#144145) 2024-02-26 [email protected] Update copyDirectory to allow links to not be followed (flutter/flutter#144040) 2024-02-26 [email protected] Roll Packages from 7df2085 to 353086c (7 revisions) (flutter/flutter#144144) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

This pull request is step 12 in the journey to make this entire repository more readable. (previous PRs: #139048, #139882, #141591, #142279, #142634, #142793, #143293, #143496, #143634, #143812, #144580) We're getting close to the end! ð��

…utter#143812)

implement switch expressions in lib/src/rendering/

bce9b45

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: scrolling Viewports, list views, slivers, etc. labels Feb 21, 2024

nate-thegrate commented Feb 21, 2024

View reviewed changes

packages/flutter/lib/src/rendering/sliver_padding.dart Show resolved Hide resolved

nate-thegrate marked this pull request as ready for review February 21, 2024 03:21

whitespace

2b7891f

Piinks reviewed Feb 21, 2024

View reviewed changes

nate-thegrate added 3 commits February 21, 2024 17:46

no more enum ==!

559ac73

if-statement readability

408005a

Merge branch 'master' into switch-expressions-10

779eb4a

nate-thegrate requested a review from Piinks February 23, 2024 16:59

Piinks approved these changes Feb 24, 2024

View reviewed changes

Piinks requested a review from goderbauer February 24, 2024 00:10

goderbauer approved these changes Feb 26, 2024

View reviewed changes

goderbauer added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 26, 2024

auto-submit bot merged commit 60d28ad into flutter:master Feb 26, 2024

nate-thegrate deleted the switch-expressions-10 branch February 26, 2024 17:36

engine-flutter-autoroll mentioned this pull request Feb 27, 2024

Roll Flutter from b77560e92a58 to c30f998eb5db (12 revisions) flutter/packages#6211

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 27, 2024

60d28ad Implementing switch expressions in rendering/ (flutter/fl…

d228dca

…utter#143812)

HansMuller mentioned this pull request Mar 7, 2024

Flutter Framework Team: Notable Commits #121415

Closed

nate-thegrate mentioned this pull request Mar 23, 2024

Implementing switch expressions in flutter_tools/ #145632

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 14, 2024

60d28ad Implementing switch expressions in rendering/ (flutter/fl…

2d94338

…utter#143812)

nate-thegrate mentioned this pull request Jun 21, 2024

feat: Introduce WrapFit #136161

Closed

8 tasks

nate-thegrate mentioned this pull request Jul 2, 2024

Interactable ScrollView content when settling a scroll activity #145848

Merged

9 tasks

nate-thegrate added the refactor Improving readability/efficiency without behavioral changes label Jul 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Implementing `switch` expressions in `rendering/` #143812

Implementing `switch` expressions in `rendering/` #143812

Uh oh!

nate-thegrate commented Feb 21, 2024

Uh oh!

Uh oh!

goderbauer commented Feb 21, 2024

Uh oh!

Piinks commented Feb 21, 2024

Uh oh!

Piinks Feb 21, 2024

Uh oh!

nate-thegrate Feb 22, 2024

Uh oh!

Piinks Feb 21, 2024

Uh oh!

nate-thegrate Feb 21, 2024

Uh oh!

Uh oh!

Piinks left a comment

Uh oh!

goderbauer left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Implementing switch expressions in rendering/ #143812

Implementing switch expressions in rendering/ #143812

Uh oh!

Conversation

nate-thegrate commented Feb 21, 2024

Pre-launch Checklist

Uh oh!

Uh oh!

goderbauer commented Feb 21, 2024

Uh oh!

Piinks commented Feb 21, 2024

Uh oh!

Piinks Feb 21, 2024

Choose a reason for hiding this comment

Uh oh!

nate-thegrate Feb 22, 2024

Choose a reason for hiding this comment

Uh oh!

Piinks Feb 21, 2024

Choose a reason for hiding this comment

Uh oh!

nate-thegrate Feb 21, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Piinks left a comment

Choose a reason for hiding this comment

Uh oh!

goderbauer left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Implementing `switch` expressions in `rendering/` #143812

Implementing `switch` expressions in `rendering/` #143812