-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Change GlobalObjectKey.toString to strip away State<StatefulWidget>. #14558
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
This allows const GlobalObjectKey(0) to be concisely formatted as [GlobalObjectKey int#0000] in both Dart 2 and Dart 1 modes. Without this change it would be formatted as [GlobalObjectKey<State<StatefulWidget>> int#0000] because in Dart 2 types are instantiated to bounds. In addition to retaining general readability this also fixes few tests that rely on this short string representation (see test/widgets/global_keys_duplicated_test.dart).
| // const GlobalObjectKey<State<StatefulWidget>>(). | ||
| if (selfType == 'GlobalObjectKey<State<StatefulWidget>>') { | ||
| selfType = 'GlobalObjectKey'; | ||
| } |
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.
why is this only a temporary work-around?
Seems like we'll need something permanently for this, no?
Can we compare T instead of the reified string? That might be cleaner.
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.
I will remove the word temporary from my comment.
Comparison against T does not work because GlobalObjectKey can have subclasses, so if I do something like:
if (T == State) {
selfType = 'GlobalObjectKey';
}that would be incorrect for subclasses.
Alternative to full matching I could do:
const suffix = '<State<StatefulWidget>>'
if (selfType.endsWith(suffix)) {
selfType = selfType.substring(0, selfType.length - suffix.length);
}What do you think?
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.
I will remove the word temporary from my comment.
I would just remove all the mention of Dart 1, too. We're not doing this because of what Dart 1 did, we're doing it because we want a particular effect to make the dumps less confusing and cleaner, and that's how you get the effect.
...subclasses...
Ah, yeah. Your proposed code seems reasonable. We should test for that too if we don't already (and test for the case where we do want the output to include generics).
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.
Addressed comments, added test. Please take another look.
|
Analyzer is now happy too. |
|
@Hixie ping |
* master: (88 commits) Upgrade dartdoc to 0.16.0. (flutter#14602) Reduce noise in xcodebuild stdout (flutter#14586) Add annotations to ignore analyzer errors on pre-dev.22 SDKs (flutter#14599) Roll engine to 337764e (flutter#14597) Roll engine with rolled dart (flutter#14538) Upgrade packages (flutter#14588) Change GlobalObjectKey.toString to strip away State<StatefulWidget>. (flutter#14558) Disable selection of the Android ARM64 target platform based on the attached device (flutter#14581) Add a flutter run option that can override the default target platform (flutter#14537) Fix message channel usage in few tests. (flutter#14557) Update documentation on how to test flutter_tools (flutter#14567) Fix text_style_text.dart to be Dart 2 compliant. (flutter#14559) Hide the NDK warning that should never happen with a regexp (flutter#14503) Fix iOS build which broke after the change to fuse --strong option into --preview-dart-2. Mark run_machine_concurrent_hot_reload as flaky (flutter#14563) Fuse --strong into --preview-dart-2 option. Partially fix Dart 2 issues in animated_icons.test. (flutter#14531) Strong mode fixes in tests (flutter#14520) Fix typo. Fix issues. ...
…lutter#14558) This allows const GlobalObjectKey(0) to be concisely formatted as [GlobalObjectKey int#0000] in both Dart 2 and Dart 1 modes. Without this change it would be formatted as [GlobalObjectKey<State<StatefulWidget>> int#0000] because in Dart 2 types are instantiated to bounds. In addition to retaining general readability this also fixes few tests that rely on this short string representation (see test/widgets/global_keys_duplicated_test.dart).
This allows const GlobalObjectKey(0) to be concisely formatted as
[GlobalObjectKey int#0000] in both Dart 2 and Dart 1 modes.
Without this change it would be formatted as
[GlobalObjectKey<State> int#0000] because in Dart 2
types are instantiated to bounds.
In addition to retaining general readability this also fixes few
tests that rely on this short string representation (see
test/widgets/global_keys_duplicated_test.dart).