-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Sort diagnostics node dependencies so that order is stable #105319
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
| .map((InheritedElement element) => element.widget.toDiagnosticsNode(style: DiagnosticsTreeStyle.sparse)) | ||
| .toList(); | ||
| .toList() | ||
| ..sort((DiagnosticsNode a, DiagnosticsNode b) => '$a'.compareTo('$b')); |
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.
the toString for a DiagnosticsNode can be long and expensive (although shouldn't really be in this case). I would just use
toStringShort() instead to be safe to make sure we don't end up doing O(n log n) computations of the full toString version of a diagnostic.
jacob314
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.
| properties.add(FlagProperty('dirty', value: dirty, ifTrue: 'dirty')); | ||
| if (_dependencies != null && _dependencies!.isNotEmpty) { | ||
| final List<DiagnosticsNode> diagnosticsDependencies = _dependencies! | ||
| .map((InheritedElement element) => element.widget.toDiagnosticsNode(style: DiagnosticsTreeStyle.sparse)) |
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.
Sort while you still have a List and you can use toStringShort or just the runtime type of the Widget class which would also be reasonable.
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.
good idea. done.
jacob314
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.
…05319) * Sort diagnostics node dependencies so that order is stable * use toDescription * use toStringShort when possible * sort InheritedElements

This will fix flutter/devtools#4081.
@jacob314