-
Notifications
You must be signed in to change notification settings - Fork 27.2k
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
Deprecate WhitelistingTextInputFormatter and BlacklistingTextInputFormatter #59120
Conversation
Might be easier to review by reviewing the two commits separately. |
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.
This is awesome! The analyzer has some thoughts though.
@@ -13,3 +13,9 @@ export 'date_picker_deprecated.dart'; | |||
export 'date_picker_dialog.dart' show showDatePicker; | |||
export 'date_range_picker_dialog.dart' show showDateRangePicker; | |||
export 'input_date_picker.dart' show InputDatePickerFormField; | |||
|
|||
// TODO(ianh): Not exporting everything is unusual and we should |
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.
@darrenaustin may already have a plan for this.
/// Old name for [FilteringTextInputFormatter.deny]. | ||
@Deprecated( | ||
'Use FilteringTextInputFormatter.deny instead. ' | ||
'This feature was deprecated after 1.19.0-4.0.pre' |
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.
'This feature was deprecated after 1.19.0-4.0.pre' | |
'This feature was deprecated after 1.19.0-4.0.pre.' |
I think the deprecations here are just missing a period to make the analyzer happy. :)
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.
d'oh
It looks like customer tests may have to be updated as well. |
I need to talk to the gallery folks, yeah. |
681e600
to
9591ed4
Compare
Update: PR should have all the tests fixed now, and I spoke to the Gallery folks, they'll update after this lands (I made the specific deprecations that were failing on the Gallery not be failures for now; I'll deprecate them later once there's no breaking changes to worry about). |
@Piinks PTAL |
Fixes #59099 |
Merge conflicts fixed. Should be ready to land assuming no problems are found in review. |
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
/// in separate passes. | ||
/// | ||
/// See also [String.splitMapJoin], which is used to implement this | ||
/// behavior in both cases. |
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.
Nice comprehensive doc comment!
static final BlacklistingTextInputFormatter singleLineFormatter | ||
= BlacklistingTextInputFormatter(RegExp(r'\n')); | ||
} | ||
|
||
/// Old name for [FilteringTextInputFormatter.allow]. | ||
// TODO(ianh): Deprecate these once the samples are migrated. |
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.
Which samples is this referring to?
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.
Maybe add a link?
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.
gallery in samples. they've said they'll fix it when i land this.
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.
Looks like cirrus is unhappy, though. |
…matter They are replaced by the single class FilteringTextInputFormatter.
Since I ignored this lint rather than adding a bunch of //ignores in the earlier commit, I also go and clean up other places we ignored this lint.
Reverting this TBR by @Hixie as it caused LUCI post-submit to fail with:
|
…InputFormatter (flutter#59120)" (flutter#59870) This reverts commit 8665e13.
…tInputFormatter (flutter#59120)" (flutter#59876) This relands flutter#59120, which was reverted in flutter#59870.
…InputFormatter (flutter#59120)" (flutter#59870) This reverts commit 8665e13.
…tInputFormatter (flutter#59120)" (flutter#59876) This relands flutter#59120, which was reverted in flutter#59870.
Since this is deprecated, what should we use instead? |
Got it, eg:
|
They are replaced by the single class FilteringTextInputFormatter.
Also, Remove deprecated_member_use_from_same_package. Since I ignore this lint rather than adding a bunch of //ignores, I also go and clean up other places we ignored this lint.
fyi @Piinks for the deprecation stuff
fyi @xster since you added these classes