Skip to content

Arrow syntax for getters - #156483

Merged
auto-submit[bot] merged 3 commits into
flutter:masterfrom
nate-thegrate:arrow-syntax
Oct 17, 2024
Merged

auto-submit[bot] merged 3 commits into
flutter:masterfrom
nate-thegrate:arrow-syntax

Conversation

@nate-thegrate

Copy link
Copy Markdown
Contributor

This egotistical PR aims to draw attention to a style guideline that I changed:

Use => for getters and callbacks that just return literals or switch expressions


There was an opportunity for valuable discussion in #154753 about how this structure affects readability, but I shut it down pretty quick since there was a lot of other stuff going on there.

Interested to hear thoughts from @Piinks and others.

@nate-thegrate
nate-thegrate requested a review from Piinks October 9, 2024 16:44
@github-actions github-actions Bot added a: tests "flutter test", flutter_test, or one of our tests tool Affects the "flutter" command-line tool. See also t: labels. framework flutter/packages/flutter repository. See also f: labels. a: animation Animation APIs p: material_ui material_ui package in flutter/packages f: scrolling Viewports, list views, slivers, etc. p: cupertino_ui cupertino_ui package in flutter/packages d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos f: routes Navigator, Router, and related APIs. a: desktop Running on desktop labels Oct 9, 2024
Comment thread dev/devicelab/lib/framework/ab.dart Outdated
Comment thread dev/integration_tests/new_gallery/lib/demos/material/picker_demo.dart Outdated
Comment thread dev/devicelab/lib/framework/ab.dart Outdated
Comment thread examples/api/lib/material/chip/deletable_chip_attributes.on_deleted.0.dart Outdated
return false;
}
}
bool get _isOnDesktopAndWeb => kIsWeb || switch (defaultTargetPlatform) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks incorrect based on the name.. should this be true only iff it is desktop && web?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

To be clear, I am wondering if the original logic was incorrect to begin with.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Absolutely: either the logic should be changed to use && or the getter should be renamed to _isOnDesktopOrWeb.

I could change it now, though perhaps that's outside the scope of this PR.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah definitely something to follow up on separately.

Comment thread examples/api/lib/widgets/page_view/page_view.0.dart
Comment thread packages/flutter_test/lib/src/controller.dart Outdated
Comment thread packages/flutter_tools/lib/src/build_system/targets/deferred_components.dart Outdated
Comment thread packages/flutter_tools/lib/src/flutter_manifest.dart Outdated
Comment thread packages/flutter_tools/lib/src/flutter_project_metadata.dart Outdated
@github-actions github-actions Bot removed a: tests "flutter test", flutter_test, or one of our tests f: routes Navigator, Router, and related APIs. labels Oct 17, 2024

@Piinks Piinks left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@Piinks

Piinks commented Oct 17, 2024

Copy link
Copy Markdown
Contributor

Rebasing should fix the failing check.

@nate-thegrate nate-thegrate added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 17, 2024
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 17, 2024
@auto-submit

This comment was marked as resolved.

@nate-thegrate nate-thegrate added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 17, 2024
@auto-submit
auto-submit Bot merged commit 6b1bc45 into flutter:master Oct 17, 2024
@nate-thegrate
nate-thegrate deleted the arrow-syntax branch October 17, 2024 19:25
@kevmoo

kevmoo commented Oct 18, 2024

Copy link
Copy Markdown
Contributor

Yes, please! More of this!

@nate-thegrate nate-thegrate added the refactor Improving readability/efficiency without behavioral changes label Oct 18, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: animation Animation APIs a: desktop Running on desktop autosubmit Merge PR when tree becomes green via auto submit App d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels. p: cupertino_ui cupertino_ui package in flutter/packages p: material_ui material_ui package in flutter/packages refactor Improving readability/efficiency without behavioral changes tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants