Skip to content

[pigeon] Improve style of generated Swift code #5938

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

stuartmorgan-g
Copy link
Contributor

Cleans up our Swift output to be more idiomatic (or in a couple of cases to match swift-format` in particular):

  • Don't indent cases in switches.
  • Remove some stray ;s
  • Remove ()s around if conditions
  • Add trailing newlines in multi-element arrays
  • Fix some missing standard whitespace in a handful of places where it was missing (after named arguments, between () and {)
  • Indent inside #if constructs.
  • Two changes that are more arguable, as it's not clear that they are non-idiomatic vs just non-swift-format-style (and they add branching to the generator that would otherwise not be needed):
    • Don't put a trailing comma on the list element of a one-element list
    • Don't put () on zero-arg method calls with a trailing closure

With these changes, as far as can tell the diffs created by swift-format are just line wrapping.

Fixes flutter/flutter#141799

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 relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
  • I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
  • 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.

@stuartmorgan-g
Copy link
Contributor Author

Let me know if you want me to revert the two special-case style things at the end of the list. I went back and forth on whether to write them, and still don't feel strongly either way.

@tarrinneal
Copy link
Contributor

Let me know if you want me to revert the two special-case style things at the end of the list. I went back and forth on whether to write them, and still don't feel strongly either way.

I actually would prefer to keep those implementations as well.

Copy link
Contributor

@tarrinneal tarrinneal left a comment

Choose a reason for hiding this comment

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

These look good to me, thanks @stuartmorgan

@stuartmorgan-g stuartmorgan-g added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 20, 2024
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 20, 2024
Copy link
Contributor

auto-submit bot commented Jan 20, 2024

auto label is removed for flutter/packages/5938, due to - The status or check suite Linux_android custom_package_tests stable has failed. Please fix the issues identified (or deflake) before re-applying this label.

@stuartmorgan-g stuartmorgan-g added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 20, 2024
@auto-submit auto-submit bot merged commit 2b0d290 into flutter:main Jan 20, 2024
@stuartmorgan-g stuartmorgan-g deleted the pigeon-swift-generation-style branch January 20, 2024 16:38
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 22, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jan 22, 2024
flutter/packages@129e08c...e4cbf23

2024-01-21 engine-flutter-autoroll@skia.org Roll Flutter from ddf60fb to 5dea6b9 (5 revisions) (flutter/packages#5951)
2024-01-21 stuartmorgan@google.com Update platform label rules for shared iOS/macOS (flutter/packages#5801)
2024-01-20 stuartmorgan@google.com [pigeon] Support other hosts in generated file CI checks (flutter/packages#5944)
2024-01-20 stuartmorgan@google.com [pigeon] Improve style of generated Swift code (flutter/packages#5938)
2024-01-20 engine-flutter-autoroll@skia.org Roll Flutter from 684247a to ddf60fb (12 revisions) (flutter/packages#5949)
2024-01-20 41930132+hellohuanlin@users.noreply.github.com [camera]fix a sample buffer memory leak on pause resume recording (flutter/packages#5927)
2024-01-19 magder@google.com [ci] Run Swift formatter and linter during CI formatting (flutter/packages#5928)
2024-01-19 engine-flutter-autoroll@skia.org Manual roll Flutter from f77f824 to 684247a (39 revisions) (flutter/packages#5948)
2024-01-19 john@johnmccutchan.com Expose registered widget libraries and local widget library widgets. (flutter/packages#5936)
2024-01-19 49699333+dependabot[bot]@users.noreply.github.com Bump actions/upload-artifact from 4.1.0 to 4.2.0 (flutter/packages#5937)
2024-01-19 engine-flutter-autoroll@skia.org Manual roll Flutter (stable) from ef1af02 to 67457e6 (1 revision) (flutter/packages#5932)

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-flutter-autoroll
Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: pigeon platform-ios platform-macos
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[pigeon] Improve Swift generator output style
2 participants