-
Notifications
You must be signed in to change notification settings - Fork 3.5k
[pigeon] Fixes double prefixes added to enum names for Objc HostApis and FlutterApis #6263
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
Conversation
|
What scenario is missing from our integration tests that would have triggered this? |
|
@tarrinneal It looks like the scenario is a combination of:
It looks like CoreTests.gen.h doesn't use a prefix. Although scenario 1 does occur for it. Would it be a good idea to update core tests with a prefix? |
yup |
|
@tarrinneal Adding the prefix to core tests also found a location where the prefix wasn't added to the enum. I think a better solution might be to actually get rid of the |
|
Actually it looks like the problem is bigger than enums. Objective-c prefixes may need a bigger fix. |
|
I can take this over if you'd like. |
f4808c1 to
4a8dc2f
Compare
4a8dc2f to
be16600
Compare
|
@stuartmorgan this is ready for final review |
stuartmorgan-g
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…ostApis and FlutterApis (flutter/packages#6263)
flutter/packages@b21c542...756dcc1 2024-03-15 32538273+ValentinVignal@users.noreply.github.com [go_router] Use `leak_tracker_flutter_testing` (flutter/packages#6210) 2024-03-15 10687576+bparrishMines@users.noreply.github.com [camera_web][google_maps_flutter] Fix tests throwing errors after test completion with manual roll (flutter/packages#6318) 2024-03-14 10687576+bparrishMines@users.noreply.github.com [pigeon] Fixes double prefixes added to enum names for Objc HostApis and FlutterApis (flutter/packages#6263) 2024-03-14 10687576+bparrishMines@users.noreply.github.com [webview_flutter_android][webview_flutter_wkwebview] Adds platform implementations for onHttpError (flutter/packages#6149) 2024-03-14 34871572+gmackall@users.noreply.github.com [image_picker_android] Fix deprecation warnings by branching based on build version, and suppressing only when needed (flutter/packages#6233) 2024-03-14 30870216+gaaclarke@users.noreply.github.com [google_maps_flutter] Started dispatching platform messages from platform thread (flutter/packages#6069) 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
In some areas of the Objc generator, the the name created from _objcTypeForDartType is being passed to _enumName and both of these methods add a prefix to enums. The locations are when they are used in
HostApiorFlutterApimethods, but the name of the generated enum would be correct. e.g.FLTEnumNamevsFLTFLTEnumName.This fixes the locations where this is happening by passing the AST name to
_enumNameinstead.Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.