Skip to content

Conversation

@gspencergoog
Copy link
Contributor

@gspencergoog gspencergoog commented Mar 9, 2023

Description

This adds an auto-migration to the tool for updating the Runner's Info.plist file in projects that are run so that if it has a key for NSPrincipalClass that is set to NSApplication, then it updates it to be FlutterApplication, so that the exitApplication and didRequestAppExit functionality will be functional. Otherwise, it's conservative, and won't modify Info.plist files that don't contain an NSPrincipalClass or if the value is something other than NSApplication.

It uses the XCode plutil tool to modify the Info.plist, and adds a replaceKey function to the PlistParser class that uses the tool to remove or replace a key with a new value.

Tests

  • Added tests for the migration.

@flutter-dashboard flutter-dashboard bot added platform-ios iOS applications specifically tool Affects the "flutter" command-line tool. See also t: labels. labels Mar 9, 2023
@gspencergoog gspencergoog requested a review from cbracken March 9, 2023 22:52
@gspencergoog gspencergoog force-pushed the nsapplication_migrate branch 2 times, most recently from 5651c36 to 903f9de Compare March 13, 2023 23:27
Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

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

lgtm - one tiny suggestion around the wording of a message.

@gspencergoog gspencergoog force-pushed the nsapplication_migrate branch from 310f1ae to e37e812 Compare March 15, 2023 20:38
@gspencergoog gspencergoog added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 15, 2023
@auto-submit auto-submit bot merged commit c2f5bf9 into flutter:master Mar 16, 2023
cbracken added a commit to cbracken/flutter that referenced this pull request Apr 5, 2023
In flutter#122336 we migrated the principal class from NSApplication to
FlutterApplication in the app Info.plist. We removed the need for
FlutterApplicaiton in flutter/engine#40929. This
reverses the migration for anyone who previously upgraded on the Flutter
master branch.

Issue: flutter#30735
cbracken added a commit that referenced this pull request Apr 5, 2023
In #122336 we migrated the principal class from NSApplication to
FlutterApplication in the app Info.plist. We removed the need for
FlutterApplication in flutter/engine#40929. This
reverses the migration for anyone who previously upgraded on the Flutter
master branch.

Issue: #30735

## Pre-launch Checklist

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

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
exaby73 pushed a commit to NevercodeHQ/flutter that referenced this pull request Apr 17, 2023
In flutter#122336 we migrated the principal class from NSApplication to
FlutterApplication in the app Info.plist. We removed the need for
FlutterApplication in flutter/engine#40929. This
reverses the migration for anyone who previously upgraded on the Flutter
master branch.

Issue: flutter#30735

## Pre-launch Checklist

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

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
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 platform-ios iOS applications specifically tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants